Merge pull request #1747 from andrewthauer/log-errors

Ensure errors are logged in errorHandler

packages/backend-common/src/middleware/errorHandler.test.ts

@@ -96,4 +96,38 @@ describe('errorHandler', () => {
     expect((await r.get('/NotFoundError')).status).toBe(404);
     expect((await r.get('/ConflictError')).status).toBe(409);
   });
+
+  it('logs all 500 errors', async () => {
+    const app = express();
+
+    const mockLogger = { child: jest.fn(), error: jest.fn() };
+    mockLogger.child.mockImplementation(() => mockLogger as any);
+
+    const thrownError = new Error('some error');
+
+    app.use('/breaks', () => {
+      throw thrownError;
+    });
+    app.use(errorHandler({ logger: mockLogger as any }));
+
+    await request(app).get('/breaks');
+
+    expect(mockLogger.error).toHaveBeenCalledWith(thrownError);
+  });
+
+  it('does not log 400 errors', async () => {
+    const app = express();
+
+    const mockLogger = { child: jest.fn(), error: jest.fn() };
+    mockLogger.child.mockImplementation(() => mockLogger as any);
+
+    app.use('/NotFound', () => {
+      throw new errors.NotFoundError();
+    });
+    app.use(errorHandler({ logger: mockLogger as any }));
+
+    await request(app).get('/NotFound');
+
+    expect(mockLogger.error).not.toHaveBeenCalled();
+  });
 });

packages/backend-common/src/middleware/errorHandler.ts

@@ -15,7 +15,9 @@
  */
 
 import { ErrorRequestHandler, NextFunction, Request, Response } from 'express';
+import { Logger } from 'winston';
 import * as errors from '../errors';
+import { getRootLogger } from '../logging';
 
 export type ErrorHandlerOptions = {
   /**
@@ -24,6 +26,13 @@ export type ErrorHandlerOptions = {
    * If not specified, by default shows stack traces only in development mode.
    */
   showStackTraces?: boolean;
+
+  /**
+   * Logger
+   *
+   * If not specified, by default shows stack traces only in development mode.
+   */
+  logger?: Logger;
 };
 
 /**
@@ -39,12 +48,17 @@ export type ErrorHandlerOptions = {
  *
  * @returns An Express error request handler
  */
+
 export function errorHandler(
   options: ErrorHandlerOptions = {},
 ): ErrorRequestHandler {
   const showStackTraces =
     options.showStackTraces ?? process.env.NODE_ENV === 'development';
 
+  const logger = (options.logger || getRootLogger()).child({
+    type: 'errorHandler',
+  });
+
   /* eslint-disable @typescript-eslint/no-unused-vars */
   return (
     error: Error,
@@ -61,6 +75,11 @@ export function errorHandler(
 
     const status = getStatusCode(error);
     const message = showStackTraces ? error.stack : error.message;
+
+    if (logger && status >= 500) {
+      logger.error(error);
+    }
+
     response.status(status).send(message);
   };
 }