From 2fc3c9092bda40adee552ff2f8d5518fc9fba4a4 Mon Sep 17 00:00:00 2001
From: Dan Ribbens <dan.ribbens@gmail.com>
Date: Thu, 7 May 2020 12:22:11 -0400
Subject: [PATCH 1/6] add common error handler middleware and adopts it in a
 few places

---
 demo/collections/Post/index.js            |  4 +-
 demo/payload.config.js                    |  4 ++
 demo/policies/checkRole.js                |  5 +-
 src/collections/graphql/resolvers/find.js |  2 +-
 src/collections/requestHandlers/create.js |  5 +-
 src/collections/requestHandlers/find.js   |  9 ++-
 src/errors/errorHandler.js                | 26 ++++++++
 src/errors/errorHandler.spec.js           | 77 +++++++++++++++++++++++
 src/errors/index.js                       |  2 +
 src/index.js                              |  3 +
 10 files changed, 127 insertions(+), 10 deletions(-)
 create mode 100644 src/errors/errorHandler.js
 create mode 100644 src/errors/errorHandler.spec.js

diff --git a/demo/collections/Post/index.js b/demo/collections/Post/index.js
index 871eb22f1f..705629cd18 100644
--- a/demo/collections/Post/index.js
+++ b/demo/collections/Post/index.js
@@ -24,7 +24,9 @@ module.exports = {
   },
   hooks: {
     beforeCreate: operation => operation,
-    beforeRead: operation => operation,
+    beforeRead: operation => {
+      throw new Error('user defined error');
+    },
     beforeUpdate: operation => operation,
     beforeDelete: (operation) => {
       console.log(`About to delete ${operation.id}`);
diff --git a/demo/payload.config.js b/demo/payload.config.js
index 9f8be4f726..526cf2a2db 100644
--- a/demo/payload.config.js
+++ b/demo/payload.config.js
@@ -87,4 +87,8 @@ module.exports = {
       // Sidebar: path.resolve(__dirname, 'client/components/layout/Sidebar/index.js'),
     },
   },
+  // TODO: change to normal function
+  errorHandler(err) {
+    console.error('USER CONFIG DEFINED HANDLER', err.stack);
+  },
 };
diff --git a/demo/policies/checkRole.js b/demo/policies/checkRole.js
index 8092aac172..84fffa9194 100644
--- a/demo/policies/checkRole.js
+++ b/demo/policies/checkRole.js
@@ -1,12 +1,11 @@
 /**
  * authorize a request by comparing the current user with one or more roles
- * @param roles
+ * @param allRoles
  * @param user
  * @returns {Function}
  */
 const checkRole = (allRoles, user) => {
-  const hasPermission = !!(user && allRoles.some(role => user.roles.some(individualRole => individualRole === role)));
-  return hasPermission;
+  return !!(user && allRoles.some(role => user.roles.some(individualRole => individualRole === role)));
 };
 
 module.exports = checkRole;
diff --git a/src/collections/graphql/resolvers/find.js b/src/collections/graphql/resolvers/find.js
index 675bba1084..281ddb66b1 100644
--- a/src/collections/graphql/resolvers/find.js
+++ b/src/collections/graphql/resolvers/find.js
@@ -17,7 +17,7 @@ const findResolver = collection => async (_, args, context) => {
   };
 
   const results = await find(options);
-
+  // TODO: research proper way to error handle
   return results;
 };
 
diff --git a/src/collections/requestHandlers/create.js b/src/collections/requestHandlers/create.js
index 7fdda8846e..bdfec91cdc 100644
--- a/src/collections/requestHandlers/create.js
+++ b/src/collections/requestHandlers/create.js
@@ -3,7 +3,7 @@ const formatErrorResponse = require('../../express/responses/formatError');
 const formatSuccessResponse = require('../../express/responses/formatSuccess');
 const { create } = require('../operations');
 
-const createHandler = async (req, res) => {
+const createHandler = async (req, res, next) => {
   try {
     const doc = await create({
       req,
@@ -17,7 +17,8 @@ const createHandler = async (req, res) => {
       doc,
     });
   } catch (error) {
-    return res.status(error.status || httpStatus.INTERNAL_SERVER_ERROR).json(formatErrorResponse(error));
+    return next(error);
+    // return res.status(error.status || httpStatus.INTERNAL_SERVER_ERROR).json(formatErrorResponse(error));
   }
 };
 
diff --git a/src/collections/requestHandlers/find.js b/src/collections/requestHandlers/find.js
index 395f401432..9b9bdc2394 100644
--- a/src/collections/requestHandlers/find.js
+++ b/src/collections/requestHandlers/find.js
@@ -1,7 +1,7 @@
 const httpStatus = require('http-status');
 const { find } = require('../operations');
 
-const findHandler = async (req, res) => {
+const findHandler = async (req, res, next) => {
   try {
     const options = {
       req,
@@ -16,9 +16,12 @@ const findHandler = async (req, res) => {
 
     const result = await find(options);
 
+    // throw new Error('testing error handler');
+
     return res.status(httpStatus.OK).json(result);
-  } catch (err) {
-    return res.status(400).json(err);
+  } catch (error) {
+    return next(error);
+    // return res.status(400).json(err);
   }
 };
 
diff --git a/src/errors/errorHandler.js b/src/errors/errorHandler.js
new file mode 100644
index 0000000000..d0c2590f61
--- /dev/null
+++ b/src/errors/errorHandler.js
@@ -0,0 +1,26 @@
+const httpStatus = require('http-status');
+
+const errorHandler = config => (err, req, res, next) => {
+  let response;
+  if (config.errorHandler) {
+    response = config.errorHandler(err);
+  } else {
+    const data = {};
+
+    // TODO: use payload logging
+    console.error(err.stack);
+
+    res.status(err.status || httpStatus.INTERNAL_SERVER_ERROR);
+
+    if (config.debug && config.debug === true) {
+      data.stack = err.stack;
+    }
+    response = {
+      ...data,
+      error: err.message,
+    };
+  }
+  res.send(response);
+};
+
+module.exports = errorHandler;
diff --git a/src/errors/errorHandler.spec.js b/src/errors/errorHandler.spec.js
new file mode 100644
index 0000000000..102380ee2e
--- /dev/null
+++ b/src/errors/errorHandler.spec.js
@@ -0,0 +1,77 @@
+const errorHandler = require('./errorHandler');
+const APIError = require('./APIError');
+
+const testError = new APIError('test error', 503);
+
+const mockResponse = () => {
+  const res = {};
+  res.status = jest.fn()
+    .mockReturnValue(res);
+  res.send = jest.fn()
+    .mockReturnValue(res);
+  return res;
+};
+
+describe('errorHandler', () => {
+  let res;
+  const req = {};
+  beforeAll(async (done) => {
+    res = mockResponse();
+    done();
+  });
+
+  it('should send the response with the error', () => {
+    const handler = errorHandler({ debug: true });
+    handler(testError, req, res);
+    expect(res.send)
+      .toHaveBeenCalledWith(
+        expect.objectContaining({ error: 'test error' }),
+      );
+  });
+
+  it('should include stack trace when config debug is on', () => {
+    const handler = errorHandler({ debug: true });
+    handler(testError, req, res);
+    expect(res.send)
+      .toHaveBeenCalledWith(
+        expect.objectContaining({ stack: expect.any(String) }),
+      );
+  });
+
+  it('should not include stack trace when config debug is not set', () => {
+    const handler = errorHandler({});
+    handler(testError, req, res);
+    expect(res.send)
+      .toHaveBeenCalledWith(
+        expect.not.objectContaining({ stack: expect.any(String) }),
+      );
+  });
+
+  it('should not include stack trace when config debug is false', () => {
+    const handler = errorHandler({ debug: false });
+    handler(testError, req, res);
+    expect(res.send)
+      .toHaveBeenCalledWith(
+        expect.not.objectContaining({ stack: expect.any(String) }),
+      );
+  });
+
+  it('should show the status code when given an error with a code', () => {
+    const handler = errorHandler({ debug: false });
+    handler(testError, req, res);
+    expect(res.status)
+      .toHaveBeenCalledWith(
+        503,
+      );
+  });
+
+  it('should default to 500 when an error does not have a status code', () => {
+    const handler = errorHandler({ debug: false });
+    testError.status = undefined;
+    handler(testError, req, res);
+    expect(res.status)
+      .toHaveBeenCalledWith(
+        500,
+      );
+  });
+});
diff --git a/src/errors/index.js b/src/errors/index.js
index b66f8adb05..608b1e7e6b 100644
--- a/src/errors/index.js
+++ b/src/errors/index.js
@@ -7,9 +7,11 @@ const MissingUseAsTitle = require('./MissingUseAsTitle');
 const NotFound = require('./NotFound');
 const Forbidden = require('./Forbidden');
 const ValidationError = require('./ValidationError');
+const errorHandler = require('./errorHandler');
 const MissingFile = require('./MissingFile');
 
 module.exports = {
+  errorHandler,
   APIError,
   DuplicateCollection,
   DuplicateGlobal,
diff --git a/src/index.js b/src/index.js
index bbf1924226..7d7314d932 100644
--- a/src/index.js
+++ b/src/index.js
@@ -16,6 +16,7 @@ const GraphQL = require('./graphql');
 const sanitizeConfig = require('./utilities/sanitizeConfig');
 const buildEmail = require('./email/build');
 const identifyAPI = require('./express/middleware/identifyAPI');
+const errorHandler = require('./errors/errorHandler');
 
 class Payload {
   constructor(options) {
@@ -69,6 +70,8 @@ class Payload {
       },
     }));
 
+    this.router.use('', errorHandler(this.config));
+
     // Bind router to API
     this.express.use(this.config.routes.api, this.router);
 

From b1d6bd9e0c9d62a356f3aa477810a4b3f97e85f6 Mon Sep 17 00:00:00 2001
From: Dan Ribbens <dan.ribbens@gmail.com>
Date: Fri, 8 May 2020 17:42:51 -0400
Subject: [PATCH 2/6] moves error handling to express middleware, changes
 pattern to hook

---
 demo/collections/Post/index.js                |  4 +--
 src/collections/requestHandlers/create.js     |  3 +--
 src/collections/requestHandlers/find.js       |  3 ---
 src/errors/errorHandler.js                    | 26 ------------------
 src/errors/index.js                           |  2 +-
 src/express/middleware/errorHandler.js        | 27 +++++++++++++++++++
 .../middleware}/errorHandler.spec.js          |  2 +-
 src/express/responses/formatError.js          |  2 ++
 src/index.js                                  |  8 ++++--
 src/utilities/sanitizeConfig.js               |  1 +
 10 files changed, 40 insertions(+), 38 deletions(-)
 delete mode 100644 src/errors/errorHandler.js
 create mode 100644 src/express/middleware/errorHandler.js
 rename src/{errors => express/middleware}/errorHandler.spec.js (97%)

diff --git a/demo/collections/Post/index.js b/demo/collections/Post/index.js
index 705629cd18..871eb22f1f 100644
--- a/demo/collections/Post/index.js
+++ b/demo/collections/Post/index.js
@@ -24,9 +24,7 @@ module.exports = {
   },
   hooks: {
     beforeCreate: operation => operation,
-    beforeRead: operation => {
-      throw new Error('user defined error');
-    },
+    beforeRead: operation => operation,
     beforeUpdate: operation => operation,
     beforeDelete: (operation) => {
       console.log(`About to delete ${operation.id}`);
diff --git a/src/collections/requestHandlers/create.js b/src/collections/requestHandlers/create.js
index bdfec91cdc..9979575ae6 100644
--- a/src/collections/requestHandlers/create.js
+++ b/src/collections/requestHandlers/create.js
@@ -1,5 +1,5 @@
 const httpStatus = require('http-status');
-const formatErrorResponse = require('../../express/responses/formatError');
+// const formatErrorResponse = require('../../express/responses/formatError');
 const formatSuccessResponse = require('../../express/responses/formatSuccess');
 const { create } = require('../operations');
 
@@ -18,7 +18,6 @@ const createHandler = async (req, res, next) => {
     });
   } catch (error) {
     return next(error);
-    // return res.status(error.status || httpStatus.INTERNAL_SERVER_ERROR).json(formatErrorResponse(error));
   }
 };
 
diff --git a/src/collections/requestHandlers/find.js b/src/collections/requestHandlers/find.js
index 9b9bdc2394..a0a178c513 100644
--- a/src/collections/requestHandlers/find.js
+++ b/src/collections/requestHandlers/find.js
@@ -16,12 +16,9 @@ const findHandler = async (req, res, next) => {
 
     const result = await find(options);
 
-    // throw new Error('testing error handler');
-
     return res.status(httpStatus.OK).json(result);
   } catch (error) {
     return next(error);
-    // return res.status(400).json(err);
   }
 };
 
diff --git a/src/errors/errorHandler.js b/src/errors/errorHandler.js
deleted file mode 100644
index d0c2590f61..0000000000
--- a/src/errors/errorHandler.js
+++ /dev/null
@@ -1,26 +0,0 @@
-const httpStatus = require('http-status');
-
-const errorHandler = config => (err, req, res, next) => {
-  let response;
-  if (config.errorHandler) {
-    response = config.errorHandler(err);
-  } else {
-    const data = {};
-
-    // TODO: use payload logging
-    console.error(err.stack);
-
-    res.status(err.status || httpStatus.INTERNAL_SERVER_ERROR);
-
-    if (config.debug && config.debug === true) {
-      data.stack = err.stack;
-    }
-    response = {
-      ...data,
-      error: err.message,
-    };
-  }
-  res.send(response);
-};
-
-module.exports = errorHandler;
diff --git a/src/errors/index.js b/src/errors/index.js
index 608b1e7e6b..bd65a857d6 100644
--- a/src/errors/index.js
+++ b/src/errors/index.js
@@ -7,7 +7,7 @@ const MissingUseAsTitle = require('./MissingUseAsTitle');
 const NotFound = require('./NotFound');
 const Forbidden = require('./Forbidden');
 const ValidationError = require('./ValidationError');
-const errorHandler = require('./errorHandler');
+const errorHandler = require('../express/middleware/errorHandler');
 const MissingFile = require('./MissingFile');
 
 module.exports = {
diff --git a/src/express/middleware/errorHandler.js b/src/express/middleware/errorHandler.js
new file mode 100644
index 0000000000..80ad76beae
--- /dev/null
+++ b/src/express/middleware/errorHandler.js
@@ -0,0 +1,27 @@
+const httpStatus = require('http-status');
+const formatErrorResponse = require('../responses/formatError');
+
+const errorHandler = config => async (err, req, res, next) => {
+  let response;
+  const data = formatErrorResponse(err);
+  let status = err.status || httpStatus.INTERNAL_SERVER_ERROR;
+
+  // TODO: use payload logging
+  console.error(err.stack);
+
+  if (config.debug && config.debug === true) {
+    data.stack = err.stack;
+  }
+  response = {
+    ...data,
+  };
+
+  if (typeof config.hooks.afterError === 'function') {
+    ({ response, status } = await config.hooks.afterError(err, response) || { response, status });
+  }
+
+  res.status(status)
+    .send(response);
+};
+
+module.exports = errorHandler;
diff --git a/src/errors/errorHandler.spec.js b/src/express/middleware/errorHandler.spec.js
similarity index 97%
rename from src/errors/errorHandler.spec.js
rename to src/express/middleware/errorHandler.spec.js
index 102380ee2e..db7f399228 100644
--- a/src/errors/errorHandler.spec.js
+++ b/src/express/middleware/errorHandler.spec.js
@@ -1,5 +1,5 @@
 const errorHandler = require('./errorHandler');
-const APIError = require('./APIError');
+const APIError = require('../../errors/APIError');
 
 const testError = new APIError('test error', 503);
 
diff --git a/src/express/responses/formatError.js b/src/express/responses/formatError.js
index b8a9b63dd3..7ecb0d992a 100644
--- a/src/express/responses/formatError.js
+++ b/src/express/responses/formatError.js
@@ -1,9 +1,11 @@
 const formatErrorResponse = (incoming) => {
   if (incoming) {
+    // mongoose
     if (incoming.errors) {
       return {
         errors: Object.keys(incoming.errors).reduce((acc, key) => {
           acc.push({
+            field: incoming.errors[key].path,
             message: incoming.errors[key].message,
           });
           return acc;
diff --git a/src/index.js b/src/index.js
index 7d7314d932..14c5c4d82d 100644
--- a/src/index.js
+++ b/src/index.js
@@ -16,7 +16,7 @@ const GraphQL = require('./graphql');
 const sanitizeConfig = require('./utilities/sanitizeConfig');
 const buildEmail = require('./email/build');
 const identifyAPI = require('./express/middleware/identifyAPI');
-const errorHandler = require('./errors/errorHandler');
+const errorHandler = require('./express/middleware/errorHandler');
 
 class Payload {
   constructor(options) {
@@ -70,13 +70,17 @@ class Payload {
       },
     }));
 
-    this.router.use('', errorHandler(this.config));
+    // this.router.use('', (err, req, res) => {
+    //   console.log('wtf');
+    // });
 
     // Bind router to API
     this.express.use(this.config.routes.api, this.router);
 
     // Enable static routes for all collections permitting upload
     this.initStatic();
+
+    this.router.use(errorHandler(this.config));
   }
 
   async sendEmail(message) {
diff --git a/src/utilities/sanitizeConfig.js b/src/utilities/sanitizeConfig.js
index 3c26dd1497..0823fea60c 100644
--- a/src/utilities/sanitizeConfig.js
+++ b/src/utilities/sanitizeConfig.js
@@ -8,6 +8,7 @@ const sanitizeConfig = (config) => {
     graphQLPlayground: (config.routes && config.routes.graphQLPlayground) ? config.routes.graphQLPlayground : '/graphql-playground',
   };
   sanitizedConfig.components = { ...(config.components || {}) };
+  sanitizedConfig.hooks = { ...(config.hooks || {}) };
 
   return sanitizedConfig;
 };

From d998ea35fe5024c79be6df43af00d9f9effb2778 Mon Sep 17 00:00:00 2001
From: Dan Ribbens <dan.ribbens@gmail.com>
Date: Sun, 10 May 2020 21:05:48 -0400
Subject: [PATCH 3/6] add test coverage for error handler hooks

---
 demo/payload.config.js                      |  7 +-
 src/collections/requestHandlers/delete.js   |  6 +-
 src/collections/requestHandlers/findByID.js |  8 +--
 src/collections/requestHandlers/update.js   |  5 +-
 src/express/middleware/errorHandler.js      |  3 +
 src/express/middleware/errorHandler.spec.js | 73 +++++++++++++++------
 6 files changed, 68 insertions(+), 34 deletions(-)

diff --git a/demo/payload.config.js b/demo/payload.config.js
index 526cf2a2db..da2b8aa677 100644
--- a/demo/payload.config.js
+++ b/demo/payload.config.js
@@ -87,8 +87,9 @@ module.exports = {
       // Sidebar: path.resolve(__dirname, 'client/components/layout/Sidebar/index.js'),
     },
   },
-  // TODO: change to normal function
-  errorHandler(err) {
-    console.error('USER CONFIG DEFINED HANDLER', err.stack);
+  hooks: {
+    afterError: (err, response) => {
+      console.error('global error config handler');
+    },
   },
 };
diff --git a/src/collections/requestHandlers/delete.js b/src/collections/requestHandlers/delete.js
index d440c11451..3fc93522eb 100644
--- a/src/collections/requestHandlers/delete.js
+++ b/src/collections/requestHandlers/delete.js
@@ -2,7 +2,7 @@ const httpStatus = require('http-status');
 const { NotFound } = require('../../errors');
 const { deleteQuery } = require('../operations');
 
-const deleteHandler = async (req, res) => {
+const deleteHandler = async (req, res, next) => {
   try {
     const doc = await deleteQuery({
       req,
@@ -16,8 +16,8 @@ const deleteHandler = async (req, res) => {
     }
 
     return res.status(httpStatus.OK).send(doc);
-  } catch (err) {
-    return res.status(httpStatus.INTERNAL_SERVER_ERROR).json(err);
+  } catch (error) {
+    return next(error);
   }
 };
 
diff --git a/src/collections/requestHandlers/findByID.js b/src/collections/requestHandlers/findByID.js
index 421e58093e..3b6894c86d 100644
--- a/src/collections/requestHandlers/findByID.js
+++ b/src/collections/requestHandlers/findByID.js
@@ -1,8 +1,6 @@
-const httpStatus = require('http-status');
 const { findByID } = require('../operations');
-const formatErrorResponse = require('../../express/responses/formatError');
 
-const findByIDHandler = async (req, res) => {
+const findByIDHandler = async (req, res, next) => {
   const options = {
     req,
     Model: req.collection.Model,
@@ -14,8 +12,8 @@ const findByIDHandler = async (req, res) => {
   try {
     const doc = await findByID(options);
     return res.json(doc);
-  } catch (err) {
-    return res.status(err.status || httpStatus.INTERNAL_SERVER_ERROR).json(formatErrorResponse(err));
+  } catch (error) {
+    return next(error);
   }
 };
 
diff --git a/src/collections/requestHandlers/update.js b/src/collections/requestHandlers/update.js
index 33384651ef..c0bcd1b841 100644
--- a/src/collections/requestHandlers/update.js
+++ b/src/collections/requestHandlers/update.js
@@ -1,9 +1,8 @@
 const httpStatus = require('http-status');
-const formatErrorResponse = require('../../express/responses/formatError');
 const formatSuccessResponse = require('../../express/responses/formatSuccess');
 const { update } = require('../operations');
 
-const updateHandler = async (req, res) => {
+const updateHandler = async (req, res, next) => {
   try {
     const doc = await update({
       req,
@@ -18,7 +17,7 @@ const updateHandler = async (req, res) => {
       doc,
     });
   } catch (error) {
-    return res.status(error.status || httpStatus.INTERNAL_SERVER_ERROR).json(formatErrorResponse(error));
+    return next(error);
   }
 };
 
diff --git a/src/express/middleware/errorHandler.js b/src/express/middleware/errorHandler.js
index 80ad76beae..00e205c45f 100644
--- a/src/express/middleware/errorHandler.js
+++ b/src/express/middleware/errorHandler.js
@@ -16,6 +16,9 @@ const errorHandler = config => async (err, req, res, next) => {
     ...data,
   };
 
+  if (req.collection && typeof req.collection.config.hooks.afterError === 'function') {
+    ({ response, status } = await req.collection.config.hooks.afterError(err, response) || { response, status });
+  }
   if (typeof config.hooks.afterError === 'function') {
     ({ response, status } = await config.hooks.afterError(err, response) || { response, status });
   }
diff --git a/src/express/middleware/errorHandler.spec.js b/src/express/middleware/errorHandler.spec.js
index db7f399228..b044891e85 100644
--- a/src/express/middleware/errorHandler.spec.js
+++ b/src/express/middleware/errorHandler.spec.js
@@ -12,66 +12,99 @@ const mockResponse = () => {
   return res;
 };
 
+const mockRequest = async () => {
+  const req = {};
+  req.collection = {
+    config: {
+      hooks: {},
+    },
+  };
+  req.collection.config.hooks.afterError = await jest.fn();
+  return req;
+};
+
 describe('errorHandler', () => {
   let res;
-  const req = {};
+  let req;
   beforeAll(async (done) => {
     res = mockResponse();
+    req = await mockRequest();
     done();
   });
 
-  it('should send the response with the error', () => {
-    const handler = errorHandler({ debug: true });
-    handler(testError, req, res);
+  it('should send the response with the error', async () => {
+    const handler = errorHandler({ debug: true, hooks: {}});
+    await handler(testError, req, res);
     expect(res.send)
       .toHaveBeenCalledWith(
-        expect.objectContaining({ error: 'test error' }),
+        expect.objectContaining({ errors: [{ message: 'test error' }] }),
       );
   });
 
-  it('should include stack trace when config debug is on', () => {
-    const handler = errorHandler({ debug: true });
-    handler(testError, req, res);
+  it('should include stack trace when config debug is on', async () => {
+    const handler = errorHandler({ debug: true, hooks: {} });
+    await handler(testError, req, res);
     expect(res.send)
       .toHaveBeenCalledWith(
         expect.objectContaining({ stack: expect.any(String) }),
       );
   });
 
-  it('should not include stack trace when config debug is not set', () => {
-    const handler = errorHandler({});
-    handler(testError, req, res);
+  it('should not include stack trace when config debug is not set', async () => {
+    const handler = errorHandler({hooks: {}});
+    await handler(testError, req, res);
     expect(res.send)
       .toHaveBeenCalledWith(
         expect.not.objectContaining({ stack: expect.any(String) }),
       );
   });
 
-  it('should not include stack trace when config debug is false', () => {
-    const handler = errorHandler({ debug: false });
-    handler(testError, req, res);
+  it('should not include stack trace when config debug is false', async () => {
+    const handler = errorHandler({ debug: false, hooks: {} });
+    await handler(testError, req, res);
     expect(res.send)
       .toHaveBeenCalledWith(
         expect.not.objectContaining({ stack: expect.any(String) }),
       );
   });
 
-  it('should show the status code when given an error with a code', () => {
-    const handler = errorHandler({ debug: false });
-    handler(testError, req, res);
+  it('should show the status code when given an error with a code', async () => {
+    const handler = errorHandler({ debug: false, hooks: {} });
+    await handler(testError, req, res);
     expect(res.status)
       .toHaveBeenCalledWith(
         503,
       );
   });
 
-  it('should default to 500 when an error does not have a status code', () => {
-    const handler = errorHandler({ debug: false });
+  it('should default to 500 when an error does not have a status code', async () => {
+    const handler = errorHandler({ debug: false, hooks: {} });
     testError.status = undefined;
-    handler(testError, req, res);
+    await handler(testError, req, res);
     expect(res.status)
       .toHaveBeenCalledWith(
         500,
       );
   });
+
+  it('should call payload config afterError hook', async () => {
+    const afterError = jest.fn();
+    const handler = errorHandler({
+      debug: false,
+      hooks: { afterError },
+    });
+    await handler(testError, req, res);
+    expect(afterError)
+      .toHaveBeenCalled();
+  });
+
+  it('should call collection config afterError hook', async () => {
+    const handler = errorHandler({
+      debug: false,
+      hooks: {},
+    });
+    await handler(testError, req, res);
+    expect(req.collection.config.hooks.afterError)
+      .toHaveBeenCalled();
+  });
 });

From 49d58662a1240739f78f1fcede59b30590d3158c Mon Sep 17 00:00:00 2001
From: Dan Ribbens <dan.ribbens@gmail.com>
Date: Sat, 16 May 2020 08:26:07 -0400
Subject: [PATCH 4/6] wip on graphql after error hook

---
 src/collections/graphql/resolvers/find.js |  4 +-
 src/express/middleware/errorHandler.js    |  2 +-
 src/graphql/errorHandler.js               | 28 ++++++++++++++
 src/graphql/index.js                      | 46 +++++++++++++++++++++--
 4 files changed, 74 insertions(+), 6 deletions(-)
 create mode 100644 src/graphql/errorHandler.js

diff --git a/src/collections/graphql/resolvers/find.js b/src/collections/graphql/resolvers/find.js
index 281ddb66b1..2c028de803 100644
--- a/src/collections/graphql/resolvers/find.js
+++ b/src/collections/graphql/resolvers/find.js
@@ -16,8 +16,10 @@ const findResolver = collection => async (_, args, context) => {
     req: context,
   };
 
+  // TODO: remove faked error
+  // throw new Error('blow up for science');
+
   const results = await find(options);
-  // TODO: research proper way to error handle
   return results;
 };
 
diff --git a/src/express/middleware/errorHandler.js b/src/express/middleware/errorHandler.js
index 00e205c45f..d86c2b7253 100644
--- a/src/express/middleware/errorHandler.js
+++ b/src/express/middleware/errorHandler.js
@@ -2,8 +2,8 @@ const httpStatus = require('http-status');
 const formatErrorResponse = require('../responses/formatError');
 
 const errorHandler = config => async (err, req, res, next) => {
-  let response;
   const data = formatErrorResponse(err);
+  let response;
   let status = err.status || httpStatus.INTERNAL_SERVER_ERROR;
 
   // TODO: use payload logging
diff --git a/src/graphql/errorHandler.js b/src/graphql/errorHandler.js
new file mode 100644
index 0000000000..496b14f19b
--- /dev/null
+++ b/src/graphql/errorHandler.js
@@ -0,0 +1,28 @@
+const httpStatus = require('http-status');
+
+const errorHandler = async (err, info, debug, afterErrorHook) => {
+  let { status } = info.context.statusCode || httpStatus.INTERNAL_SERVER_ERROR;
+  let response = {
+    ...err,
+  };
+
+  // TODO: use payload logging
+  console.error(err.stack);
+
+  if (afterErrorHook) {
+    ({ response, status } = await afterErrorHook(err, response) || { response, status });
+  }
+
+  if (debug && debug === true) {
+    response.stack = err.stack;
+  }
+
+  // TODO: how to handle collection level hooks for graphQL?
+  // if (req.collection && typeof req.collection.config.hooks.afterError === 'function') {
+  //   ({ response, status } = await req.collection.config.hooks.afterError(err, response) || { response, status });
+  // }
+
+  return response;
+};
+
+module.exports = errorHandler;
diff --git a/src/graphql/index.js b/src/graphql/index.js
index d75e8f3056..60d2bd6352 100644
--- a/src/graphql/index.js
+++ b/src/graphql/index.js
@@ -10,6 +10,7 @@ const registerCollections = require('../collections/graphql/register');
 const registerGlobals = require('../globals/graphql/register');
 const initUser = require('../users/graphql/init');
 const buildWhereInputType = require('./schema/buildWhereInputType');
+const errorHandler = require('./errorHandler');
 
 class GraphQL {
   constructor(init) {
@@ -23,8 +24,14 @@ class GraphQL {
       fallbackLocaleInputType: buildFallbackLocaleInputType(this.config.localization),
     };
 
-    this.Query = { name: 'Query', fields: {} };
-    this.Mutation = { name: 'Mutation', fields: {} };
+    this.Query = {
+      name: 'Query',
+      fields: {},
+    };
+    this.Mutation = {
+      name: 'Mutation',
+      fields: {},
+    };
 
     this.buildBlockType = buildBlockType.bind(this);
     this.buildMutationInputType = buildMutationInputType.bind(this);
@@ -42,9 +49,40 @@ class GraphQL {
 
     const query = new GraphQLObjectType(this.Query);
     const mutation = new GraphQLObjectType(this.Mutation);
-    const schema = new GraphQLSchema({ query, mutation });
+    const schema = new GraphQLSchema({
+      query,
+      mutation,
+    });
 
-    return graphQLHTTP({ schema });
+    let errorExtension;
+    let errorExtensionIteration = 0;
+
+    const extensions = async (info) => {
+      const { result } = info;
+      if (result.errors) {
+        const afterErrorHook = typeof this.config.hooks.afterError === 'function' ? this.config.hooks.afterError : null;
+
+        errorExtension = await Promise.all(result.errors.map(async (error) => {
+          return errorHandler(error, info, this.config.debug, afterErrorHook);
+        }));
+      }
+      return null;
+    };
+
+    return graphQLHTTP({
+      schema,
+      customFormatErrorFn: (err) => {
+        const response = {
+          // message: err.message,
+          // locations: err.locations,
+          // path: err.path,
+          ...errorExtension[errorExtensionIteration],
+        };
+        errorExtensionIteration += 1;
+        return response;
+      },
+      extensions,
+    });
   }
 }
 

From 1739241cb3c2f1761e6f66bdd4fe10d144ddc635 Mon Sep 17 00:00:00 2001
From: Dan Ribbens <dan.ribbens@gmail.com>
Date: Tue, 19 May 2020 21:43:33 -0400
Subject: [PATCH 5/6] cleanup error handler for graphql

---
 src/graphql/errorHandler.js | 36 +++++++++++++++---------------------
 src/graphql/index.js        | 14 ++++----------
 src/index.js                |  4 ----
 3 files changed, 19 insertions(+), 35 deletions(-)

diff --git a/src/graphql/errorHandler.js b/src/graphql/errorHandler.js
index 496b14f19b..1ee2624520 100644
--- a/src/graphql/errorHandler.js
+++ b/src/graphql/errorHandler.js
@@ -1,28 +1,22 @@
-const httpStatus = require('http-status');
+const errorHandler = async (info, debug, afterErrorHook) => {
+  return Promise.all(info.result.errors.map(async (err) => {
+    // TODO: use payload logging
+    console.error(err.stack);
 
-const errorHandler = async (err, info, debug, afterErrorHook) => {
-  let { status } = info.context.statusCode || httpStatus.INTERNAL_SERVER_ERROR;
-  let response = {
-    ...err,
-  };
+    let response = {
+      ...err,
+    };
 
-  // TODO: use payload logging
-  console.error(err.stack);
+    if (afterErrorHook) {
+      ({ response } = await afterErrorHook(err, response) || { response });
+    }
 
-  if (afterErrorHook) {
-    ({ response, status } = await afterErrorHook(err, response) || { response, status });
-  }
+    if (debug && debug === true) {
+      response.stack = err.stack;
+    }
 
-  if (debug && debug === true) {
-    response.stack = err.stack;
-  }
-
-  // TODO: how to handle collection level hooks for graphQL?
-  // if (req.collection && typeof req.collection.config.hooks.afterError === 'function') {
-  //   ({ response, status } = await req.collection.config.hooks.afterError(err, response) || { response, status });
-  // }
-
-  return response;
+    return response;
+  }));
 };
 
 module.exports = errorHandler;
diff --git a/src/graphql/index.js b/src/graphql/index.js
index 60d2bd6352..14a55bfc6a 100644
--- a/src/graphql/index.js
+++ b/src/graphql/index.js
@@ -54,29 +54,23 @@ class GraphQL {
       mutation,
     });
 
-    let errorExtension;
+    let errorExtensions = [];
     let errorExtensionIteration = 0;
 
     const extensions = async (info) => {
       const { result } = info;
       if (result.errors) {
         const afterErrorHook = typeof this.config.hooks.afterError === 'function' ? this.config.hooks.afterError : null;
-
-        errorExtension = await Promise.all(result.errors.map(async (error) => {
-          return errorHandler(error, info, this.config.debug, afterErrorHook);
-        }));
+        errorExtensions = await errorHandler(info, this.config.debug, afterErrorHook);
       }
       return null;
     };
 
     return graphQLHTTP({
       schema,
-      customFormatErrorFn: (err) => {
+      customFormatErrorFn: () => {
         const response = {
-          // message: err.message,
-          // locations: err.locations,
-          // path: err.path,
-          ...errorExtension[errorExtensionIteration],
+          ...errorExtensions[errorExtensionIteration],
         };
         errorExtensionIteration += 1;
         return response;
diff --git a/src/index.js b/src/index.js
index 14c5c4d82d..78b19f8d2a 100644
--- a/src/index.js
+++ b/src/index.js
@@ -70,10 +70,6 @@ class Payload {
       },
     }));
 
-    // this.router.use('', (err, req, res) => {
-    //   console.log('wtf');
-    // });
-
     // Bind router to API
     this.express.use(this.config.routes.api, this.router);
 

From 3b04ed613d758fca45743061dfb613797cffc81c Mon Sep 17 00:00:00 2001
From: Dan Ribbens <dan.ribbens@gmail.com>
Date: Tue, 19 May 2020 21:48:38 -0400
Subject: [PATCH 6/6] cleanup commented lines

---
 src/collections/graphql/resolvers/find.js | 3 ---
 src/collections/requestHandlers/create.js | 1 -
 2 files changed, 4 deletions(-)

diff --git a/src/collections/graphql/resolvers/find.js b/src/collections/graphql/resolvers/find.js
index 2c028de803..6e3cf77a88 100644
--- a/src/collections/graphql/resolvers/find.js
+++ b/src/collections/graphql/resolvers/find.js
@@ -16,9 +16,6 @@ const findResolver = collection => async (_, args, context) => {
     req: context,
   };
 
-  // TODO: remove faked error
-  // throw new Error('blow up for science');
-
   const results = await find(options);
   return results;
 };
diff --git a/src/collections/requestHandlers/create.js b/src/collections/requestHandlers/create.js
index 9979575ae6..5d3599cdf8 100644
--- a/src/collections/requestHandlers/create.js
+++ b/src/collections/requestHandlers/create.js
@@ -1,5 +1,4 @@
 const httpStatus = require('http-status');
-// const formatErrorResponse = require('../../express/responses/formatError');
 const formatSuccessResponse = require('../../express/responses/formatSuccess');
 const { create } = require('../operations');