fix(handler): Response maker handles errors correctly (#45)

Author: enisdenjo

src/__tests__/fixtures/simple.ts

@@ -4,6 +4,7 @@ import {
   GraphQLString,
   GraphQLNonNull,
   GraphQLSchemaConfig,
+  GraphQLInt,
 } from 'graphql';
 
 export const schemaConfig: GraphQLSchemaConfig = {
@@ -14,6 +15,14 @@ export const schemaConfig: GraphQLSchemaConfig = {
         type: new GraphQLNonNull(GraphQLString),
         resolve: () => 'world',
       },
+      num: {
+        type: GraphQLInt,
+        args: {
+          num: {
+            type: GraphQLInt,
+          },
+        },
+      },
     },
   }),
   mutation: new GraphQLObjectType({

src/__tests__/handler.ts

@@ -97,3 +97,26 @@ it('should respond with error if execution result is iterable', async () => {
     ],
   });
 });
+
+it('should correctly serialise execution result errors', async () => {
+  const server = startTServer();
+  const url = new URL(server.url);
+  url.searchParams.set('query', 'query ($num: Int) { num(num: $num) }');
+  url.searchParams.set('variables', JSON.stringify({ num: 'foo' }));
+  const result = await fetch(url.toString());
+  expect(result.json()).resolves.toMatchInlineSnapshot(`
+    {
+      "errors": [
+        {
+          "locations": [
+            {
+              "column": 8,
+              "line": 1,
+            },
+          ],
+          "message": "Variable "$num" got invalid value "foo"; Int cannot represent non-integer value: "foo"",
+        },
+      ],
+    }
+  `);
+});

src/handler.ts

@@ -21,6 +21,7 @@ import {
   areGraphQLErrors,
   isAsyncIterable,
   isExecutionResult,
+  isGraphQLError,
   isObject,
 } from './utils';
 
@@ -681,12 +682,24 @@ export function makeResponse(
     | Readonly<GraphQLError>,
   acceptedMediaType: AcceptableMediaType,
 ): Response {
-  if (isExecutionResult(resultOrErrors)) {
+  const errors = isGraphQLError(resultOrErrors)
+    ? [resultOrErrors]
+    : areGraphQLErrors(resultOrErrors)
+    ? resultOrErrors
+    : null;
+  if (errors) {
     return [
-      JSON.stringify(resultOrErrors),
+      JSON.stringify({ errors }),
       {
-        status: 200,
-        statusText: 'OK',
+        ...(acceptedMediaType === 'application/json'
+          ? {
+              status: 200,
+              statusText: 'OK',
+            }
+          : {
+              status: 400,
+              statusText: 'Bad Request',
+            }),
         headers: {
           'content-type':
             acceptedMediaType === 'application/json'
@@ -698,23 +711,10 @@ export function makeResponse(
   }
 
   return [
-    JSON.stringify({
-      errors: Array.isArray(resultOrErrors)
-        ? isObject(resultOrErrors)
-          ? resultOrErrors
-          : new GraphQLError(String(resultOrErrors))
-        : [resultOrErrors],
-    }),
+    JSON.stringify(resultOrErrors),
     {
-      ...(acceptedMediaType === 'application/json'
-        ? {
-            status: 200,
-            statusText: 'OK',
-          }
-        : {
-            status: 400,
-            statusText: 'Bad Request',
-          }),
+      status: 200,
+      statusText: 'OK',
       headers: {
         'content-type':
           acceptedMediaType === 'application/json'

src/utils.ts

@@ -4,7 +4,8 @@
  *
  */
 
-import type { ExecutionResult, GraphQLError } from 'graphql';
+import type { ExecutionResult } from 'graphql';
+import { GraphQLError } from 'graphql';
 
 /** @private */
 export function extendedTypeof(
@@ -42,13 +43,17 @@ export function isObject(val: unknown): val is Record<
 export function areGraphQLErrors(obj: unknown): obj is readonly GraphQLError[] {
   return (
     Array.isArray(obj) &&
-    // must be at least one error
     obj.length > 0 &&
-    // error has at least a message
-    obj.every((ob) => 'message' in ob)
+    // if one item in the array is a GraphQLError, we're good
+    obj.some(isGraphQLError)
   );
 }
 
+/** @private */
+export function isGraphQLError(obj: unknown): obj is GraphQLError {
+  return obj instanceof GraphQLError;
+}
+
 /** @private */
 export function isExecutionResult(val: unknown): val is ExecutionResult {
   return (