-
Notifications
You must be signed in to change notification settings - Fork 382
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: fix the scope of recover()
#1672
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1672 +/- ##
==========================================
+ Coverage 44.84% 47.60% +2.75%
==========================================
Files 459 389 -70
Lines 67625 61641 -5984
==========================================
- Hits 30329 29344 -985
+ Misses 34756 29846 -4910
+ Partials 2540 2451 -89 ☔ View full report in Codecov by Sentry. |
converted to draft while I address some issues brought up in the comments |
This reverts commit e34a322.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 💯
Thank you for resolving this issue 🙏
I've pinged @petar-dambovaliev to give another sanity check, otherwise we're good to go
High level overview
Addresses #1656. This is meant to bring achieve parity between gno and go's
recover
functionality. The new tests help to ensure this.BREAKING CHANGE: this will break any realm code that has relies on the incorrect
recover
implementationChanges
For the convenience of the reviewer, here are the categories of changes that have been made:
recover
function has been modified to determine whether a panic can be recovered; this is based on both the "level" of nested defer statements and the frame from which the panic was initiatedFrame
s are now stored as pointers to avoid copying frame values as they are tracked by theException
s that have been thrownException
type has been defined to encapsulate both the panic string and the frame in which it was initiatedpanic
anddefer
testing
package'sRecover
function no longer works as intended; the tests have been modified to achieve the intended behaviorrecover
behaves as expected in certain edge casesContributors' checklist...
BREAKING CHANGE: xxx
message was included in the description