fix(async-hooks): mixing async and callback style in preHandler option now returns an error

lib/route.js

@@ -28,7 +28,8 @@ const {
   FST_ERR_ROUTE_METHOD_NOT_SUPPORTED,
   FST_ERR_ROUTE_METHOD_INVALID,
   FST_ERR_ROUTE_BODY_VALIDATION_SCHEMA_NOT_SUPPORTED,
-  FST_ERR_ROUTE_BODY_LIMIT_OPTION_NOT_INT
+  FST_ERR_ROUTE_BODY_LIMIT_OPTION_NOT_INT,
+  FST_ERR_HOOK_INVALID_ASYNC_HANDLER
 } = require('./errors')
 
 const {
@@ -268,6 +269,20 @@ function buildRouting (options) {
               if (typeof func !== 'function') {
                 throw new FST_ERR_HOOK_INVALID_HANDLER(hook, Object.prototype.toString.call(func))
               }
+
+              if (hook === 'onSend' || hook === 'preSerialization' || hook === 'onError' || hook === 'preParsing') {
+                if (func.constructor.name === 'AsyncFunction' && func.length === 4) {
+                  throw new FST_ERR_HOOK_INVALID_ASYNC_HANDLER()
+                }
+              } else if (hook === 'onRequestAbort') {
+                if (func.constructor.name === 'AsyncFunction' && func.length !== 1) {
+                  throw new FST_ERR_HOOK_INVALID_ASYNC_HANDLER()
+                }
+              } else {
+                if (func.constructor.name === 'AsyncFunction' && func.length === 3) {
+                  throw new FST_ERR_HOOK_INVALID_ASYNC_HANDLER()
+                }
+              }
             }
           } else if (opts[hook] !== undefined && typeof opts[hook] !== 'function') {
             throw new FST_ERR_HOOK_INVALID_HANDLER(hook, Object.prototype.toString.call(opts[hook]))

test/hooks-async.test.js

@@ -745,8 +745,8 @@ test('Should log a warning if is an async function with `done`', t => {
     try {
       fastify.addHook('onRequestAbort', async (req, done) => {})
     } catch (e) {
-      t.ok(e.code, 'FST_ERR_HOOK_INVALID_ASYNC_HANDLER')
-      t.ok(e.message === 'Async function has too many arguments. Async hooks should not use the \'done\' argument.')
+      t.equal(e.code, 'FST_ERR_HOOK_INVALID_ASYNC_HANDLER')
+      t.equal(e.message, 'Async function has too many arguments. Async hooks should not use the \'done\' argument.')
     }
   })
 
@@ -757,8 +757,8 @@ test('Should log a warning if is an async function with `done`', t => {
     try {
       fastify.addHook('onRequest', async (req, reply, done) => {})
     } catch (e) {
-      t.ok(e.code, 'FST_ERR_HOOK_INVALID_ASYNC_HANDLER')
-      t.ok(e.message === 'Async function has too many arguments. Async hooks should not use the \'done\' argument.')
+      t.equal(e.code, 'FST_ERR_HOOK_INVALID_ASYNC_HANDLER')
+      t.equal(e.message, 'Async function has too many arguments. Async hooks should not use the \'done\' argument.')
     }
   })
 
@@ -769,20 +769,20 @@ test('Should log a warning if is an async function with `done`', t => {
     try {
       fastify.addHook('onSend', async (req, reply, payload, done) => {})
     } catch (e) {
-      t.ok(e.code, 'FST_ERR_HOOK_INVALID_ASYNC_HANDLER')
-      t.ok(e.message === 'Async function has too many arguments. Async hooks should not use the \'done\' argument.')
+      t.equal(e.code, 'FST_ERR_HOOK_INVALID_ASYNC_HANDLER')
+      t.equal(e.message, 'Async function has too many arguments. Async hooks should not use the \'done\' argument.')
     }
     try {
       fastify.addHook('preSerialization', async (req, reply, payload, done) => {})
     } catch (e) {
-      t.ok(e.code, 'FST_ERR_HOOK_INVALID_ASYNC_HANDLER')
-      t.ok(e.message === 'Async function has too many arguments. Async hooks should not use the \'done\' argument.')
+      t.equal(e.code, 'FST_ERR_HOOK_INVALID_ASYNC_HANDLER')
+      t.equal(e.message, 'Async function has too many arguments. Async hooks should not use the \'done\' argument.')
     }
     try {
       fastify.addHook('onError', async (req, reply, payload, done) => {})
     } catch (e) {
-      t.ok(e.code, 'FST_ERR_HOOK_INVALID_ASYNC_HANDLER')
-      t.ok(e.message === 'Async function has too many arguments. Async hooks should not use the \'done\' argument.')
+      t.equal(e.code, 'FST_ERR_HOOK_INVALID_ASYNC_HANDLER')
+      t.equal(e.message, 'Async function has too many arguments. Async hooks should not use the \'done\' argument.')
     }
   })
 
@@ -923,3 +923,153 @@ t.test('nested hooks to do not crash on 404', t => {
     t.equal(res.statusCode, 404)
   })
 })
+
+test('Register an hook (preHandler) as route option should fail if mixing async and callback style', t => {
+  t.plan(2)
+  const fastify = Fastify()
+
+  try {
+    fastify.get(
+      '/',
+      {
+        preHandler: [
+          async (request, reply, done) => {
+            done()
+          }
+        ]
+      },
+      async (request, reply) => {
+        return { hello: 'world' }
+      }
+    )
+    t.fail('preHandler mixing async and callback style')
+  } catch (e) {
+    t.equal(e.code, 'FST_ERR_HOOK_INVALID_ASYNC_HANDLER')
+    t.equal(e.message, 'Async function has too many arguments. Async hooks should not use the \'done\' argument.')
+  }
+})
+
+test('Register an hook (onSend) as route option should fail if mixing async and callback style', t => {
+  t.plan(2)
+  const fastify = Fastify()
+
+  try {
+    fastify.get(
+      '/',
+      {
+        onSend: [
+          async (request, reply, payload, done) => {
+            done()
+          }
+        ]
+      },
+      async (request, reply) => {
+        return { hello: 'world' }
+      }
+    )
+    t.fail('onSend mixing async and callback style')
+  } catch (e) {
+    t.equal(e.code, 'FST_ERR_HOOK_INVALID_ASYNC_HANDLER')
+    t.equal(e.message, 'Async function has too many arguments. Async hooks should not use the \'done\' argument.')
+  }
+})
+
+test('Register an hook (preSerialization) as route option should fail if mixing async and callback style', t => {
+  t.plan(2)
+  const fastify = Fastify()
+
+  try {
+    fastify.get(
+      '/',
+      {
+        preSerialization: [
+          async (request, reply, payload, done) => {
+            done()
+          }
+        ]
+      },
+      async (request, reply) => {
+        return { hello: 'world' }
+      }
+    )
+    t.fail('preSerialization mixing async and callback style')
+  } catch (e) {
+    t.equal(e.code, 'FST_ERR_HOOK_INVALID_ASYNC_HANDLER')
+    t.equal(e.message, 'Async function has too many arguments. Async hooks should not use the \'done\' argument.')
+  }
+})
+
+test('Register an hook (onError) as route option should fail if mixing async and callback style', t => {
+  t.plan(2)
+  const fastify = Fastify()
+
+  try {
+    fastify.get(
+      '/',
+      {
+        onError: [
+          async (request, reply, error, done) => {
+            done()
+          }
+        ]
+      },
+      async (request, reply) => {
+        return { hello: 'world' }
+      }
+    )
+    t.fail('onError mixing async and callback style')
+  } catch (e) {
+    t.equal(e.code, 'FST_ERR_HOOK_INVALID_ASYNC_HANDLER')
+    t.equal(e.message, 'Async function has too many arguments. Async hooks should not use the \'done\' argument.')
+  }
+})
+
+test('Register an hook (preParsing) as route option should fail if mixing async and callback style', t => {
+  t.plan(2)
+  const fastify = Fastify()
+
+  try {
+    fastify.get(
+      '/',
+      {
+        preParsing: [
+          async (request, reply, payload, done) => {
+            done()
+          }
+        ]
+      },
+      async (request, reply) => {
+        return { hello: 'world' }
+      }
+    )
+    t.fail('preParsing mixing async and callback style')
+  } catch (e) {
+    t.equal(e.code, 'FST_ERR_HOOK_INVALID_ASYNC_HANDLER')
+    t.equal(e.message, 'Async function has too many arguments. Async hooks should not use the \'done\' argument.')
+  }
+})
+
+test('Register an hook (onRequestAbort) as route option should fail if mixing async and callback style', t => {
+  t.plan(2)
+  const fastify = Fastify()
+
+  try {
+    fastify.get(
+      '/',
+      {
+        onRequestAbort: [
+          async (request, done) => {
+            done()
+          }
+        ]
+      },
+      async (request, reply) => {
+        return { hello: 'world' }
+      }
+    )
+    t.fail('onRequestAbort mixing async and callback style')
+  } catch (e) {
+    t.equal(e.code, 'FST_ERR_HOOK_INVALID_ASYNC_HANDLER')
+    t.equal(e.message, 'Async function has too many arguments. Async hooks should not use the \'done\' argument.')
+  }
+})