pass exception text into javascript #264
Conversation
|
@bjfish Know why truffle test runs are failing here? (I don't know anything about truffleruby) |
|
For a second I was hoping this would also address #129 🙈 |
|
@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 |
|
|
Any idea where to start with these? Should I 'this feature doesn't work on truffle' the tests like some others? Any feedback @SamSaffron ? |
|
@eregon I suspect this would require calling |
| error_message = "Actual Error Message" | ||
| context.attach("a", proc{|a| raise error_message}) | ||
|
|
||
| assert_equal error_message, context.eval("try { a(); } catch (e) { e }") |
There was a problem hiding this comment.
Isn't it rather weird that the return value of this is a Ruby exception converted to a JS exception string, but the return value of this context.eval is just a string and not an exception? I think that's pretty confusing.
It seems like it should at least be wrapped into some MiniRacer exception or so.
EDIT: see #264 (comment)
|
I think we need a review from maintainers first to decide the desired semantics. Then I can push a commit to implement it for the GraalVM backend (fairly simple since it's all Ruby code). |
| @@ -1281,7 +1281,8 @@ gvl_ruby_callback(void* data) { | |||
|
|
|||
| if(callback_data.failed) { | |||
| rb_iv_set(parent, "@current_exception", result); | |||
| args->GetIsolate()->ThrowException(String::NewFromUtf8Literal(args->GetIsolate(), "Ruby exception")); | |||
| VALUE message = rb_funcall(result, rb_intern("message"), 0); | |||
| args->GetIsolate()->ThrowException(String::NewFromUtf8(args->GetIsolate(), RSTRING_PTR(message), NewStringType::kNormal, RSTRING_LENINT(message)).ToLocalChecked()); | |||
There was a problem hiding this comment.
The code previously throwed a JS String "Ruby exception", and now throws a JS String with the exception.message.
I think it should throw an actual JS exception (an Error or subclass IIUC https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error)
|
@eregon Yep. That implementation is a lot larger of a code change, though. @Fayti1703 is currently putting this together. 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 |
|
The changes mentioned by seanmakesgames above are technically complete in the sense that they cause a JS- However one additional thing I'd like to do is pass through the Ruby 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. |
|
Fully featured version of this was implemented here against old point in history: and has a handful of live testing against it. |
|
@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 can do the corresponding changes on the mini_racer truffleruby backend, but would like this to be reviewed first. |
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. |
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!