Skip to content
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

Panic in native functions #663

Closed
tbruyelle opened this issue Mar 28, 2023 · 2 comments · Fixed by #732
Closed

Panic in native functions #663

tbruyelle opened this issue Mar 28, 2023 · 2 comments · Fixed by #732

Comments

@tbruyelle
Copy link
Contributor

tbruyelle commented Mar 28, 2023

Currently, the panics that are invoked in native functions uses the built-in panic(), and this is not possible to recover from them in gno code. I think this is wrong and the panic invoked in native functions should be recoverable.

This can be fixed by calling machine.Panic instead of the built-in panic. I vote for this change, wdyt ?

@thehowl
Copy link
Member

thehowl commented Mar 28, 2023

I think we should actually make this slightly different. There could be reasons why the code panics inside of native code for reasons also outside of our control - so I think that when calling native functions, gnovm should guard this with a recover() which is then panicked as a machine.Panic, so it can be caught by the gno code

@tbruyelle
Copy link
Contributor Author

So you think there will be cases where a panic doesn't necessarily trigger a machine.Panic ? Maybe I don't know...

How would you identify such cases ? With a specific err/struct passed in the panic function ? (sry that gives me bad memories like throw RuntimeException ^^')

tbruyelle added a commit to tbruyelle/gno that referenced this issue Apr 6, 2023
Fix gnolang#663

Some panic invoked in native functions should be recoverable from gno
code. To do that, `panic` is replaced with `machine.Panic()` which
checks first beyond the call stack if there's a recover, before
panic-ing.
tbruyelle added a commit to tbruyelle/gno that referenced this issue Apr 12, 2023
Fix gnolang#663

Some panic invoked in native functions should be recoverable from gno
code. To do that, `panic` is replaced with `machine.Panic()` which
checks first beyond the call stack if there's a recover, before
panic-ing.
tbruyelle added a commit to tbruyelle/gno that referenced this issue Apr 12, 2023
Fix gnolang#663

Some panic invoked in native functions should be recoverable from gno
code. To do that, `panic` is replaced with `machine.Panic()` which
checks first the call stack if there's a recover, before panic-ing.
jaekwon pushed a commit that referenced this issue Apr 18, 2023
Fix #663

Some panic invoked in native functions should be recoverable from gno
code. To do that, `panic` is replaced with `machine.Panic()` which
checks first the call stack if there's a recover, before panic-ing.
peter7891 pushed a commit that referenced this issue May 9, 2023
Fix #663

Some panic invoked in native functions should be recoverable from gno
code. To do that, `panic` is replaced with `machine.Panic()` which
checks first the call stack if there's a recover, before panic-ing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging a pull request may close this issue.

2 participants