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: express mutated request.url #132

Merged

Conversation

10xLaCroixDrinker
Copy link
Contributor

Fixes #11

I tried @Eomm's suggestion, but in this case next is never called, so it does not resolve the issue.

However, this solution does cause onSend to be called where it was not before. Would you consider this a breaking change or is it alright?

Checklist

Fixes #11

Signed-off-by: Jamie King <jamie.king@aexp.com>
Comment on lines +605 to +607
fastify.addHook('onSend', (req, reply, payload, next) => {
t.ok('called')
next(null, payload)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks breaking

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this was a bug? Do we have the original idea behind this functionality saved somewhere?

Copy link
Member

@gurgunday gurgunday left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@gurgunday gurgunday requested review from Eomm and a team April 15, 2024 18:17
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mcollina mcollina merged commit f148658 into fastify:master Apr 16, 2024
19 checks passed
@10xLaCroixDrinker 10xLaCroixDrinker deleted the bugfix/mutated-request-url branch April 17, 2024 01:42
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.

request.url is mutated between onRequest and onResponse
4 participants