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

fix!: prevent req being added twice to the response and request start log #157

Merged
merged 2 commits into from
Mar 16, 2022
Merged

Conversation

AdriVanHoudt
Copy link
Contributor

There is no way to know in the response log event handling if the request was already added through the child bindings in onRequest.
And there was no way as a user to fix this behaviour properly.

So now req will be added once but you need to make sure that you pass it in childBindings for the response.
Request start event will have it regardless but also only once.

This is a breaking change as some people will have removed the req from the childBindings to get around this issue.

Fixes #91

@AdriVanHoudt AdriVanHoudt marked this pull request as ready for review March 14, 2022 16:18
@coveralls
Copy link

coveralls commented Mar 14, 2022

Coverage Status

Coverage increased (+0.02%) to 99.026% when pulling e803882 on AdriVanHoudt:91 into 9a37128 on pinojs:master.

…rt log

There is no way to know in the response log event handling if the request was already added through the child bindings in `onRequest`.
And there was no way as a user to fix this behaviour properly.

So now `req` will be added once but you need to make sure that you pass it in `childBindings` for the response.
Request start event will have it regardless but also only once.

This is a breaking change as some people will have removed the `req` from the `childBindings` to get around this issue.

Fixes #91
@AdriVanHoudt
Copy link
Contributor Author

AdriVanHoudt commented Mar 15, 2022

I added a fix to the commit to make it compatible with the changes of #156

@mcollina
Copy link
Collaborator

There is one more conflict :(

@AdriVanHoudt
Copy link
Contributor Author

Fixed ✨

@mcollina mcollina merged commit 42b3df4 into hapijs:master Mar 16, 2022
@mcollina
Copy link
Collaborator

Thanks Adri!

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

Successfully merging this pull request may close these issues.

Req element twitce with hapi.
3 participants