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

Improve response filter documentation #36995

Merged
merged 1 commit into from
Nov 10, 2023
Merged

Improve response filter documentation #36995

merged 1 commit into from
Nov 10, 2023

Conversation

geoand
Copy link
Contributor

@geoand geoand commented Nov 10, 2023

Close: #36955

@famod
Copy link
Member

famod commented Nov 10, 2023

Ah, interesting - confirmed with small reproducer; I only need a matching @ServerExceptionMapper which doesn't need to set the header though!
The irony is that for some reason I tried it via the quickstart right away instead of in my app - and there I already have matching exception handlers. 🤦‍♂️

WDYT about making it more explicit by adding:

Such a reponse filter will also be called for <<exception-mapping, handled>> exceptions.

just above

Your filters may declare any of the following parameter types:

My reasoning here is that you don't need to add the Throwable parameter and your filter will still be called.

@geoand
Copy link
Contributor Author

geoand commented Nov 10, 2023

I all for making it more clear for readers, but I am not sure I understand how you would like it to look.
If you want to push a change into my branch, that's fine with me :)

Copy link

github-actions bot commented Nov 10, 2023

🙈 The PR is closed and the preview is expired.

@famod
Copy link
Member

famod commented Nov 10, 2023

I've pushed what I had in mind.

Quarkus Documentation automation moved this from To do to Reviewer approved Nov 10, 2023
@geoand
Copy link
Contributor Author

geoand commented Nov 10, 2023

🙏🏼

@geoand geoand added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Nov 10, 2023
@famod
Copy link
Member

famod commented Nov 10, 2023

I've just realized that even a Jakarta-way ContainerResponseFilter is called for mapped exceptions. I'll add a note there too.

@famod famod merged commit 8948da3 into quarkusio:main Nov 10, 2023
5 checks passed
Quarkus Documentation automation moved this from Reviewer approved to Done Nov 10, 2023
@quarkus-bot quarkus-bot bot added this to the 3.6 - main milestone Nov 10, 2023
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Nov 10, 2023
@gsmet gsmet modified the milestones: 3.6 - main, 3.5.2 Nov 13, 2023
@aloubyansky aloubyansky modified the milestones: 3.5.2, 3.2.9.Final Nov 14, 2023
@geoand geoand deleted the #36955 branch February 27, 2024 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

@ServerResponseFilter with Throwable parameter is not called when REST resource is throwing an exception
4 participants