Restify req.url broken by #29 #88
Comments
|
Would you like to send a Pull Request to address this issue? Remember to add unit tests. |
|
Working on it, sure. I plan on testing |
rektide
added a commit
to rektide/pino-std-serializers
that referenced
this issue
Feb 22, 2022
in pinojs#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.
rektide
added a commit
to rektide/pino-std-serializers
that referenced
this issue
Feb 23, 2022
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 pinojs#29.
mcollina
pushed a commit
that referenced
this issue
Feb 23, 2022
* 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.
Mechanicalkeyboard
pushed a commit
to Mechanicalkeyboard/pino
that referenced
this issue
Apr 19, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Hi, apologies,
In #29,
req.pathwas made preferred toreq.url, for finding the url. In restify however req.path is a fucntion (this link is to current master branch). This causes the url not to be logged for restify systems.The text was updated successfully, but these errors were encountered: