Error middleware improvements (#18169)

* Defer logging to Failbot in error middleware until AFTER we finish responding

* If headers have already been written, log to failbot and then delegate to the default error handler

http://expressjs.com/en/guide/error-handling.html#the-default-error-handler

* Comment tweak

Author: JamesMGreene

middleware/handle-errors.js

@@ -17,7 +17,24 @@ function shouldLogException (error) {
   return true
 }
 
+async function logException (error, req) {
+  if (process.env.NODE_ENV !== 'test' && shouldLogException(error)) {
+    await FailBot.report(error, {
+      path: req.path
+    })
+  }
+}
+
 module.exports = async function handleError (error, req, res, next) {
+  // If the headers have already been sent...
+  if (res.headersSent) {
+    // Report to Failbot
+    await logException(error, req)
+
+    // We MUST delegate to the default Express error handler
+    return next(error)
+  }
+
   // if the error is thrown before req.context is created (say, in the Page class),
   // set req.context.site here so we can pass data/ui.yml text to the 500 layout
   if (!req.context) {
@@ -46,13 +63,10 @@ module.exports = async function handleError (error, req, res, next) {
   if (process.env.NODE_ENV !== 'test') {
     console.error('500 error!', req.path)
     console.error(error)
-
-    if (shouldLogException(error)) {
-      await FailBot.report(error, {
-        path: req.path
-      })
-    }
   }
 
   res.status(500).send(await liquid.parseAndRender(layouts['error-500'], req.context))
+
+  // Report to Failbot AFTER responding to the user
+  await logException(error, req)
 }