apollo-server-core: use UserInputError for variable coercion errors (#…

…5091)

This particular error can be trivially triggered by clients, so it doesn't make
sense to use `INTERNAL_SERVER_ERROR` for it. It seems like a good fit for
`BAD_USER_INPUT`, which previously was only used if you explicitly throw a
`UserInputError` in your app.

Fixes #3498.

CHANGELOG.md

@@ -15,6 +15,7 @@ The version headers in this history reflect the versions of Apollo Server itself
 - `apollo-server-cache-redis`: New `BaseRedisCache` class which takes an `ioredis`-compatible Redis client as an argument. The existing classes `RedisCache` and `RedisClusterCache` (which pass their arguments to `ioredis` constructors) are now implemented in terms of this class. This allows you to use any of the `ioredis` constructor forms rather than just the ones recognized by our classes. This also fixes a long-standing bug where the Redis cache implementations returned a number from `delete()`; it now returns a number, matching what the `KeyValueCache` interface and the TypeScript types expect. [PR #5034](https://github.com/apollographql/apollo-server/pull/5034) [PR #5088](https://github.com/apollographql/apollo-server/pull/5088) [Issue #4870](https://github.com/apollographql/apollo-server/issues/4870) [Issue #5006](https://github.com/apollographql/apollo-server/issues/5006)
 - `apollo-server-core`: Fix type for `formatResponse` function. It never is called with a `null` argument, and is allowed to return `null`. [Issue #5009](https://github.com/apollographql/apollo-server/issues/5009) [PR #5089](https://github.com/apollographql/apollo-server/pull/5089)
 - `apollo-server-lambda`: Fix regression in v2.21.2 where thrown errors were replaced by throwing the JS Error class itself. [PR #5085](https://github.com/apollographql/apollo-server/pull/5085)
+- `apollo-server-core`: If a client sends a variable of the wrong type, this is now reported as an error with an `extensions.code` of `BAD_USER_INPUT` rather than `INTERNAL_SERVER_ERROR`. [PR #5091](https://github.com/apollographql/apollo-server/pull/5091) [Issue #3498](https://github.com/apollographql/apollo-server/issues/3498)
 
 ## v2.22.2
 

packages/apollo-server-core/src/requestPipeline.ts

@@ -10,6 +10,7 @@ import {
   validate as graphqlValidate,
   parse as graphqlParse,
   execute as graphqlExecute,
+  Kind,
 } from 'graphql';
 import {
   GraphQLExtension,
@@ -31,6 +32,7 @@ import {
   PersistedQueryNotSupportedError,
   PersistedQueryNotFoundError,
   formatApolloErrors,
+  UserInputError,
 } from 'apollo-server-errors';
 import {
   GraphQLRequest,
@@ -451,16 +453,36 @@ export async function processGraphQLRequest<TContext>(
           requestContext as GraphQLRequestContextExecutionDidStart<TContext>,
         );
 
-        if (result.errors) {
-          await didEncounterErrors(result.errors);
+        // The first thing that execution does is coerce the request's variables
+        // to the types declared in the operation, which can lead to errors if
+        // they are of the wrong type. We change any such errors into
+        // UserInputError so that their code doesn't end up being
+        // INTERNAL_SERVER_ERROR, since these are client errors.
+        const resultErrors = result.errors?.map((e) => {
+          if (
+            e.nodes?.length === 1 &&
+            e.nodes[0].kind === Kind.VARIABLE_DEFINITION &&
+            e.message.startsWith(
+              `Variable "$${e.nodes[0].variable.name.value}" got invalid value `,
+            )
+          ) {
+            return fromGraphQLError(e, {
+              errorClass: UserInputError,
+            });
+          }
+          return e;
+        });
+
+        if (resultErrors) {
+          await didEncounterErrors(resultErrors);
         }
 
         response = {
           ...result,
-          errors: result.errors ? formatErrors(result.errors) : undefined,
+          errors: resultErrors ? formatErrors(resultErrors) : undefined,
         };
 
-        executionDispatcher.reverseInvokeHookSync("executionDidEnd");
+        executionDispatcher.reverseInvokeHookSync('executionDidEnd');
       } catch (executionError) {
         executionDispatcher.reverseInvokeHookSync("executionDidEnd", executionError);
         return await sendErrorResponse(executionError);

packages/apollo-server-errors/src/index.ts

@@ -134,7 +134,9 @@ export function toApolloError(
 
 export interface ErrorOptions {
   code?: string;
-  errorClass?: typeof ApolloError;
+  // This declaration means it takes any "class" that has a constructor that
+  // takes a single string, and should be invoked via the `new` operator.
+  errorClass?: new (message: string) => ApolloError;
 }
 
 export function fromGraphQLError(error: GraphQLError, options?: ErrorOptions) {

packages/apollo-server-integration-testsuite/src/ApolloServer.ts

@@ -348,6 +348,27 @@ export function testApolloServer<AS extends ApolloServerBase>(
         });
       });
 
+      it('variable coercion errors', async () => {
+        const { url: uri } = await createApolloServer({
+          typeDefs: gql`
+            type Query {
+              hello(x: String): String
+            }
+          `,
+        });
+
+        const apolloFetch = createApolloFetch({ uri });
+
+        const result = await apolloFetch({
+          query: `query ($x:String) {hello(x:$x)}`,
+          variables: { x: 2 },
+        });
+        expect(result.data).toBeUndefined();
+        expect(result.errors).toBeDefined();
+        expect(result.errors[0].message).toMatch(/got invalid value 2; Expected type String/);
+        expect(result.errors[0].extensions.code).toBe('BAD_USER_INPUT');
+      });
+
       describe('schema creation', () => {
         it('accepts typeDefs and resolvers', async () => {
           const typeDefs = gql`