Skip to content

Commit

Permalink
fix(async-hooks): mixing async and callback style (#5069)
Browse files Browse the repository at this point in the history
  • Loading branch information
@giuliowaitforitdavide
giuliowaitforitdavide committed Oct 3, 2023
1 parent 7d97dcd commit 16859bb
Show file tree
Hide file tree
Showing 2 changed files with 176 additions and 11 deletions.
17 changes: 16 additions & 1 deletion lib/route.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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]))
Expand Down
170 changes: 160 additions & 10 deletions test/hooks-async.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.')
}
})

Expand All @@ -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.')
}
})

Expand All @@ -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.')
}
})

Expand Down Expand Up @@ -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.')
}
})

0 comments on commit 16859bb

Please sign in to comment.