rubyjs / mini_racer

Minimal embedded v8
MIT License
585 stars 91 forks source link

pass exception text into javascript #264

Open seanmakesgames opened 1 year ago

seanmakesgames commented 1 year ago

Our usage of mini_racer needs some way for exceptional cases in ruby to be able to be handled in javascript. Currently there is no way to identify what exception happened when it is caught by JS.

Also not sure what I do or do not need to do with handles and scopes here to make this string activity safe. Please let me know!

seanmakesgames commented 1 year ago

@bjfish Know why truffle test runs are failing here? (I don't know anything about truffleruby)

tisba commented 1 year ago

For a second I was hoping this would also address https://github.com/rubyjs/mini_racer/issues/129 🙈

seanmakesgames commented 1 year ago

@tisba If I read your issue correctly, then this PR might be an incremental start to what you are looking for. We’d also love to get a backtrace for the JS entry callstack as well as a typed Error object, but wanted to start with something less controversial and simpler

bjfish commented 1 year ago

Know why truffle test runs are failing here? @eregon Could you review this?

seanmakesgames commented 1 year ago

Thanks @bjfish and @eregon

seanmakesgames commented 1 year ago

Any idea where to start with these? Are there dev environment setup instructions somewhere?

Should I 'this feature doesn't work on truffle' the tests like some others?

Any feedback @SamSaffron ?

bjfish commented 1 year ago

@eregon I suspect this would require calling message, string conversion if needed, and throwing in javascript (maybe with a helper method) similar to the implementation in this PR around: https://github.com/rubyjs/mini_racer/blob/master/lib/mini_racer/truffleruby.rb#L17-L23

eregon commented 1 year ago

I think we need a review from maintainers first to decide the desired semantics. For example what should be the return value of that test, what should a JS exception converted to Ruby return, etc.

Then I can push a commit to implement it for the GraalVM backend (fairly simple since it's all Ruby code).

seanmakesgames commented 1 year ago

@eregon Yep. That implementation is a lot larger of a code change, though. @Fayti1703 is currently putting this together. https://github.com/rubyjs/mini_racer/compare/master...Fayti1703:mini_racer:ex_msg_text

If those features are the minimum requirements for getting these in, we can make that happen. The changes in this PR are the minimum requirements for our usage to be able to 'discern in any way at all' which exception happened in ruby for error handling on the JS side.

cc @tisba

Fayti1703 commented 1 year ago

The changes mentioned by seanmakesgames above are technically complete in the sense that they cause a JS-Error subclass to be thrown.

However one additional thing I'd like to do is pass through the Ruby error VALUE via the JS-Exception to rethrow it at the JS->Ruby boundary (instead of the current @current_exception storage, which only works for the first thrown error).

Currently still considering how best to do that, given the GC interactions.

I would be happy to create a PR with what I have so far, though.

seanmakesgames commented 1 year ago

Fully featured version of this was implemented here against old point in history: https://github.com/seanmakesgames/mini_racer/pull/4

and has a handful of live testing against it.

eregon commented 1 year ago

@SamSaffron Could you review this or ask another maintainer to review?

IMHO it's fine to get this in, although it seems of limited value if there is already a way to throw a proper JS Error. I think a PR here to convert Ruby exceptions to JS exceptions would be welcome.

eregon commented 1 year ago

I can do the corresponding changes on the mini_racer truffleruby backend, but would like this to be reviewed first.

seanmakesgames commented 1 year ago

IMHO it's fine to get this in, although it seems of limited value if there is already a way to throw a proper JS Error. I think a PR here to convert Ruby exceptions to JS exceptions would be welcome.

This PR was a minimum requirement for my game to be able to do --anything-- with the results of an exception in ruby. I've got the fully featured deal running off of a fork. I'll wait to create that one until the test suite changes and libv8 upgrade is in.