execute: Correctly report missing root type error

graphql · Oct 11, 2021 · 0f33b29 · 0f33b29
1 parent 71c7a14
commit 0f33b29
Show file tree

Hide file tree

Showing 2 changed files with 71 additions and 43 deletions.
diff --git a/src/execution/__tests__/executor-test.ts b/src/execution/__tests__/executor-test.ts
@@ -917,18 +917,42 @@ describe('Execute: Handles basic execution tasks', () => {
       subscription S { __typename }
     `);
 
-    // FIXME: errors should be wrapped into ExecutionResult
-    expect(() =>
+    expectJSON(
       executeSync({ schema, document, operationName: 'Q' }),
-    ).to.throw('Schema is not configured to execute query operation.');
+    ).to.deep.equal({
+      data: null,
+      errors: [
+        {
+          message: 'Schema is not configured to execute query operation.',
+          locations: [{ line: 2, column: 7 }],
+        },
+      ],
+    });
 
-    expect(() =>
+    expectJSON(
       executeSync({ schema, document, operationName: 'M' }),
-    ).to.throw('Schema is not configured to execute mutation operation.');
+    ).to.deep.equal({
+      data: null,
+      errors: [
+        {
+          message: 'Schema is not configured to execute mutation operation.',
+          locations: [{ line: 3, column: 7 }],
+        },
+      ],
+    });
 
-    expect(() =>
+    expectJSON(
       executeSync({ schema, document, operationName: 'S' }),
-    ).to.throw('Schema is not configured to execute subscription operation.');
+    ).to.deep.equal({
+      data: null,
+      errors: [
+        {
+          message:
+            'Schema is not configured to execute subscription operation.',
+          locations: [{ line: 4, column: 7 }],
+        },
+      ],
+    });
   });
 
   it('correct field ordering despite execution order', async () => {

diff --git a/src/execution/execute.ts b/src/execution/execute.ts
@@ -185,8 +185,27 @@ export function execute(args: ExecutionArgs): PromiseOrValue<ExecutionResult> {
   // field and its descendants will be omitted, and sibling fields will still
   // be executed. An execution which encounters errors will still result in a
   // resolved Promise.
-  const data = executeOperation(exeContext, exeContext.operation, rootValue);
-  return buildResponse(exeContext, data);
+  //
+  // Errors from sub-fields of a NonNull type may propagate to the top level,
+  // at which point we still log the error and null the parent field, which
+  // in this case is the entire response.
+  try {
+    const { operation } = exeContext;
+    const result = executeOperation(exeContext, operation, rootValue);
+    if (isPromise(result)) {
+      return result.then(
+        (data) => buildResponse(data, exeContext.errors),
+        (error) => {
+          exeContext.errors.push(error);
+          return buildResponse(null, exeContext.errors);
+        },
+      );
+    }
+    return buildResponse(result, exeContext.errors);
+  } catch (error) {
+    exeContext.errors.push(error);
+    return buildResponse(null, exeContext.errors);
+  }
 }
 
 /**
@@ -210,15 +229,10 @@ export function executeSync(args: ExecutionArgs): ExecutionResult {
  * response defined by the "Response" section of the GraphQL specification.
  */
 function buildResponse(
-  exeContext: ExecutionContext,
-  data: PromiseOrValue<ObjMap<unknown> | null>,
-): PromiseOrValue<ExecutionResult> {
-  if (isPromise(data)) {
-    return data.then((resolved) => buildResponse(exeContext, resolved));
-  }
-  return exeContext.errors.length === 0
-    ? { data }
-    : { errors: exeContext.errors, data };
+  data: ObjMap<unknown> | null,
+  errors: ReadonlyArray<GraphQLError>,
+): ExecutionResult {
+  return errors.length === 0 ? { data } : { errors, data };
 }
 
 /**
@@ -349,33 +363,23 @@ function executeOperation(
     rootType,
     operation.selectionSet,
   );
-
   const path = undefined;
 
-  // Errors from sub-fields of a NonNull type may propagate to the top level,
-  // at which point we still log the error and null the parent field, which
-  // in this case is the entire response.
-  try {
-    const result =
-      operation.operation === 'mutation'
-        ? executeFieldsSerially(
-            exeContext,
-            rootType,
-            rootValue,
-            path,
-            rootFields,
-          )
-        : executeFields(exeContext, rootType, rootValue, path, rootFields);
-    if (isPromise(result)) {
-      return result.then(undefined, (error) => {
-        exeContext.errors.push(error);
-        return null;
-      });
-    }
-    return result;
-  } catch (error) {
-    exeContext.errors.push(error);
-    return null;
+  switch (operation.operation) {
+    case 'query':
+      return executeFields(exeContext, rootType, rootValue, path, rootFields);
+    case 'mutation':
+      return executeFieldsSerially(
+        exeContext,
+        rootType,
+        rootValue,
+        path,
+        rootFields,
+      );
+    case 'subscription':
+      // TODO: deprecate `subscribe` and move all logic here
+      // Temporary solution until we finish merging execute and subscribe together
+      return executeFields(exeContext, rootType, rootValue, path, rootFields);
   }
 }