[DX-2128] Better error messages in Seth #2207
Conversation
Tofel
force-pushed
the
dx-2128-seth-better-error-msgs
branch
from
0d05ec3 to
90fa726
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR focuses on improving error messages across the Seth library by replacing the github.com/pkg/errors package with Go's standard error handling (fmt.Errorf and errors package) and enhancing error messages with actionable troubleshooting guidance. The changes systematically improve user experience by providing clearer, more detailed error messages that include potential causes and solutions.
Reviewed Changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| seth/go.mod | Moved pkg/errors from direct to indirect dependency |
| seth/.changeset/v1.51.4.md | Added changelog entry documenting the error message improvements |
| seth/.tool-versions | Added golangci-lint version specification |
| seth/util.go | Enhanced error messages for insufficient funds with detailed balance information and actionable solutions |
| seth/tracing.go | Improved error messages for tracer initialization, trace retrieval, and call decoding with troubleshooting steps |
| seth/retry.go | Enhanced error messages for transaction retry failures and gas bumping issues |
| seth/nonce.go | Improved nonce manager error messages with configuration guidance |
| seth/keyfile.go | Enhanced error messages for key generation and fund return operations |
| seth/header_cache.go | Improved nil header error message with issue reporting guidance |
| seth/gas_adjuster.go | Enhanced gas estimation error messages with network congestion and RPC troubleshooting |
| seth/gas.go | Improved gas fee estimation error messages with actionable solutions |
| seth/decode.go | Enhanced transaction decoding error messages with detailed failure explanations |
| seth/contract_store.go | Improved ABI/BIN loading error messages with file discovery and generation guidance |
| seth/config.go | Enhanced configuration validation error messages with detailed setup instructions |
| seth/cmd/seth.go | Updated CLI error handling to use standard error wrapping |
| seth/client.go | Comprehensive error message improvements across client initialization, validation, and transaction handling |
| seth/block_stats.go | Enhanced block statistics error messages with RPC connectivity troubleshooting |
| seth/abi_finder.go | Improved ABI lookup error messages with detailed troubleshooting steps |
| seth/client_builder.go | Updated imports to use standard errors package |
| lib/utils/seth/seth.go | Enhanced Seth configuration error messages with setup guidance |
| Test files (multiple) | Updated test assertions to use Contains instead of exact string matching for improved flexibility |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| gasLeft := mustSafeInt64(dc.GasLimit) - mustSafeInt64(dc.GasUsed) | ||
| if gasLeft < 0 { | ||
| l.Debug().Str(fmt.Sprintf("%s- Gas left", indentation), fmt.Sprintf("%d (negative due to gas refunds or stipends)", gasLeft)).Send() | ||
| } else { | ||
| l.Debug().Str(fmt.Sprintf("%s- Gas left", indentation), fmt.Sprintf("%d", gasLeft)).Send() | ||
| } |
There was a problem hiding this comment.
Potential panic from mustSafeInt64 if dc.GasLimit or dc.GasUsed exceed math.MaxInt64. The values come from blockchain data which uses uint64, but gas values in practice rarely exceed int64 max. Consider adding validation or handling overflow gracefully instead of panicking, especially since this is just for logging.
| if diff == 0 { | ||
| return []*DecodedCall{} | ||
| } | ||
|
|
There was a problem hiding this comment.
This early return when diff == 0 causes the function to skip creating and returning the missedCalls slice that would be populated below. However, the logic at line 547-624 still constructs missedCalls even when not needed. Consider moving this check earlier in the function or restructuring the logic to avoid unnecessary computation when there are no missing calls.
| // Only construct missedCalls if there are missing calls |
There was a problem hiding this comment.
this is absolutely crazy... since there are no missed calls (diff == 0), then early exist happens when it should. And if there are missed calls then constructing them is always needed 🤯
1 participant
This pull request focuses on improving error messages and making them more actionable throughout the
sethpackage. It replaces the usage of thegithub.com/pkg/errorslibrary with Go's standardfmt.Errorf, and revises error handling to provide clearer guidance and troubleshooting steps for users. The changes affect configuration validation, contract ABI lookup, block statistics, and client initialization, making it easier for developers to diagnose and resolve common issues.Error message improvements and actionable guidance:
seth.go,abi_finder.go, andblock_stats.go. These messages now include troubleshooting steps and links to documentation or issue trackers where relevant. [1] [2] [3] [4] [5] [6]Error handling and library updates:
github.com/pkg/errorsand replaced them with standardfmt.Errorf, simplifying imports and error wrapping throughout the codebase. [1] [2] [3] [4]Client initialization and configuration validation:
NewClientWithConfig,NewClientRaw) to clarify configuration issues, RPC connectivity problems, and contract mapping errors, providing actionable next steps for users. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11]Documentation:
Below is a summarization created by an LLM (gpt-4-0125-preview). Be mindful of hallucinations and verify accuracy.
Why
This patch aims to improve error handling and messaging across the Seth library, particularly focusing on providing more actionable feedback and ensuring compatibility with updated dependencies.
What
github.com/pkg/errorswith Go's standarderrorspackage and custom error formatting to provide clearer, more actionable error messages.Specific Changes Include:
github.com/pkg/errorswith standarderrorspackage for error wrapping.