From 551a5dde4f5b081caf69b57dda344de053008b19 Mon Sep 17 00:00:00 2001
From: David Glasser <glasser@davidglasser.net>
Date: Tue, 6 Apr 2021 23:59:14 +0000
Subject: [PATCH] apollo-server-core: use UserInputError for variable coercion
 errors

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.
---
 .../apollo-server-core/src/requestPipeline.ts | 29 ++++++++++++++++---
 packages/apollo-server-errors/src/index.ts    |  6 +++-
 .../src/ApolloServer.ts                       | 21 ++++++++++++++
 3 files changed, 51 insertions(+), 5 deletions(-)

diff --git a/packages/apollo-server-core/src/requestPipeline.ts b/packages/apollo-server-core/src/requestPipeline.ts
index d1a43dfdb9b..55da2cc086b 100644
--- a/packages/apollo-server-core/src/requestPipeline.ts
+++ b/packages/apollo-server-core/src/requestPipeline.ts
@@ -31,6 +31,7 @@ import {
   PersistedQueryNotSupportedError,
   PersistedQueryNotFoundError,
   formatApolloErrors,
+  UserInputError,
 } from 'apollo-server-errors';
 import {
   GraphQLRequest,
@@ -451,16 +452,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 === 'VariableDefinition' &&
+            e.message.startsWith(
+              `Variable "$${e.nodes[0].variable.name.value}" got invalid value `,
+            )
+          ) {
+            return fromGraphQLError(e, {
+              errorConstructor: (message) => new UserInputError(message),
+            });
+          }
+          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);
diff --git a/packages/apollo-server-errors/src/index.ts b/packages/apollo-server-errors/src/index.ts
index 8f2fb3ecb4f..702cafb1d64 100644
--- a/packages/apollo-server-errors/src/index.ts
+++ b/packages/apollo-server-errors/src/index.ts
@@ -134,12 +134,16 @@ export function toApolloError(
 
 export interface ErrorOptions {
   code?: string;
+  // Sometimes it's more convenient to pass a function than a class name.
+  errorConstructor?: (message: string) => ApolloError;
   errorClass?: typeof ApolloError;
 }
 
 export function fromGraphQLError(error: GraphQLError, options?: ErrorOptions) {
   const copy: ApolloError =
-    options && options.errorClass
+    options && options.errorConstructor
+      ? options.errorConstructor(error.message)
+      : options && options.errorClass
       ? new options.errorClass(error.message)
       : new ApolloError(error.message);
 
diff --git a/packages/apollo-server-integration-testsuite/src/ApolloServer.ts b/packages/apollo-server-integration-testsuite/src/ApolloServer.ts
index 706af759511..e7fa2a03c6e 100644
--- a/packages/apollo-server-integration-testsuite/src/ApolloServer.ts
+++ b/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`