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

All decoders follow rule: if a status is 404 it returns empty or null value #1597

Merged
merged 2 commits into from
Mar 19, 2022

Conversation

vitalijr2
Copy link
Collaborator

Now some decoders follow this rule but other don't.

After 11.6 the flag decode404 doesn't work as it is described in the documentation. In new implementation a response body will be ignored, not decoded. I propose to replace decode404 with dismiss404 following the comment #1529 (comment)

Fixes #1595

@vitalijr2
Copy link
Collaborator Author

Should I update the changelog?
This change is important but the changelog hasn't updated since 10.9.

@velo
Copy link
Member

velo commented Mar 19, 2022

Should I update the changelog? This change is important but the changelog hasn't updated since 10.9.

Since we started doing GitHub releases, we pretty much stop updating the changelog

May be needs a final entry saying so

@velo velo merged commit 04a85e6 into OpenFeign:master Mar 19, 2022
@vitalijr2 vitalijr2 deleted the dismiss404-returns-null-or-empty branch March 20, 2022 04:09
@ugrepel
Copy link

ugrepel commented Mar 21, 2022

Cool, thanks for fixing this. I've got a question however: is there any reason why the decoders distinguish between 204/404 decoding to (if defined) an empty version of the required type (e.g. an empty List), and, with other status codes, a null body, decoding to null?

@vitalijr2
Copy link
Collaborator Author

vitalijr2 commented Mar 22, 2022

Good note.

I suppose that we should use emptyValueOf for any cases when a response body is null or cannot be decoded: 204, 205, 404 (optional) and just body is null with any other HTTP status code.

The code

if (response.body() == null) return null;

is too old, from first implementation, e. g. GsonDecoder.

velo pushed a commit that referenced this pull request Oct 7, 2024
…l value (#1597)

* All decoders follow rule: if status 404 it returns empty or null value

* Replace decode404 with dismiss404
velo pushed a commit that referenced this pull request Oct 8, 2024
…l value (#1597)

* All decoders follow rule: if status 404 it returns empty or null value

* Replace decode404 with dismiss404
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants