fix(gateway): ipld.ErrNotFound should result in a 404 #630
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In case of blockstore restriction like with --offline or similar restriction, returning a 500 is inapropriate. It's also going back to a previous gateway behavior (at least in kubo v0.22, possibly later).
just a precaution to ensure 410 takes precedence, until we address ipfs#591
lidel
added a commit
to ipfs/kubo
that referenced
this pull request
Jul 30, 2024
applies and tests fix from ipfs/boxo#630
lidel
approved these changes
Jul 30, 2024
There was a problem hiding this comment.
Thank you for fixing this @MichaelMure. Took bit longer to review as the semantic meaning of this ipld error type changed multiple times over the years, and we also had big refactor since v0.22, but it seems to do the trick with how things are today.
Tested in ipfs/kubo#10466 and lgtm (we had NoFetch tests that expected 500, changed them to 404).
Sharness tests failing here can be ignored (circular dep, will clear up once we ship releases in both places).
This will be included in Kubo 0.30 (ipfs/kubo#10436).
lidel
added a commit
to ipfs/kubo
that referenced
this pull request
Jul 30, 2024
commit from upstream repo, includes ipfs/boxo#630
4 tasks
lidel
added a commit
to ipfs/kubo
that referenced
this pull request
Jul 30, 2024
applies and tests fix from ipfs/boxo#630
lidel
added a commit
to ipfs/kubo
that referenced
this pull request
Jul 30, 2024
* chore: go get -u * fix(gateway): return 404 in no-fetch contexts (#10466) applies and tests fix from ipfs/boxo#630
wenyue
pushed a commit
to wenyue/boxo
that referenced
this pull request
Oct 17, 2024
* fix(gateway): ipld.ErrNotFound should result in a 404 In case of blockstore restriction like with --offline or similar restriction, returning a 500 is inappropriate. It's also going back to a previous gateway behavior (at least in kubo v0.22, possibly later). * refactor(gateway): match 410 before 404s just a precaution to ensure 410 takes precedence, until we address ipfs#591 * docs: clarify 404 use case --------- Co-authored-by: Marcin Rataj <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix #593
In case of blockstore restriction like with --offline or similar restriction, returning a 500 is inapropriate.
It's also going back to a previous gateway behavior (at least in kubo v0.22, possibly later).