Skip to content

Commit

Permalink
Run onSend hooks when an encapsulated handler invokes the notFound ha…
Browse files Browse the repository at this point in the history
…ndler (#870)

* Run onSend hooks when an encapsulated handler invokes the notFound handler

* Build the right 404 context during boot

* Use capital 'F' in 'Not Found'
  • Loading branch information
nwoltman authored and delvedor committed Apr 3, 2018
1 parent b010287 commit cc2f9c9
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 9 deletions.
8 changes: 5 additions & 3 deletions fastify.js
Expand Up @@ -551,7 +551,9 @@ function build (options) {

// Must store the 404 Context in 'preReady' because it is only guaranteed to
// be available after all of the plugins and routes have been loaded.
context._404Context = _fastify._404Context
const _404Context = Object.assign({}, _fastify._404Context)
_404Context.onSend = context.onSend
context._404Context = _404Context
})

done(notHandledErr)
Expand Down Expand Up @@ -686,7 +688,7 @@ function build (options) {
}

function basic404 (req, reply) {
reply.code(404).send(new Error('Not found'))
reply.code(404).send(new Error('Not Found'))
}

function fourOhFourFallBack (req, res) {
Expand All @@ -708,7 +710,7 @@ function build (options) {
req.log.warn(fourOhFour.prettyPrint())
const request = new Request(null, req, null, req.headers, req.log)
const reply = new Reply(res, { onSend: [] }, request)
reply.code(404).send(new Error('Not found'))
reply.code(404).send(new Error('Not Found'))
}

function setNotFoundHandler (opts, handler) {
Expand Down
30 changes: 27 additions & 3 deletions test/404s.test.js
Expand Up @@ -919,7 +919,7 @@ test('log debug for 404', t => {

const INFO_LEVEL = 30
t.strictEqual(JSON.parse(logStream.logs[0]).msg, 'incoming request')
t.strictEqual(JSON.parse(logStream.logs[1]).msg, 'Not found')
t.strictEqual(JSON.parse(logStream.logs[1]).msg, 'Not Found')
t.strictEqual(JSON.parse(logStream.logs[1]).level, INFO_LEVEL)
t.strictEqual(JSON.parse(logStream.logs[2]).msg, 'request completed')
t.strictEqual(logStream.logs.length, 3)
Expand Down Expand Up @@ -953,7 +953,7 @@ test('recognizes errors from the http-errors module', t => {
const obj = JSON.parse(body.toString())
t.strictDeepEqual(obj, {
error: 'Not Found',
message: 'Not found',
message: 'Not Found',
statusCode: 404
})
})
Expand All @@ -979,7 +979,7 @@ test('the default 404 handler can be invoked inside a prefixed plugin', t => {
t.strictEqual(res.statusCode, 404)
t.strictDeepEqual(JSON.parse(res.payload), {
error: 'Not Found',
message: 'Not found',
message: 'Not Found',
statusCode: 404
})
})
Expand Down Expand Up @@ -1163,3 +1163,27 @@ test('Not found on unsupported method (should return a 404)', t => {
})
})
})

// https://github.com/fastify/fastify/issues/868
test('onSend hooks run when an encapsulated route invokes the notFound handler', t => {
t.plan(3)
const fastify = Fastify()

fastify.register((instance, options, done) => {
instance.addHook('onSend', (request, reply, payload, next) => {
t.pass('onSend hook called')
next()
})

instance.get('/', (request, reply) => {
reply.send(new errors.NotFound())
})

done()
})

fastify.inject('/', (err, res) => {
t.error(err)
t.strictEqual(res.statusCode, 404)
})
})
2 changes: 1 addition & 1 deletion test/internals/logger.test.js
Expand Up @@ -91,7 +91,7 @@ test('The logger should add a timestamp if logging to stdout', t => {
stream.once('data', (line) => {
t.equal(line.msg, 'incoming request')
stream.once('data', (line) => {
t.equal(line.msg, 'Not found')
t.equal(line.msg, 'Not Found')
stream.once('data', (line) => {
t.equal(line.msg, 'request completed')
t.ok(line.responseTime, 'responseTime exists')
Expand Down
2 changes: 1 addition & 1 deletion test/internals/reply.test.js
Expand Up @@ -491,7 +491,7 @@ test('reply.send(new NotFound()) should invoke the 404 handler', t => {
t.deepEqual(JSON.parse(body.toString()), {
statusCode: 404,
error: 'Not Found',
message: 'Not found'
message: 'Not Found'
})
})

Expand Down
2 changes: 1 addition & 1 deletion test/logger.test.js
Expand Up @@ -640,7 +640,7 @@ test('Should set a custom log level for a specific route', t => {
test('The default 404 handler logs the incoming request', t => {
t.plan(5)

const expectedMessages = ['incoming request', 'Not found', 'request completed']
const expectedMessages = ['incoming request', 'Not Found', 'request completed']

const splitStream = split(JSON.parse)
splitStream.on('data', (line) => {
Expand Down

0 comments on commit cc2f9c9

Please sign in to comment.