app startup error handling #20941
app startup error handling #20941
Conversation
krehermann
requested review from
MStreet3
and removed request for
vyzaldysanchez
|
👋 krehermann, thanks for creating this pull request! To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team. Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks! |
There was a problem hiding this comment.
Pull request overview
This pull request improves error handling throughout the application's startup and lifecycle by replacing the deprecated github.com/pkg/errors package with standard library fmt.Errorf with %w verb for error wrapping. The PR aims to remove cryptic "not enabled" boot errors by adding context to all error messages, particularly around chain relayer initialization.
Changes:
- Replaced all
errors.Wrapcalls withfmt.Errorf(...: %w, err)for consistent error wrapping - Added contextual information to all error messages to aid debugging
- Removed the deprecated
github.com/pkg/errorsimport - Added debug logging for OS arguments and configuration files
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| core/main.go | Added debug output for OS arguments |
| core/cmd/shell_local.go | Replaced all error wrapping calls with fmt.Errorf, added context to error messages throughout node startup and key management |
| core/cmd/app.go | Added logging for configuration file arguments |
| .changeset/thick-bushes-occur.md | Added changeset entry documenting the error handling improvements |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
core/cmd/shell_local.go
Outdated
| numVersion, err := strconv.ParseInt(arg, 10, 64) | ||
| if err != nil { | ||
| return s.errorOut(errors.Errorf("Unable to parse %v as integer", arg)) | ||
| return s.errorOut(fmt.Errorf("Unable to parse %v as integer", arg)) |
There was a problem hiding this comment.
Corrected capitalization of 'Unable' to 'unable' to follow Go error message conventions (error messages should not be capitalized unless they begin with a proper noun).
| return s.errorOut(fmt.Errorf("Unable to parse %v as integer", arg)) | |
| return s.errorOut(fmt.Errorf("unable to parse %v as integer", arg)) |
krehermann
force-pushed
the
chore/app-startup-errs
branch
from
caffd28 to
721d7b9
Compare
| addressBytes, err := hexutil.Decode(addressHex) | ||
| if err != nil { | ||
| return s.errorOut(errors.Wrap(err, "could not decode address")) | ||
| return s.errorOut(fmt.Errorf("could not decode address '%s': %w", addressHex, err)) |
There was a problem hiding this comment.
@MStreet3 the empty addr would error here
![trunk-io[bot]](https://avatars.githubusercontent.com/in/120106?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| numVersion, err := strconv.ParseInt(arg, 10, 64) | ||
| if err != nil { | ||
| return s.errorOut(errors.Errorf("Unable to parse %v as integer", arg)) | ||
| return s.errorOut(fmt.Errorf("unable to parse %v as integer", arg)) |
There was a problem hiding this comment.
The error message could be more specific by including the actual parsing error. Consider: fmt.Errorf(\"unable to parse %v as integer: %w\", arg, err)
| return s.errorOut(fmt.Errorf("unable to parse %v as integer", arg)) | |
| return s.errorOut(fmt.Errorf("unable to parse %v as integer: %w", arg, err)) |
3 participants
The intention is to remove the cryptic boot error 'not enabled', which occurs in the chainlink-evm lib when a chain or address is not properly configured.
To do this, is a simply matter of error wrapping around the relayer calls.
While in the neighborhood, I
Requires
Supports