Merge branch 'master' of github.com:graphql/graphql-js

Author: leebyron

src/execution/__tests__/executor.js

@@ -246,15 +246,18 @@ describe('Execute: Handles basic execution tasks', () => {
 
   it('nulls out error subtrees', async () => {
     var doc = `{
-      sync,
-      syncError,
-      syncRawError,
-      async,
-      asyncReject,
-      asyncRawReject,
-      asyncEmptyReject,
-      asyncError,
+      sync
+      syncError
+      syncRawError
+      syncReturnError
+      syncReturnErrorList
+      async
+      asyncReject
+      asyncRawReject
+      asyncEmptyReject
+      asyncError
       asyncRawError
+      asyncReturnError
     }`;
 
     var data = {
@@ -269,6 +272,17 @@ describe('Execute: Handles basic execution tasks', () => {
         throw 'Error getting syncRawError';
         /* eslint-enable */
       },
+      syncReturnError() {
+        return new Error('Error getting syncReturnError');
+      },
+      syncReturnErrorList() {
+        return [
+          'sync0',
+          new Error('Error getting syncReturnErrorList1'),
+          'sync2',
+          new Error('Error getting syncReturnErrorList3')
+        ];
+      },
       async() {
         return new Promise(resolve => resolve('async'));
       },
@@ -294,7 +308,10 @@ describe('Execute: Handles basic execution tasks', () => {
           throw 'Error getting asyncRawError';
           /* eslint-enable */
         });
-      }
+      },
+      asyncReturnError() {
+        return Promise.resolve(new Error('Error getting asyncReturnError'));
+      },
     };
 
     let ast = parse(doc);
@@ -305,12 +322,15 @@ describe('Execute: Handles basic execution tasks', () => {
           sync: { type: GraphQLString },
           syncError: { type: GraphQLString },
           syncRawError: { type: GraphQLString },
+          syncReturnError: { type: GraphQLString },
+          syncReturnErrorList: { type: new GraphQLList(GraphQLString) },
           async: { type: GraphQLString },
           asyncReject: { type: GraphQLString },
           asyncRawReject: { type: GraphQLString },
           asyncEmptyReject: { type: GraphQLString },
           asyncError: { type: GraphQLString },
           asyncRawError: { type: GraphQLString },
+          asyncReturnError: { type: GraphQLString },
         }
       })
     });
@@ -321,29 +341,40 @@ describe('Execute: Handles basic execution tasks', () => {
       sync: 'sync',
       syncError: null,
       syncRawError: null,
+      syncReturnError: null,
+      syncReturnErrorList: [ 'sync0', null, 'sync2', null ],
       async: 'async',
       asyncReject: null,
       asyncRawReject: null,
       asyncEmptyReject: null,
       asyncError: null,
       asyncRawError: null,
+      asyncReturnError: null,
     });
 
     expect(result.errors && result.errors.map(formatError)).to.deep.equal([
       { message: 'Error getting syncError',
         locations: [ { line: 3, column: 7 } ] },
       { message: 'Error getting syncRawError',
         locations: [ { line: 4, column: 7 } ] },
-      { message: 'Error getting asyncReject',
+      { message: 'Error getting syncReturnError',
+        locations: [ { line: 5, column: 7 } ] },
+      { message: 'Error getting syncReturnErrorList1',
+        locations: [ { line: 6, column: 7 } ] },
+      { message: 'Error getting syncReturnErrorList3',
         locations: [ { line: 6, column: 7 } ] },
+      { message: 'Error getting asyncReturnError',
+        locations: [ { line: 13, column: 7 } ] },
+      { message: 'Error getting asyncReject',
+        locations: [ { line: 8, column: 7 } ] },
       { message: 'Error getting asyncRawReject',
-        locations: [ { line: 7, column: 7 } ] },
+        locations: [ { line: 9, column: 7 } ] },
       { message: 'An unknown error occurred.',
-        locations: [ { line: 8, column: 7 } ] },
+        locations: [ { line: 10, column: 7 } ] },
       { message: 'Error getting asyncError',
-        locations: [ { line: 9, column: 7 } ] },
+        locations: [ { line: 11, column: 7 } ] },
       { message: 'Error getting asyncRawError',
-        locations: [ { line: 10, column: 7 } ] },
+        locations: [ { line: 12, column: 7 } ] },
     ]);
   });
 

src/execution/execute.js

@@ -508,20 +508,9 @@ function resolveField(
     variableValues: exeContext.variableValues,
   };
 
-  // If an error occurs while calling the field `resolve` function, ensure that
-  // it is wrapped as a GraphQLError with locations. Log this error and return
-  // null if allowed, otherwise throw the error so the parent field can handle
-  // it.
-  try {
-    var result = resolveFn(source, args, info);
-  } catch (error) {
-    var reportedError = locatedError(error, fieldASTs);
-    if (returnType instanceof GraphQLNonNull) {
-      throw reportedError;
-    }
-    exeContext.errors.push(reportedError);
-    return null;
-  }
+  // Get the resolve function, regardless of if it's result is normal
+  // or abrupt (error).
+  var result = resolveOrError(resolveFn, source, args, info);
 
   return completeValueCatchingError(
     exeContext,
@@ -532,6 +521,29 @@ function resolveField(
   );
 }
 
+// Isolates the "ReturnOrAbrupt" behavior to not de-opt the `resolveField`
+// function. Returns the result of resolveFn or the abrupt-return Error object.
+function resolveOrError<T>(
+  resolveFn: (
+    source: any,
+    args: { [key: string]: any },
+    info: GraphQLResolveInfo
+  ) => T,
+  source: any,
+  args: { [key: string]: any },
+  info: GraphQLResolveInfo
+): Error | T {
+  try {
+    return resolveFn(source, args, info);
+  } catch (error) {
+    // Sometimes a non-error is thrown, wrap it as an Error for a
+    // consistent interface.
+    return error instanceof Error ? error : new Error(error);
+  }
+}
+
+// This is a small wrapper around completeValue which detects and logs errors
+// in the execution context.
 function completeValueCatchingError(
   exeContext: ExecutionContext,
   returnType: GraphQLType,
@@ -556,6 +568,8 @@ function completeValueCatchingError(
       result
     );
     if (isThenable(completed)) {
+      // If `completeValue` returned a rejected promise, log the rejection
+      // error and resolve to null.
       // Note: we don't rely on a `catch` method, but we do expect "thenable"
       // to take a second callback for the error case.
       return completed.then(undefined, error => {
@@ -565,6 +579,8 @@ function completeValueCatchingError(
     }
     return completed;
   } catch (error) {
+    // If `completeValue` returned abruptly (threw an error), log the error
+    // and return null.
     exeContext.errors.push(error);
     return null;
   }
@@ -595,21 +611,27 @@ function completeValue(
   info: GraphQLResolveInfo,
   result: any
 ): any {
-  // If result is a Promise, resolve it, if the Promise is rejected, construct
-  // a GraphQLError with proper locations.
+  // If result is a Promise, apply-lift over completeValue.
   if (isThenable(result)) {
     return result.then(
+      // Once resolved to a value, complete that value.
       resolved => completeValue(
         exeContext,
         returnType,
         fieldASTs,
         info,
         resolved
       ),
+      // If rejected, create a located error, and continue to reject.
       error => Promise.reject(locatedError(error, fieldASTs))
     );
   }
 
+  // If result is an Error, throw a located error.
+  if (result instanceof Error) {
+    throw locatedError(result, fieldASTs);
+  }
+
   // If field type is NonNull, complete for inner type, and throw field error
   // if result is null.
   if (returnType instanceof GraphQLNonNull) {