feat(serverError): add additional logging for server errors (#282)

* feat(serverError): add additional logging for server errors

__tests__/server/middleware/serverError.spec.js

@@ -0,0 +1,31 @@
+/*
+ * Copyright 2020 American Express Travel Related Services Company, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing
+ * permissions and limitations under the License.
+ */
+
+import serverError from '../../../src/server/middleware/serverError';
+
+// existing coverage in ssrServer.spec.js
+
+jest.spyOn(console, 'error').mockImplementation(() => {});
+const next = jest.fn();
+describe('serverError', () => {
+  it('handles req with no headers however unlikely', () => {
+    const reqWithNoHeaders = {};
+    const res = { status: jest.fn(), send: jest.fn() };
+
+    const callServerError = () => serverError('some error', reqWithNoHeaders, res, next);
+    expect(callServerError).not.toThrowError();
+  });
+});

__tests__/server/ssrServer.spec.js

@@ -573,7 +573,7 @@ describe('ssrServer', () => {
 
           expect(res.status).toEqual(500);
           expect(console.error).toHaveBeenCalledTimes(1);
-          expect(console.error).toHaveBeenCalledWith('express application error', middlewareError);
+          expect(console.error).toHaveBeenCalledWith(middlewareError, 'express application error: method GET, url "/anything", correlationId "undefined", headersSent: false');
           expect(res.type).toEqual('text/html');
           expect(res.text).toMatch('<h2 style="display: flex; justify-content: center; padding: 40px 15px 0px;">Loading Error</h2>');
           expect(res.text).toMatch('Sorry, we are unable to load this page at this time.');
@@ -620,7 +620,7 @@ describe('ssrServer', () => {
 
           expect(res.status).toEqual(201);
           expect(console.error).toHaveBeenCalledTimes(1);
-          expect(console.error).toHaveBeenCalledWith('express application error', middlewareError);
+          expect(console.error).toHaveBeenCalledWith(middlewareError, 'express application error: method GET, url "/anything", correlationId "undefined", headersSent: true');
           expect(res.text).toMatch('hello');
           return done();
         });

src/server/middleware/serverError.js

@@ -0,0 +1,44 @@
+/*
+ * Copyright 2020 American Express Travel Related Services Company, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing
+ * permissions and limitations under the License.
+ */
+
+// https://expressjs.com/en/guide/error-handling.html
+import { renderStaticErrorPage } from './sendHtml';
+
+// eslint-disable-next-line max-params
+const serverError = (err, req, res, next) => {
+  const { method, url } = req;
+  const correlationId = req.headers && req.headers['correlation-id'];
+
+  const headersSent = !!res.headersSent;
+
+  console.error(err, `express application error: method ${method}, url "${url}", correlationId "${correlationId}", headersSent: ${headersSent}`);
+
+  if (headersSent) {
+    // don't try changing the headers at this point
+    return next(err);
+  }
+
+  if (err.name === 'URIError') {
+    // invalid URL given to express, unable to parse
+    res.status(400);
+  } else {
+    res.status(500);
+  }
+
+  return renderStaticErrorPage(res);
+};
+
+module.exports = serverError;

src/server/ssrServer.js

@@ -41,7 +41,8 @@ import createRequestStore from './middleware/createRequestStore';
 import createRequestHtmlFragment from './middleware/createRequestHtmlFragment';
 import checkStateForRedirect from './middleware/checkStateForRedirect';
 import checkStateForStatusCode from './middleware/checkStateForStatusCode';
-import sendHtml, { renderStaticErrorPage } from './middleware/sendHtml';
+import sendHtml from './middleware/sendHtml';
+import serverError from './middleware/serverError';
 import logging from './utils/logging/serverMiddleware';
 import forwardedHeaderParser from './middleware/forwardedHeaderParser';
 import {
@@ -113,24 +114,8 @@ export function createApp({ enablePostToModuleRoutes = false } = {}) {
   }
 
   // https://expressjs.com/en/guide/error-handling.html
-  // eslint-disable-next-line max-params
-  app.use((err, req, res, next) => {
-    console.error('express application error', err);
 
-    if (res.headersSent) {
-      // don't try changing the headers at this point
-      return next(err);
-    }
-
-    if (err.name === 'URIError') {
-      // invalid URL given to express, unable to parse
-      res.status(400);
-    } else {
-      res.status(500);
-    }
-
-    return renderStaticErrorPage(res);
-  });
+  app.use(serverError);
 
   return app;
 }