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

fix: gateway panic because of overshadowed err #511

Merged
merged 3 commits into from
Nov 10, 2023
Merged

fix: gateway panic because of overshadowed err #511

merged 3 commits into from
Nov 10, 2023

Conversation

hacdias
Copy link
Member

@hacdias hacdias commented Nov 10, 2023

This PR corrects error handling in the GetCAR implementation of the block backend.

NOTE: sporadic panics, occurring during CAR requests under specific conditions, were harmless thanks to panic-recovery logic in the gateway handler, but created unnecessary noise in logs.

@hacdias hacdias self-assigned this Nov 10, 2023
@hacdias hacdias requested a review from lidel as a code owner November 10, 2023 12:30
@hacdias hacdias requested a review from a team as a code owner November 10, 2023 12:32
Copy link

codecov bot commented Nov 10, 2023

Codecov Report

Merging #511 (a2c5d35) into main (f1442f7) will decrease coverage by 0.06%.
The diff coverage is 16.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #511      +/-   ##
==========================================
- Coverage   65.58%   65.53%   -0.06%     
==========================================
  Files         207      207              
  Lines       25550    25553       +3     
==========================================
- Hits        16758    16745      -13     
- Misses       7319     7336      +17     
+ Partials     1473     1472       -1     
Files Coverage Δ
gateway/blocks_backend.go 42.50% <16.66%> (-0.21%) ⬇️

... and 17 files with indirect coverage changes

Copy link
Contributor

@Jorropo Jorropo left a comment

Choose a reason for hiding this comment

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

Thx, we seems to get error shadows wrong reliably, had similar issues in the connect logic of go-libp2p.

CHANGELOG.md Outdated Show resolved Hide resolved
@hacdias hacdias enabled auto-merge (squash) November 10, 2023 13:35
@hacdias hacdias merged commit 49e0f39 into main Nov 10, 2023
15 checks passed
@hacdias hacdias deleted the fix-gw-panic branch November 10, 2023 13:59
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.

3 participants