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

model exceptions as panics #87

Closed
dherman opened this issue May 12, 2016 · 4 comments
Closed

model exceptions as panics #87

dherman opened this issue May 12, 2016 · 4 comments

Comments

@dherman
Copy link
Collaborator

dherman commented May 12, 2016

Modeling JS exceptions as Result was a mistake. It's not possible to re-enter the engine when it's in a throwing state, so it's unsafe for Rust to continue running after V8 triggers an exception. Instead, we should model exceptions as panics, and offer a callback-based API for catching exceptions. This will have the nice side effect of making almost all Neon APIs vastly lighter-weight.

Also, we should use catch_unwind to catch Rust internal panics and prevent them from crashing V8.

@bnoordhuis
Copy link

You're probably well aware but you can clear a pending exception with v8::TryCatch::Reset(). If you consistently guard all entry points into the VM, you can keep on using the result-or-failure idiom. You will have to deal with empty handle return values anyway (i.e., without an associated exception) so result-or-failure is presumably still going to be necessary.

The, ah, exception to the rule is the termination exception introduced by v8::Isolate::TerminateExecution(). It can be cancelled again with a call to v8::Isolate::CancelTerminateExecution() (and detected with v8::TryCatch::HasTerminated() and v8::TryCatch::Continue()) but panicking might be more appropriate.

@DemiMarie
Copy link

I don't think panicking should be used by the library during normal operation. Rust programs can be compiled to abort on panic, and panicking is not optimized at all.

Better to use Result. When TerminateExecution is called, all calls into the API should fail. The cost of a single well-predicted branch is negligible.

Rust-side panics should result in calls to TerminateExecution if they escape to Neon.

@dherman
Copy link
Collaborator Author

dherman commented Nov 22, 2017

The design will need to be sketched out in the error safety RFC.

@dherman
Copy link
Collaborator Author

dherman commented Nov 22, 2017

Closing this since it should be part of the RFC design discussion.

@dherman dherman closed this as completed Nov 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants