app: errors revamp #4044
app: errors revamp #4044
Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request refactors error messages across the app package to follow consistent patterns. The changes improve clarity by removing subjective phrasing like "should" and "cannot," adopting more direct, action-oriented error descriptions.
Key Changes:
- Standardized error message format from subjective assertions ("should match", "cannot read") to direct action descriptions ("mismatch", "read")
- Updated error messages to be more concise and technically precise
- Improved consistency in URL/address terminology (e.g., "bn addr" → "beacon node address")
Reviewed Changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| app/utils.go | Updated directory/file comparison error messages to use direct terminology |
| app/utils_test.go | Updated test expectations to match new error message format |
| app/tracer/trace.go | Improved stdout trace exporter error message clarity |
| app/sse/client.go | Standardized beacon node address parsing and HTTP response error messages |
| app/sse/client_internal_test.go | Updated test to expect new response status error message |
| app/protonil/protonil.go | Fixed grammar in field count error message |
| app/privkeylock/privkeylock.go | Standardized private key lock file operation error messages |
| app/obolapi/*.go | Consistent URL parsing and HTTP operation error messages across API files |
| app/k1util/k1util.go | Improved clarity of scalar validation error message |
| app/eth2wrap/*.go | Standardized validator data and proposal error messages |
| app/eth1wrap/runner.go | Simplified eth1 client connection error messages |
| app/disk.go | Standardized file operation error messages |
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 #4044 +/- ##
==========================================
- Coverage 53.81% 53.81% -0.01%
==========================================
Files 242 242
Lines 39425 39425
==========================================
- Hits 21217 21215 -2
- Misses 15965 15969 +4
+ Partials 2243 2241 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
nit: here I actually prefer the "cannot be" instead of "is", similar to "nickname cannot be..."
There was a problem hiding this comment.
I agree. If we change it here, we have to do in the other PRs as well. There was one with a lot of those changes.
There was a problem hiding this comment.
Claude did that because there's no universal consensus, but the trend is to describe what IS wrong (state), not what action failed. It mentioned many sources like "Effective Go", but I totally agree we can set our own rules here. And since Kalo is the boss, I am modifying as suggested.
There was a problem hiding this comment.
I agree. If we change it here, we have to do in the other PRs as well. There was one with a lot of those changes.
pinebit
force-pushed
the
pinebit/app-errors-revamp
branch
from
91f3c31 to
7601461
Compare
|
pinebit
added
the
merge when ready
app package errors revamp with claude.
category: refactor
ticket: #3882