Invalid non-nullable or required List Type variable should return UserInputError

CHANGELOG.md

@@ -11,6 +11,7 @@ The version headers in this history reflect the versions of Apollo Server itself
 
 - `apollo-server-core`: The inline trace plugin will now include the full query plan and subgraph traces if manually installed in an Apollo Gateway. (Previously, you technically could install this plugin in a Gateway but it would not have any real trace data.) This is recommended for development use only and not in production servers. [PR #6017](https://github.com/apollographql/apollo-server/pull/6017)
 - `apollo-server-core`: The default landing page plugins now take an `includeCookies` option which allows you to specify that Explorer should send cookies to your server. [PR #6014](https://github.com/apollographql/apollo-server/pull/6014)
+- `apollo-server-core`: Apollo Server has a heuristic added in v2.23.0 and improved in v3.1.0 which tries to detect execution errors that come from the `graphql-js` variable value validation phase and report them with an `extensions.code` of `BAD_USER_INPUT` rather than `INTERNAL_SERVER_ERROR`. In this release, the heuristic is improved to include some cases including variables that are non-null lists. [PR #6066](https://github.com/apollographql/apollo-server/pull/6066)
 
 ## v3.6.2
 

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

@@ -98,6 +98,22 @@ export type DataSources<TContext> = {
 
 type Mutable<T> = { -readonly [P in keyof T]: T[P] };
 
+function isBadUserInputGraphQLError(error: GraphQLError): Boolean {
+  return (
+    error.nodes?.length === 1 &&
+    error.nodes[0].kind === Kind.VARIABLE_DEFINITION &&
+    (error.message.startsWith(
+      `Variable "$${error.nodes[0].variable.name.value}" got invalid value `,
+    ) ||
+      error.message.startsWith(
+        `Variable "$${error.nodes[0].variable.name.value}" of required type `,
+      ) ||
+      error.message.startsWith(
+        `Variable "$${error.nodes[0].variable.name.value}" of non-null type `,
+      ))
+  );
+}
+
 export async function processGraphQLRequest<TContext>(
   config: GraphQLRequestPipelineConfig<TContext>,
   requestContext: Mutable<GraphQLRequestContext<TContext>>,
@@ -400,23 +416,7 @@ export async function processGraphQLRequest<TContext>(
       // variable resolution from execution later; see
       // https://github.com/graphql/graphql-js/issues/3169
       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 `,
-          ) ||
-            (e.nodes[0].type.kind === Kind.NON_NULL_TYPE &&
-              e.nodes[0].type.type.kind === Kind.NAMED_TYPE &&
-              (e.message.startsWith(
-                `Variable "$${e.nodes[0].variable.name.value}" of required ` +
-                  `type "${e.nodes[0].type.type.name.value}!" was not provided.`,
-              ) ||
-                e.message.startsWith(
-                  `Variable "$${e.nodes[0].variable.name.value}" of non-null ` +
-                    `type "${e.nodes[0].type.type.name.value}!" must not be null.`,
-                ))))
-        ) {
+        if (isBadUserInputGraphQLError(e)) {
           return fromGraphQLError(e, {
             errorClass: UserInputError,
           });

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

@@ -320,72 +320,143 @@ export function testApolloServer<AS extends ApolloServerBase>(
         });
       });
 
-      it('variable coercion errors', async () => {
-        const { url: uri } = await createApolloServer({
-          typeDefs: gql`
-            type Query {
-              hello(x: String): String
-            }
-          `,
+      describe('appropriate error for bad user input', () => {
+        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; String cannot represent a non string value: 2/,
+          );
+          expect(result.errors[0].extensions.code).toBe('BAD_USER_INPUT');
         });
 
-        const apolloFetch = createApolloFetch({ uri });
+        it('catches required type variable error and returns UserInputError', async () => {
+          const { url: uri } = await createApolloServer({
+            typeDefs: gql`
+              type Query {
+                hello(x: String!): String
+              }
+            `,
+          });
 
-        const result = await apolloFetch({
-          query: `query ($x:String) {hello(x:$x)}`,
-          variables: { x: 2 },
+          const apolloFetch = createApolloFetch({ uri });
+
+          const result = await apolloFetch({
+            query: `query ($x:String!) {hello(x:$x)}`,
+          });
+          expect(result.data).toBeUndefined();
+          expect(result.errors).toBeDefined();
+          expect(result.errors[0].message).toMatch(
+            `Variable "$x" of required type "String!" was not provided.`,
+          );
+          expect(result.errors[0].extensions.code).toBe('BAD_USER_INPUT');
         });
-        expect(result.data).toBeUndefined();
-        expect(result.errors).toBeDefined();
-        expect(result.errors[0].message).toMatch(
-          /got invalid value 2; String cannot represent a non string value: 2/,
-        );
-        expect(result.errors[0].extensions.code).toBe('BAD_USER_INPUT');
-      });
 
-      it('catches required type variable error and returns UserInputError', async () => {
-        const { url: uri } = await createApolloServer({
-          typeDefs: gql`
-            type Query {
-              hello(x: String!): String
-            }
-          `,
+        it('catches required List type variable error and returns UserInputError', 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)}`,
+          });
+          expect(result.data).toBeUndefined();
+          expect(result.errors).toBeDefined();
+          expect(result.errors[0].message).toMatch(
+            `Variable "$x" of required type "[String]!" was not provided.`,
+          );
+          expect(result.errors[0].extensions.code).toBe('BAD_USER_INPUT');
         });
 
-        const apolloFetch = createApolloFetch({ uri });
+        it('catches non-null type variable error and returns UserInputError', async () => {
+          const { url: uri } = await createApolloServer({
+            typeDefs: gql`
+              type Query {
+                hello(x: String!): String
+              }
+            `,
+          });
 
-        const result = await apolloFetch({
-          query: `query ($x:String!) {hello(x:$x)}`,
+          const apolloFetch = createApolloFetch({ uri });
+
+          const result = await apolloFetch({
+            query: `query ($x:String!) {hello(x:$x)}`,
+            variables: { x: null },
+          });
+          expect(result.data).toBeUndefined();
+          expect(result.errors).toBeDefined();
+          expect(result.errors[0].message).toMatch(
+            `Variable "$x" of non-null type "String!" must not be null.`,
+          );
+          expect(result.errors[0].extensions.code).toBe('BAD_USER_INPUT');
         });
-        expect(result.data).toBeUndefined();
-        expect(result.errors).toBeDefined();
-        expect(result.errors[0].message).toMatch(
-          `Variable "$x" of required type "String!" was not provided.`,
-        );
-        expect(result.errors[0].extensions.code).toBe('BAD_USER_INPUT');
-      });
 
-      it('catches non-null type variable error and returns UserInputError', async () => {
-        const { url: uri } = await createApolloServer({
-          typeDefs: gql`
-            type Query {
-              hello(x: String!): String
-            }
-          `,
+        it('catches non-null List type variable error and returns UserInputError', 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: null },
+          });
+          expect(result.data).toBeUndefined();
+          expect(result.errors).toBeDefined();
+          expect(result.errors[0].message).toMatch(
+            `Variable "$x" of non-null type "[String]!" must not be null.`,
+          );
+          expect(result.errors[0].extensions.code).toBe('BAD_USER_INPUT');
         });
 
-        const apolloFetch = createApolloFetch({ uri });
+        it('catches List of non-null type variable error and returns UserInputError', async () => {
+          const { url: uri } = await createApolloServer({
+            typeDefs: gql`
+              type Query {
+                hello(x: [String!]!): String
+              }
+            `,
+          });
 
-        const result = await apolloFetch({
-          query: `query ($x:String!) {hello(x:$x)}`,
-          variables: { x: null },
+          const apolloFetch = createApolloFetch({ uri });
+
+          const result = await apolloFetch({
+            query: `query ($x:[String!]!) {hello(x:$x)}`,
+            variables: { x: [null] },
+          });
+          expect(result.data).toBeUndefined();
+          expect(result.errors).toBeDefined();
+          expect(result.errors[0].message).toBe(
+            `Variable "$x" got invalid value null at "x[0]"; ` +
+              `Expected non-nullable type "String!" not to be null.`,
+          );
+          expect(result.errors[0].extensions.code).toBe('BAD_USER_INPUT');
         });
-        expect(result.data).toBeUndefined();
-        expect(result.errors).toBeDefined();
-        expect(result.errors[0].message).toMatch(
-          `Variable "$x" of non-null type "String!" must not be null.`,
-        );
-        expect(result.errors[0].extensions.code).toBe('BAD_USER_INPUT');
       });
 
       describe('schema creation', () => {