Skip to content

fix(gateway): return HTTP 500 on namesys.ErrResolveFailed#150

Merged
lidel merged 1 commit intomainfrom
fix/kubo-9514
Feb 22, 2023
Merged

fix(gateway): return HTTP 500 on namesys.ErrResolveFailed#150
lidel merged 1 commit intomainfrom
fix/kubo-9514

Conversation

@hacdias
Copy link
Copy Markdown
Member

@hacdias hacdias commented Feb 1, 2023

Fixes ipfs/kubo#9514.

There's a catch here. AFAIK, there is no good way of distinguishing between a non-existing DNSLink entry and an IPNS key that cannot be resolved. With this PR, both return 500 Internal Server Error, as both return namesys.ErrResolveFailed as error. What do you think @lidel? If we want to treat both errors the same way, I think it's good to go. Otherwise, we may need further changes in the namesys package.

Sharness tests will fail here. See ipfs/kubo#9589.

@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 1, 2023

Codecov Report

Merging #150 (77fb194) into main (c394290) will increase coverage by 0.21%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #150      +/-   ##
==========================================
+ Coverage   29.19%   29.41%   +0.21%     
==========================================
  Files          96       99       +3     
  Lines       11018    11093      +75     
==========================================
+ Hits         3217     3263      +46     
- Misses       7448     7472      +24     
- Partials      353      358       +5     
Impacted Files Coverage Δ
gateway/handler.go 62.75% <100.00%> (+0.71%) ⬆️
gateway/handler_unixfs__redirects.go 38.79% <0.00%> (-0.55%) ⬇️
files/is_hidden_windows.go 82.35% <0.00%> (ø)
tar/sanitize_windows.go 41.66% <0.00%> (ø)
files/filewriter_windows.go 100.00% <0.00%> (ø)

@hacdias hacdias self-assigned this Feb 1, 2023
@hacdias hacdias marked this pull request as ready for review February 1, 2023 09:40
@hacdias hacdias requested a review from lidel as a code owner February 1, 2023 09:40
@hacdias
Copy link
Copy Markdown
Member Author

hacdias commented Feb 8, 2023

@lidel this would be easier to rebase once #156 is merged.

Copy link
Copy Markdown
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

#156 Is merged, feel free to rebase and move tests from ipfs/kubo#9589 to this PR

@lidel lidel changed the title fix(gateway): return http.StatusInternalServerError by default if cannot resolve fix(gateway): return http.StatusInternalServerError on namesys.ErrResolveFailed Feb 15, 2023
@lidel lidel changed the title fix(gateway): return http.StatusInternalServerError on namesys.ErrResolveFailed fix(gateway): return HTTP 500 on namesys.ErrResolveFailed Feb 15, 2023
@hacdias hacdias force-pushed the fix/kubo-9514 branch 2 times, most recently from 3c9b71d to 87e8bc6 Compare February 15, 2023 07:28
@hacdias
Copy link
Copy Markdown
Member Author

hacdias commented Feb 15, 2023

@lidel sharness will also fail here, as there is a single sharness test that checks the status code for this cases. See ipfs/kubo#9589 for Kubo PR.

@lidel lidel merged commit 7b20141 into main Feb 22, 2023
@lidel lidel deleted the fix/kubo-9514 branch February 22, 2023 01:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects
Archived in project

Development

Successfully merging this pull request may close these issues.

gateway: IPNS key resolution timeout should return HTTP 5xx

2 participants