Merge pull request #262 from rogchap/error-handling-promise

Handle logging errors if resolve fn is a Promise. fixes #254

src/schemaGenerator.ts

@@ -424,11 +424,8 @@ function decorateWithLogger(fn: GraphQLFieldResolver<any, any> | undefined, logg
     fn = defaultResolveFn;
   }
 
-  return (root, args, ctx, info) => {
-    try {
-      return fn(root, args, ctx, info);
-    } catch (e) {
-      // TODO: clone the error properly
+  const logError = (e: Error) => {
+    // TODO: clone the error properly
       const newE = new Error();
       newE.stack = e.stack;
       /* istanbul ignore else: always get the hint from addErrorLoggingToSchema */
@@ -437,6 +434,25 @@ function decorateWithLogger(fn: GraphQLFieldResolver<any, any> | undefined, logg
         newE['message'] = `Error in resolver ${hint}\n${e.message}`;
       }
       logger.log(newE);
+  };
+
+  return (root, args, ctx, info) => {
+    try {
+      const result = fn(root, args, ctx, info);
+      // If the resolve function returns a Promise log any Promise rejects.
+      if (result && typeof result.then === 'function' && typeof result.catch === 'function') {
+        result.catch((reason: Error | string) => {
+          // make sure that it's an error we're logging.
+          const error = reason instanceof Error ? reason : new Error(reason);
+          logError(error);
+
+          // We don't want to leave an unhandled exception so pass on error.
+          return reason;
+        });
+      }
+      return result;
+    } catch (e) {
+      logError(e);
       // we want to pass on the error, just in case.
       throw e;
     }

src/test/testLogger.ts

@@ -110,4 +110,84 @@ describe('Logger', () => {
       done();
     });
   });
+
+  it('logs any Promise reject errors', (done) => {
+    const shorthand = `
+      type RootQuery {
+        just_a_field: Int
+      }
+      type RootMutation {
+        species(name: String): String
+        stuff: String
+      }
+      schema {
+        query: RootQuery
+        mutation: RootMutation
+      }
+    `;
+    const resolve = {
+      RootMutation: {
+        species: () => {
+          return new Promise((_, reject) => {
+            reject(new Error('oops!'));
+          });
+        },
+        stuff: () => {
+          return new Promise((_, reject) => {
+            reject(new Error('oh noes!'));
+          });
+        },
+      },
+    };
+    const logger = new Logger();
+    const jsSchema = makeExecutableSchema({
+      typeDefs: shorthand,
+      resolvers: resolve,
+      logger,
+    });
+
+    const testQuery = 'mutation { species, stuff }';
+    const expected0 = 'Error in resolver RootMutation.species\noops!';
+    const expected1 = 'Error in resolver RootMutation.stuff\noh noes!';
+    graphql(jsSchema, testQuery).then(() => {
+      assert.equal(logger.errors.length, 2);
+      assert.equal(logger.errors[0].message, expected0);
+      assert.equal(logger.errors[1].message, expected1);
+      done();
+    });
+  });
+
+  it('all Promise rejects will log an Error', (done) => {
+    const shorthand = `
+      type RootQuery {
+        species(name: String): String
+      }
+      schema {
+        query: RootQuery
+      }
+    `;
+    const resolve = {
+      RootQuery: {
+        species: () => {
+          return new Promise((_, reject) => {
+            reject('oops!');
+          });
+        },
+      },
+    };
+
+    let loggedErr: Error = null;
+    const logger = new Logger('LoggyMcLogface', (e: Error) => { loggedErr = e; });
+    const jsSchema = makeExecutableSchema({
+      typeDefs: shorthand,
+      resolvers: resolve,
+      logger,
+    });
+
+    const testQuery = '{ species }';
+    graphql(jsSchema, testQuery).then(() => {
+      assert.equal(loggedErr, logger.errors[0]);
+      done();
+    });
+  });
 });