Skip to content

Commit

Permalink
Fix restify (#88) by only using req.path if a string (#89)
Browse files Browse the repository at this point in the history
* call req.path() if a function. restify compat, fixes #88

in #29 we preferred req.path, because req.url would throw in hapi if the path was invalid. in restify though, req.path is a function, and this change made the url no longer log. check req.path to see if it is a function, and invoke it if it is.

* add test for calling req.path() if it is a function.

* only get url from req.path if it's a string. fixes #88.

restify has a req.path but it's a function that returns an object. rather than try to determine how to handle path if we have it, only use path if it's a simple string. otherwise, keep using req.url, like we used to pre #29.

* guard against req.url being falsy

it's tempting to simply stop looking at req.url.path altogether, since that was introduced for hapi 17 in 83d58a7#diff-2c4782cb20f7f0d7ca90c7bc04939f4420c4bdb42fdce618984da393ba9202f7R60 .

but since #29 we look at req.path first, which is safer in hapi, so hapi doesn't care about req.url.path anymore.

but perhaps someone else does depend on req.url.path . so for now, continuing to look at req.url.path , which means a bit more guarding in the req serializer.
  • Loading branch information
rektide authored Feb 23, 2022
1 parent ed85319 commit 2b9d5f5
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 2 deletions.
5 changes: 3 additions & 2 deletions lib/req.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,9 @@ function reqSerializer (req) {
_req.query = req.query
_req.params = req.params
} else {
// req.url.path is for hapi compat.
_req.url = req.path || (req.url ? (req.url.path || req.url) : undefined)
const path = req.path
// path for safe hapi compat.
_req.url = typeof path === 'string' ? path : (req.url ? req.url.path || req.url : undefined)
}
_req.headers = req.headers
_req.remoteAddress = connection && connection.remoteAddress
Expand Down
41 changes: 41 additions & 0 deletions test/req.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,47 @@ test('req.url will be obtained from input request originalUrl when available', f
}
})

test('req.url will be obtained from input request url when req path is a function', function (t) {
t.plan(1)

const server = http.createServer(handler)
server.unref()
server.listen(0, () => {
http.get(server.address(), () => {})
})

t.teardown(() => server.close())

function handler (req, res) {
req.path = function () {
throw new Error('unexpected invocation')
}
req.url = '/test'
const serialized = serializers.reqSerializer(req)
t.equal(serialized.url, '/test')
res.end()
}
})

test('req.url being undefined does not throw an error', function (t) {
t.plan(1)

const server = http.createServer(handler)
server.unref()
server.listen(0, () => {
http.get(server.address(), () => {})
})

t.teardown(() => server.close())

function handler (req, res) {
req.url = undefined
const serialized = serializers.reqSerializer(req)
t.equal(serialized.url, undefined)
res.end()
}
})

test('can wrap request serializers', function (t) {
t.plan(3)

Expand Down

0 comments on commit 2b9d5f5

Please # to comment.