Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: update 404 onBadUrl behavior #3813

Merged
merged 1 commit into from Apr 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
33 changes: 16 additions & 17 deletions lib/fourOhFour.js
Expand Up @@ -30,17 +30,20 @@ const fourOhFourContext = {
* kFourOhFourContext: the context in the reply object where the handler will be executed
*/
function fourOhFour (options) {
const { logger, genReqId, disableRequestLogging } = options
const { logger, genReqId } = options

// 404 router, used for handling encapsulated 404 handlers
const router = FindMyWay({ onBadUrl, defaultRoute: fourOhFourFallBack })
const router = FindMyWay({ onBadUrl: createOnBadUrl(), defaultRoute: fourOhFourFallBack })
let _onBadUrlHandler = null

return { router, setNotFoundHandler, setContext, arrange404 }

function arrange404 (instance) {
// Change the pointer of the fastify instance to itself, so register + prefix can add new 404 handler
instance[kFourOhFourLevelInstance] = instance
instance[kCanSetNotFoundHandler] = true
// we need to bind instance for the context
router.onBadUrl = router.onBadUrl.bind(instance)
}

function basic404 (request, reply) {
Expand All @@ -54,24 +57,16 @@ function fourOhFour (options) {
})
}

function onBadUrl (path, req, res) {
const { url, method } = req
const message = `Route ${method}:${url} not found`
const body = `{"error":"Not Found","message":"${message}","statusCode":404}`

// simulate normal route logging
if (!disableRequestLogging) {
function createOnBadUrl () {
return function onBadUrl (path, req, res) {
const id = genReqId(req)
const childLogger = logger.child({ reqId: id })
childLogger.info({ req }, 'incoming request')
childLogger.info({ req }, message)
}
const fourOhFourContext = this[kFourOhFourLevelInstance][kFourOhFourContext]
const request = new Request(id, null, req, null, childLogger, fourOhFourContext)
const reply = new Reply(res, request, childLogger)

res.writeHead(404, {
'Content-Type': 'application/json',
'Content-Length': body.length
})
res.end(body)
_onBadUrlHandler(request, reply)
}
}

function setContext (instance, context) {
Expand Down Expand Up @@ -123,8 +118,12 @@ function fourOhFour (options) {
if (handler) {
this[kFourOhFourLevelInstance][kCanSetNotFoundHandler] = false
handler = handler.bind(this)
// update onBadUrl handler
_onBadUrlHandler = handler
} else {
handler = basic404
// update onBadUrl handler
_onBadUrlHandler = basic404
}

this.after((notHandledErr, done) => {
Expand Down
16 changes: 16 additions & 0 deletions test/404s.test.js
Expand Up @@ -1872,6 +1872,22 @@ test('400 in case of bad url (pre find-my-way v2.2.0 was a 404)', t => {
})
})

t.test('customized 404', t => {
t.plan(3)
const fastify = Fastify({ logger: true })
fastify.setNotFoundHandler(function (req, reply) {
reply.code(404).send('this was not found')
})
fastify.inject({
url: '/%c0',
method: 'GET'
}, (err, response) => {
t.error(err)
t.equal(response.statusCode, 404)
t.same(response.payload, 'this was not found')
})
})

t.end()
})

Expand Down
3 changes: 2 additions & 1 deletion test/logger.test.js
Expand Up @@ -1517,7 +1517,8 @@ test('should not log incoming request and outgoing response for 404 onBadUrl whe
url: '/%c0',
method: 'GET'
}, (e, res) => {
t.same(lines.length, 0)
// it will log 1 line only because of basic404
t.same(lines.length, 1)
})
})

Expand Down