chore(e2e): improve balance error messages #3375
Conversation
📝 WalkthroughWalkthroughThe pull request focuses on enhancing error handling in the Changes
Possibly related PRs
Suggested reviewers
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #3375 +/- ##
============================================
- Coverage 62.43% 49.39% -13.04%
============================================
Files 449 482 +33
Lines 31706 40254 +8548
============================================
+ Hits 19795 19885 +90
- Misses 11024 19482 +8458
Partials 887 887 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
e2e/runner/balances.go (2)
Line range hint
151-157: Align Bitcoin balance error handling with the new patternThe error handling in Bitcoin-related functions is inconsistent with the improved error messages pattern used elsewhere.
Apply this improvement:
func (r *E2ERunner) GetBitcoinBalance() (string, error) { address, _ := r.GetBtcAddress() total, err := r.GetBitcoinBalanceByAddress(address) if err != nil { - return "", err + return "", fmt.Errorf("get bitcoin balance: %w", err) } return total.String(), nil } func (r *E2ERunner) GetBitcoinBalanceByAddress(address btcutil.Address) (btcutil.Amount, error) { unspentList, err := r.BtcRPCClient.ListUnspentMinMaxAddresses(1, 9999999, []btcutil.Address{address}) if err != nil { - return 0, errors.Wrap(err, "failed to list unspent") + return 0, fmt.Errorf("list unspent bitcoin transactions: %w", err) }Also applies to: 164-167
Line range hint
128-130: Improve SPL balance parsing error handlingThe SPL balance parsing error could be more descriptive and consistent with the new error pattern.
Apply this improvement:
solSPLParsed, ok := new(big.Int).SetString(splBalance.Value.Amount, 10) if !ok { - return AccountBalances{}, errors.New("can't parse spl balance") + return AccountBalances{}, fmt.Errorf("parse spl balance %q: invalid format", splBalance.Value.Amount) }
🧹 Nitpick comments (2)
e2e/runner/balances.go (2)
53-73: Consider enhancing error handling structureThe error wrapping provides good context but could be improved for better error handling and reduced duplication.
Consider this refactoring approach:
+ type BalanceError struct { + TokenType string + Err error + } + + func (e *BalanceError) Error() string { + return fmt.Sprintf("get %s balance: %v", e.TokenType, e.Err) + } + + func (e *BalanceError) Unwrap() error { + return e.Err + } // In GetAccountBalances: - return AccountBalances{}, fmt.Errorf("get zeta balance: %w", err) + return AccountBalances{}, &BalanceError{TokenType: "zeta", Err: err}This approach:
- Provides structured errors that can be type-asserted upstream
- Maintains consistent error messaging
- Reduces code duplication
- Enables better error handling patterns
Line range hint
53-124: Overall error handling improvements look goodThe enhanced error messages provide better context for debugging balance-related issues. The consistent use of
fmt.Errorfwith the%wverb maintains error wrapping capabilities, allowing proper error inspection upstream.Consider these additional improvements for the future:
- Document common error types and their handling in the package documentation
- Add error codes or error types for programmatic error handling
- Consider implementing error grouping for batch operations
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
e2e/runner/balances.go(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
e2e/runner/balances.go (1)
Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: start-e2e-test / e2e
Add context to get balance error messages to make debugging easier. See https://github.com/zeta-chain/infrastructure/issues/1958 as an example of why the current error messages are not sufficient.
Summary by CodeRabbit