core: errors revamp #4046
core: errors revamp #4046
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR revamps error messages across the core package to follow a consistent style by removing prefixes like "failed to", "cannot", and "could not" from error messages. The changes maintain the same semantic meaning while providing more concise error descriptions that are better suited for error wrapping contexts.
- Standardizes error message formatting by removing action prefixes (e.g., "failed to", "cannot")
- Updates test expectations to match new error message format
- Improves consistency across error messages throughout the codebase
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| core/validatorapi/validatorapi.go | Removes "failed to" and "cannot" prefixes from error messages |
| core/unsigneddata_test.go | Updates test error expectations to match new error message format |
| core/unsigneddata.go | Changes error messages from "cannot be blinded" to "is not blinded" |
| core/tracker/tracker_internal_test.go | Updates test error expectations to remove "could not" prefix |
| core/tracker/tracker.go | Removes "failed at step" from duty error message |
| core/tracker/inclusion.go | Removes "could not" prefix from proposal synthetic determination error |
| core/sigagg/sigagg.go | Changes "verification failed" to "verify" in error wrapper |
| core/scheduler/scheduler.go | Changes "cannot be" to "is" in nil validation error messages |
| core/qbft/qbft.go | Shortens timeout error message for comparison data waiting |
| core/fetcher/fetcher.go | Changes "cannot be nil" to "is nil" for attestation data validation |
| core/bcast/bcast.go | Changes "cannot broadcast" to "convert to blinded proposal" and "cannot be nil" to "is nil" |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4046 +/- ##
==========================================
+ Coverage 53.79% 53.80% +0.01%
==========================================
Files 242 242
Lines 39425 39425
==========================================
+ Hits 21207 21213 +6
+ Misses 15980 15972 -8
- Partials 2238 2240 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
core/tracker/inclusion.go
Outdated
| defer func() { | ||
| if r := recover(); r != nil { | ||
| err = errors.New("could not determine if proposal was synthetic or not", | ||
| err = errors.New("determine if proposal was synthetic", |
There was a problem hiding this comment.
I'd keep it as it is. I usually try to follow what we have in the docs, which is to use negative words (cannot/failed/etc.) in new errors and not to use them in wrapping.
| err = errors.New("determine if proposal was synthetic", | |
| err = errors.New("could not determine if proposal was synthetic or not", |
pinebit
force-pushed
the
pinebit/core-errors-revamp
branch
from
d714711 to
3523a67
Compare
|
pinebit
added
the
merge when ready
core package errors revamp with claude.
category: refactor
ticket: #3882