Improve error handling #366
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe recent changes focus on improving error handling across multiple functions in the codebase. Enhancements include more informative error messages that provide context for failures, making it easier for developers to diagnose issues. Key functions have been updated to ensure that errors are wrapped with relevant details, promoting better maintainability and debugging capabilities. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Requester as Requester Service
participant e.client as External Client
Client->>Requester: Call executeScriptAtHeight
Requester->>e.client: ExecuteScriptAtBlockHeight
e.client-->>Requester: Return result or error
alt Error contains specific message
Requester-->>Client: Return ErrOutOfRange
else Other errors
Requester-->>Client: Return original error
end
Assessment against linked issues
Poem
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- services/requester/requester.go (1 hunks)
Additional comments not posted (2)
services/requester/requester.go (2)
680-680: LGTM!The addition of error handling logic improves the clarity and specificity of error messages.
686-692: EnsureErrOutOfRangeis handled correctly.The new error handling logic introduces
ErrOutOfRange. Ensure that this error is handled correctly in the upstream code.Verification successful
ErrOutOfRangeis handled correctly.The error
ErrOutOfRangeintroduced in the new error handling logic is appropriately checked and handled in the codebase. The handling includes proper logging, returning the error when necessary, and adequate test coverage.
services/requester/requester.go: Error is logged if notErrOutOfRange.services/requester/cross-spork_client.go: Error is returned when the client is nil.services/requester/cross-spork_client_test.go: Test cases verify scenarios involvingErrOutOfRange.api/api.go: Error is handled and a formatted error message is returned.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage and handling of `ErrOutOfRange` in the codebase. # Test: Search for the usage and handling of `ErrOutOfRange`. Expect: Proper handling of the error. rg --type go 'ErrOutOfRange'Length of output: 1056
Script:
#!/bin/bash # Extract lines around the usage of ErrOutOfRange in requester.go rg --type go -A 5 -B 5 'ErrOutOfRange' services/requester/requester.go # Extract lines around the usage of ErrOutOfRange in cross-spork_client.go rg --type go -A 5 -B 5 'ErrOutOfRange' services/requester/cross-spork_client.go # Extract lines around the usage of ErrOutOfRange in cross-spork_client_test.go rg --type go -A 5 -B 5 'ErrOutOfRange' services/requester/cross-spork_client_test.go # Extract lines around the usage of ErrOutOfRange in api.go rg --type go -A 5 -B 5 'ErrOutOfRange' api/api.goLength of output: 3800
| @@ -246,15 +246,15 @@ func decodeTransactionEvent( | |||
| if txEvent.TransactionType == types.DirectCallTxType { | |||
| directCall, err := types.DirectCallFromEncoded(encodedTx) | |||
| if err != nil { | |||
| return nil, nil, fmt.Errorf("failed to RLP-decode direct call: %w", err) | |||
| return nil, nil, fmt.Errorf("failed to RLP-decode direct call [%x]: %w", encodedTx, err) | |||
There was a problem hiding this comment.
It occurred to me that we have the same exact error message in decodeTransactionEvent & UnmarshalTransaction:
In decodeTransactionEvent:
directCall, err := types.DirectCallFromEncoded(encodedTx)
if err != nil {
return nil, nil, fmt.Errorf("failed to RLP-decode direct call [%x]: %w", encodedTx, err)
}
gethTx := &gethTypes.Transaction{}
if err := gethTx.UnmarshalBinary(encodedTx); err != nil {
return nil, nil, fmt.Errorf("failed to RLP-decode transaction [%x]: %w", encodedTx, err)
}In UnmarshalTransaction:
directCall, err := types.DirectCallFromEncoded(value)
if err != nil {
return nil, fmt.Errorf("failed to RLP-decode direct call [%x]: %w", value, err)
}
if err := tx.UnmarshalBinary(value); err == nil {
return TransactionCall{Transaction: tx}, nil
}
return nil, fmt.Errorf("failed to RLP-decode transaction [%x]: %w", value, err)Can we add a prefix to the error message, so we can know whether the decoding error comes the ingestion component, or from the DB?
There was a problem hiding this comment.
no need because the error in the API gets wrapped.
Co-authored-by: Ardit Marku <[email protected]>
Co-authored-by: Ardit Marku <[email protected]>
Closes: #70
Description
TBD
For contributor use:
masterbranchFiles changedin the Github PR explorerSummary by CodeRabbit