Handle promises in the error handler with the same logic of normal ha…

…ndlers (#1134)

lib/handleRequest.js

@@ -5,6 +5,7 @@ const urlUtil = require('url')
 const validation = require('./validation')
 const validateSchema = validation.validate
 const runHooks = require('./hookRunner').hookRunner
+const wrapThenable = require('./wrapThenable')
 
 function handleRequest (req, res, params, context) {
   var method = req.method
@@ -90,28 +91,7 @@ function preHandlerCallback (err, reply) {
 
   var result = reply.context.handler(reply.request, reply)
   if (result && typeof result.then === 'function') {
-    result
-      .then(function (payload) {
-        // this is for async functions that
-        // are using reply.send directly
-        if (payload !== undefined || (reply.res.statusCode === 204 && reply.sent === false)) {
-          // we use a try-catch internally to avoid adding a catch to another
-          // promise, increase promise perf by 10%
-          try {
-            reply.send(payload)
-          } catch (err) {
-            reply.sent = false
-            reply._isError = true
-            reply.send(err)
-          }
-        } else if (reply.sent === false) {
-          reply.res.log.error(new Error(`Promise may not be fulfilled with 'undefined' when statusCode is not 204`))
-        }
-      }, function (err) {
-        reply.sent = false
-        reply._isError = true
-        reply.send(err)
-      })
+    wrapThenable(result, reply)
   }
 }
 

lib/reply.js

@@ -9,6 +9,7 @@ const statusCodes = require('http').STATUS_CODES
 const flatstr = require('flatstr')
 const FJS = require('fast-json-stringify')
 const runHooks = require('./hookRunner').onSendHookRunner
+const wrapThenable = require('./wrapThenable')
 
 const serializeError = FJS({
   type: 'object',
@@ -306,9 +307,7 @@ function handleError (reply, error, cb) {
     reply._customError = true
     var result = customErrorHandler(error, reply.request, reply)
     if (result && typeof result.then === 'function') {
-      result
-        .then(payload => reply.send(payload))
-        .catch(err => reply.send(err))
+      wrapThenable(result, reply)
     }
     return
   }

lib/wrapThenable.js

@@ -0,0 +1,27 @@
+'use strict'
+
+function wrapThenable (thenable, reply) {
+  thenable.then(function (payload) {
+    // this is for async functions that
+    // are using reply.send directly
+    if (payload !== undefined || (reply.res.statusCode === 204 && reply.sent === false)) {
+      // we use a try-catch internally to avoid adding a catch to another
+      // promise, increase promise perf by 10%
+      try {
+        reply.send(payload)
+      } catch (err) {
+        reply.sent = false
+        reply._isError = true
+        reply.send(err)
+      }
+    } else if (reply.sent === false) {
+      reply.res.log.error(new Error(`Promise may not be fulfilled with 'undefined' when statusCode is not 204`))
+    }
+  }, function (err) {
+    reply.sent = false
+    reply._isError = true
+    reply.send(err)
+  })
+}
+
+module.exports = wrapThenable

test/async-await.js

@@ -522,6 +522,36 @@ function asyncTest (t) {
       )
     })
   })
+
+  test('customErrorHandler support without throwing', t => {
+    t.plan(4)
+
+    const fastify = Fastify()
+
+    fastify.get('/', async (req, reply) => {
+      const error = new Error('ouch')
+      error.statusCode = 400
+      throw error
+    })
+
+    fastify.setErrorHandler(async (err, req, reply) => {
+      t.is(err.message, 'ouch')
+      reply.code(401).send('kaboom')
+      reply.send = t.fail.bind(t, 'should not be called')
+    })
+
+    fastify.inject({
+      method: 'GET',
+      url: '/'
+    }, (err, res) => {
+      t.error(err)
+      t.strictEqual(res.statusCode, 401)
+      t.deepEqual(
+        'kaboom',
+        res.payload
+      )
+    })
+  })
 }
 
 module.exports = asyncTest