catch ruby exception (raised in attached function) within JS: change in behaviour

[pascalbetz]

Hi Mini Racer team.

We've run into an issue with MiniRacer and are wondering if we were relying on a undocumented feature or if this is a bug in more recent versions.

We used to be able to catch exception thrown in attached ruby functions inside a JS catch block. The ruby exception was "converted" to a JS exception.

Running this ruby script

require "mini_racer"
context = MiniRacer::Context.new
context.attach("externalFunction", proc { raise "raised in ruby"} )
result = context.eval(<<~JS)
  try {
    externalFunction()
  } catch(error) {
   "catch in JS"
  }
JS

gives me different results starting with 0.17.0.pre6

mini_racer-0.17.0.pre5 (and in all version since at least 0.12.0)

"catch in JS"

mini_racer-0.17.0.pre6

demo.rb:3:in 'block in <main>': raised in ruby (RuntimeError)
	from bla.rb:4:in 'MiniRacer::Context#eval'
	from bla.rb:4:in '<main>'

Is this not supported anymore?

We found this issue #44 from 2016, which discusses this feature and then got closed.
I tried versions down to 0.5.0 where it works. In versions before this i get segfaults or have issues installing.

Thanks for your help and have a nice day.

[SamSaffron]

tough one @bnoordhuis not sure how we would swing this with the new serialization protocol?

[bnoordhuis]

@pascalbetz I can't test it tonight but can you check if this patch produces the behavior you're after?

diff --git a/ext/mini_racer_extension/mini_racer_v8.cc b/ext/mini_racer_extension/mini_racer_v8.cc
index 38d14ce..3d21796 100644
--- a/ext/mini_racer_extension/mini_racer_v8.cc
+++ b/ext/mini_racer_extension/mini_racer_v8.cc
@@ -388,8 +388,11 @@ void v8_api_callback(const v8::FunctionCallbackInfo<v8::Value>& info)
         v8_roundtrip(st.ruby_context, &p, &n);
         if (*p == 'c') // callback reply
             break;
-        if (*p == 'e') // ruby exception pending
-            return st.isolate->TerminateExecution();
+        if (*p == 'e') { // ruby exception pending
+            auto message = v8::String::NewFromUtf8Literal(st.isolate, "Ruby exception");
+            auto exception = v8::Exception::TypeError(message);
+            return st.isolate->ThrowException(exception);
+        }
         v8_dispatch(st.ruby_context);
     }
     v8::ValueDeserializer des(st.isolate, p+1, n-1);

[pascalbetz]

thanks @bnoordhuis. unfortunately this does not work (void function 'v8_api_callback' should not return a value).
unfortunately i'm not familiar enough with the native code and mini_racer to be of great help or fix this myself.

This testcase should reproduce the issue

  def test_exception_in_attached_function_is_caught_in_js
    context = MiniRacer::Context.new
    context.attach("raiseInRuby", proc { raise "ruby raised" })

    context.eval <<~JS
      try {
        raiseInRuby()
      } catch(error) {
        // do nothing
      }
    JS
  end

[bnoordhuis]

I'm looking into this. Some coordination and agreement needs to take place between the Ruby and JS threads.

The Ruby thread has to stop propagating the Ruby exception when the JS thread catches the JS counterpart exception, but only then - an uncaught JS exception should still result in the Ruby exception propagating.

[pascalbetz]

Hi @bnoordhuis

Returned back to work today and saw your reply. Thanks for looking into this.

If there is a way I can support this work (would need some (probably lots of) guidance though), let me know.

[bnoordhuis]

It turned out surprisingly tricky to implement but there's a fix in #373 now.

[pascalbetz]

Awesome.
I should be able to give it a try on monday.

[pascalbetz]

I ran our application with your branch and this worked and our tests passed (whereas they fail on main branch)