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

Preserve response headers when redirecting application error to gateway error pages #136

Merged
merged 1 commit into from
Jul 26, 2024

Conversation

groldan
Copy link
Member

@groldan groldan commented Jul 20, 2024

Commit 37ff94b make the ApplicationError Gateway filter lose the original response headers when throwing a ResponseStatusException for the Gateway to show up the customized HTML error pages instead of the orignal (usually whitelabel) errors.

This patch makes it so that the ApplicationError filter runs only when text/html is accepted by the request, and the request method is idempotent (e.g. GET, HEAD, etc.).

Additionally, the original response headers are not lost, since the exception is thrown at ServerHttpResponseDecorator.beforeCommit(), and respecting the reactive chain.


Fixes #128

…ay error pages

Commit 37ff94b make the `ApplicationError` Gateway filter lose the
original response headers when throwing a `ResponseStatusException`
for the Gateway to show up the customized HTML error pages instead of
the orignal (usually whitelabel) errors.

This patch makes it so that the `ApplicationError` filter runs
only when `text/html` is accepted by the request, and the request
method is idempotent (e.g. GET, HEAD, etc.).

Additionally, the original response headers are not lost, since the
exception is thrown at `ServerHttpResponseDecorator.beforeCommit()`, and
respecting the reactive chain.
@groldan groldan requested a review from edevosc2c July 20, 2024 19:52
Copy link
Member

@edevosc2c edevosc2c left a comment

Choose a reason for hiding this comment

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

Thank you @groldan.
I just deployed this PR in dev at https://geodata.dev.inrae.fr and this works great!
I'm now using the all default-filters and all the headers are correctly passed on the error pages.
For me it's good to be merged. I didn't review the code, just validated that it fixes the bug.

@groldan groldan merged commit a7c497f into main Jul 26, 2024
3 checks passed
@groldan groldan deleted the bug/applicationerror_filter_removes_response_headers branch July 26, 2024 12:25
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.

Commit 37ff94b9 modify the response by the application
2 participants