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

Allow disabling default ResponseExceptionMapper per REST client #44813

Closed
neon-dev opened this issue Nov 28, 2024 · 5 comments · Fixed by #44816
Closed

Allow disabling default ResponseExceptionMapper per REST client #44813

neon-dev opened this issue Nov 28, 2024 · 5 comments · Fixed by #44816
Labels
Milestone

Comments

@neon-dev
Copy link
Contributor

neon-dev commented Nov 28, 2024

Description

microprofile.rest.client.disable.default.mapper=false works on a global level.
It would be useful to have a configuration option that works on a per client basis.
In addition, an annotation could be introduced as a counterpart to the programmatic way.

A new section in the REST cllient docs about it would also be helpful.

Implementation ideas

An annotation could also work at the method level to solve #23957 without introducing unwanted side effects with other methods.

@neon-dev neon-dev added the kind/enhancement New feature or request label Nov 28, 2024
Copy link

quarkus-bot bot commented Nov 28, 2024

/cc @cescoffier (rest-client), @geoand (rest-client)

@geoand
Copy link
Contributor

geoand commented Nov 28, 2024

If I have time, I'll look into getting this into 3.18 as it is clearly useful

geoand added a commit to geoand/quarkus that referenced this issue Nov 28, 2024
The default mapper has a very confusing behavior where
it throws an exception HTTP code > 400 even if the
method return type is Response.
This PR means to provide a way so users can disable this mapper
per client instead of globally.

Closes: quarkusio#44813
geoand added a commit to geoand/quarkus that referenced this issue Nov 28, 2024
The default mapper has a very confusing behavior where
it throws an exception HTTP code >= 400 even if the
method return type is Response.
This PR means to provide a way so users can disable this mapper
per client instead of globally.

Closes: quarkusio#44813
geoand added a commit to geoand/quarkus that referenced this issue Nov 28, 2024
The default mapper has a very confusing behavior where
it throws an exception HTTP code >= 400 even if the
method return type is Response.
This PR means to provide a way so users can disable this mapper
per client instead of globally.

Closes: quarkusio#44813
geoand added a commit to geoand/quarkus that referenced this issue Nov 29, 2024
The default mapper has a very confusing behavior where
it throws an exception HTTP code >= 400 even if the
method return type is Response.
This PR means to provide a way so users can disable this mapper
per client instead of globally.

Closes: quarkusio#44813
geoand added a commit to geoand/quarkus that referenced this issue Nov 29, 2024
The default mapper has a very confusing behavior where
it throws an exception HTTP code >= 400 even if the
method return type is Response.
This PR means to provide a way so users can disable this mapper
per client instead of globally.

Closes: quarkusio#44813
@geoand
Copy link
Contributor

geoand commented Nov 29, 2024

#44816 adds this functionality

@chonton
Copy link
Contributor

chonton commented Nov 30, 2024

@geoand geoand closed this as completed in 6ca76ec Dec 2, 2024
geoand added a commit that referenced this issue Dec 2, 2024
Introduce property to disable default mapper per REST Client
@quarkus-bot quarkus-bot bot added this to the 3.18 - main milestone Dec 2, 2024
@geoand
Copy link
Contributor

geoand commented Dec 2, 2024

That's nice, thanks!

alex-pumpkin pushed a commit to alex-pumpkin/quarkus that referenced this issue Dec 2, 2024
The default mapper has a very confusing behavior where
it throws an exception HTTP code >= 400 even if the
method return type is Response.
This PR means to provide a way so users can disable this mapper
per client instead of globally.

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

Successfully merging a pull request may close this issue.

3 participants