celestia-node icon indicating copy to clipboard operation
celestia-node copied to clipboard

fix(core): correctly set Listener.handleNewSignedBlockfrom's spanStatus from returned error or panics

Open odeke-em opened this issue 1 year ago • 4 comments

This change ensures that if a panic occurs, that span.RecordError correctly records it and then if an error is returned that span.Status correctly captures and records it.

Fixes #3814

odeke-em avatar Oct 06 '24 00:10 odeke-em

Codecov Report

Attention: Patch coverage is 40.00000% with 9 lines in your changes missing coverage. Please review.

Project coverage is 46.41%. Comparing base (2469e7a) to head (fbc937c). Report is 335 commits behind head on main.

Files with missing lines Patch % Lines
core/listener.go 40.00% 8 Missing and 1 partial :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3817      +/-   ##
==========================================
+ Coverage   44.83%   46.41%   +1.57%     
==========================================
  Files         265      314      +49     
  Lines       14620    18151    +3531     
==========================================
+ Hits         6555     8424    +1869     
- Misses       7313     8709    +1396     
- Partials      752     1018     +266     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar Oct 06 '24 01:10 codecov-commenter

There is a lint issue and LGTM

Wondertan avatar Oct 07 '24 09:10 Wondertan

Thanks @Wondertan! Kindly help me take a look again.

odeke-em avatar Oct 07 '24 10:10 odeke-em

All checks passing, we are good to go @Wondertan

odeke-em avatar Oct 07 '24 10:10 odeke-em

@odeke-em would you be able to implement the changes requested by Hlib here or should we take over the PR from you? If we don't hear back in a week, we'll take it over. Thanks!

renaynay avatar Nov 19 '24 13:11 renaynay

Hey @renaynay, please feel free to take over. Thank you.

odeke-em avatar Nov 19 '24 15:11 odeke-em