[Merged by Bors] - chore: refactor panics into error handling #6345
Conversation
fasmat
requested review from
dshulyak,
poszu,
ivan4th and
acud
as code owners
fasmat
changed the title
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #6345 +/- ##
=========================================
- Coverage 81.7% 81.7% -0.1%
=========================================
Files 312 312
Lines 34637 34657 +20
=========================================
+ Hits 28327 28331 +4
- Misses 4479 4489 +10
- Partials 1831 1837 +6 ☔ View full report in Codecov by Sentry. |
fetch/fetch.go
Outdated
| bs := datastore.NewBlobStore(cdb, proposals) | ||
|
|
||
| hashPeerCache, err := NewHashPeersCache(cacheSize) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("create hash peer cache: %w", err) |
There was a problem hiding this comment.
IMHO, propagating this up isn't the right choice. It can only fail if the private cacheSize const is invalid so the problem is:
- a clear programmer error
- entirely isolated to this pkg/file
I think it's totally fine to panic on it rather than complicating other code with an impossible (in normal conditions, when cacheSize >=0 invariant isn't violated) error, wdyt?
There was a problem hiding this comment.
OK, I changed it to panic, but left the changes regarding returning an error from NewFetch in the case this config option isn't a constant in the future (at which point I'd prefer returning an error instead of panicing).
|
bors merge |
## Motivation Updated some places where `log.Panic` is used and replaced it by returning the error, callers already handle them.
|
Build failed:
|
|
bors merge |
## Motivation Updated some places where `log.Panic` is used and replaced it by returning the error, callers already handle them.
|
Build failed: |
|
bors merge |
## Motivation Updated some places where `log.Panic` is used and replaced it by returning the error, callers already handle them.
|
Build failed:
|
|
bors merge |
## Motivation Updated some places where `log.Panic` is used and replaced it by returning the error, callers already handle them.
|
Pull request successfully merged into develop. Build succeeded:
|
spacemesh-bors
bot
changed the title
Motivation
Updated some places where
log.Panicis used and replaced it by returning the error, callers already handle them.Description
Return errors in some places where
log.Panicwas used beforeTest Plan
Existing tests pass.
TODO