Wrap panic errors #7
Conversation
WalkthroughThe recent changes enhance the virtual machine's error handling capabilities by introducing a new function, Changes
Sequence Diagram(s)sequenceDiagram
participant VM
participant FunctionRegistry
participant ErrorHandler
VM->>FunctionRegistry: Register do_not_call_wrapped
VM->>VM: Call do_not_call_wrapped
VM-->>ErrorHandler: Trigger panic
ErrorHandler->>ErrorHandler: Wrap original error in PanicError
ErrorHandler->>VM: Return PanicError
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 (
|
Codecov ReportAttention: Patch coverage is @@ Coverage Diff @@
## main #7 +/- ##
==========================================
- Coverage 98.34% 98.31% -0.03%
==========================================
Files 23 23
Lines 6513 6519 +6
==========================================
+ Hits 6405 6409 +4
- Misses 79 81 +2
Partials 29 29
|
bdeneux
force-pushed
the
feat/impove-panic-error
branch
from
fd40b89
to
7d0eab8
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- engine/builtin_test.go (2 hunks)
- engine/promise.go (2 hunks)
- engine/variable.go (2 hunks)
Files skipped from review due to trivial changes (1)
- engine/variable.go
Additional context used
GitHub Check: codecov/patch
engine/promise.go
[warning] 178-179: engine/promise.go#L178-L179
Added lines #L178 - L179 were not covered by tests
Additional comments not posted (4)
engine/promise.go (2)
173-176: LGTM!The introduction of the
PanicErrorstruct improves error tracking and management by wrapping original error messages.
132-137: LGTM! Verify the usage ofPanicErrorthroughout the codebase.The changes to the
panicErrorfunction improve error handling by distinguishing betweenerrortypes and other types, and wrapping them in aPanicErrortype.However, ensure that the new
PanicErrortype is used correctly throughout the codebase.Verification successful
Verified: The
PanicErrortype is used correctly throughout the codebase.The
PanicErrortype is consistently used inengine/promise.gofor error handling and is also tested inengine/builtin_test.go.
engine/promise.go: Definition and usage ofPanicError.engine/builtin_test.go: Test cases involvingPanicError.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the new `PanicError` type throughout the codebase. # Test: Search for the usage of `PanicError`. Expect: Correct usage in all relevant places. rg --type go 'PanicError'Length of output: 699
engine/builtin_test.go (2)
36-38: LGTM! The new function registration is consistent with improved error handling.The
do_not_call_wrappedfunction panics with a wrapped error message, which aligns with the goal of enhancing error reporting.
66-68: LGTM! The updated error handling in test cases is consistent with improved error reporting.The use of
PanicErrorin test cases enhances the structure and clarity of error handling. The new test case fordo_not_call_wrappedensures proper testing of the new function.
Add a wrapper typed error over the panic error to facilitate error catching and comparison, enabling the return of more appropriate errors on the logic module side.
This improvement will be used in the logic module to correctly catch the
ErrMaxVariableserror in order to return a more appropriate gRPC error (LimitExceeded). Available in axone-protocol/axoned#703 (not implemented yet)