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

express support: check req.originalUrl #575

Closed
wants to merge 3 commits into from

Conversation

twelve17
Copy link

@twelve17 twelve17 commented Feb 5, 2018

This is a follow up to a years-old abandoned PR from a different user. Per that PR, I've added a new unit test to test this new feature, as well as made the change backward compatible for non-express usage.

CHANGES.md addition:

Express support: inspect req.originalUrl, and, if present, prefer that over req.url.

Note: I attempted to run make check, to confirm the code style, but ran into this error:

$ make check
./tools/jsstyle -o indent=4,doxygen,unparenthesized-return=0,blank-after-start-comment=0,leading-right-paren-ok=1 lib/bunyan.js test/add-stream.test.js test/buffer.test.js test/child-behaviour.test.js test/cli-client-req.test.js test/cli-res.test.js test/cli.test.js test/ctor.test.js test/cycles.test.js test/dtrace.test.js test/error-event.test.js test/level.test.js test/log-some-loop.js test/log-some.js test/log.test.js test/other-api.test.js test/process-exit.js test/process-exit.test.js test/raw-stream.test.js test/ringbuffer.test.js test/safe-json-stringify-1.js test/safe-json-stringify-2.js test/safe-json-stringify-3.js test/safe-json-stringify-4.js test/safe-json-stringify.test.js test/serializers.test.js test/src.test.js test/stream-levels.test.js test/tap4nodeunit.js tools/timechild.js tools/timeguard.js tools/timenop.js tools/timesrc.js examples/err.js examples/handle-fs-error.js examples/hi.js examples/level.js examples/log-undefined-values.js examples/long-running.js examples/multi.js examples/mute-by-envvars-stream.js examples/raw-stream.js examples/ringbuffer.js examples/rot-specific-levels.js examples/server.js examples/specific-level-streams.js examples/src.js examples/unstringifyable.js bin/bunyan
/bin/sh: json: command not found
version is:
[[ `cat package.json | json version` == `grep '^## ' CHANGES.md | head -2 | tail -1 | awk '{print $2}'` ]]
/bin/sh: json: command not found
make: *** [versioncheck] Error 1

I am on a Mac-- I couldn't find a json utility on home-brew. Is this a custom program? That being said, I ran this part of the command, and it executed without errors:

./tools/jsstyle -o indent=4,doxygen,unparenthesized-return=0,blank-after-start-comment=0,leading-right-paren-ok=1 lib/bunyan.js test/add-stream.test.js test/buffer.test.js test/child-behaviour.test.js test/cli-client-req.test.js test/cli-res.test.js test/cli.test.js test/ctor.test.js test/cycles.test.js test/dtrace.test.js test/error-event.test.js test/level.test.js test/log-some-loop.js test/log-some.js test/log.test.js test/other-api.test.js test/process-exit.js test/process-exit.test.js test/raw-stream.test.js test/ringbuffer.test.js test/safe-json-stringify-1.js test/safe-json-stringify-2.js test/safe-json-stringify-3.js test/safe-json-stringify-4.js test/safe-json-stringify.test.js test/serializers.test.js test/src.test.js test/stream-levels.test.js test/tap4nodeunit.js tools/timechild.js tools/timeguard.js tools/timenop.js tools/timesrc.js examples/err.js examples/handle-fs-error.js examples/hi.js examples/level.js examples/log-undefined-values.js examples/long-running.js examples/multi.js examples/mute-by-envvars-stream.js examples/raw-stream.js examples/ringbuffer.js examples/rot-specific-levels.js examples/server.js examples/specific-level-streams.js examples/src.js examples/unstringifyable.js bin/bunyan

@Mickael-van-der-Beek
Copy link

Can this be merged sometime soon? This has been waiting for 4 year now and it's a relatively small change.

@voxpelli
Copy link

voxpelli commented Jul 3, 2019

@Mickael-van-der-Beek See #586, you're probably better of trying to get a change into an alternative project, like http://getpino.io/

@trentm
Copy link
Owner

trentm commented Jul 8, 2020

I've commited your change as 7641566 (adding a comment and a CHANGES.md entry). Thanks for doing everything right here.

(FYI on the delay in my responding: #335 (comment))

@twelve17
Copy link
Author

twelve17 commented Jul 8, 2020

@trentm No worries. I can understand being overwhelmed having to support a project. The plight of open source work. :)
Thanks for the follow up.

@Srokap
Copy link
Contributor

Srokap commented Jan 7, 2021

FYI it seems that this change did not make into any version yet. Seems to be missing from 2.0.4, not sure if that's intentional or it got missed.

@trentm
Copy link
Owner

trentm commented Jan 8, 2021

@Srokap Ah, yah, sorry: https://github.com/trentm/node-bunyan/blob/82e6f417476c5eb2f0e55b13a965007d1e59329e/CHANGES.md#not-yet-released

I'll get releases out now.

  • bunyan@1.8.15 published
  • bunyan@2.0.5 published

@Srokap
Copy link
Contributor

Srokap commented Jan 9, 2021

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants