JS code can now catch Ruby exceptions (#373)

Before this commit exceptions from Ruby callbacks were translated to
termination exceptions that cannot be caught by JS code. Turn them
into regular exceptions that can be caught.

Fixes: #357
Co-authored-by: Benoit Daloze <[email protected]>

Author: bnoordhuis

ext/mini_racer_extension/mini_racer_extension.c

@@ -13,6 +13,13 @@
 #include "serde.c"
 #include "mini_racer_v8.h"
 
+// for debugging
+#define RB_PUTS(v)                                          \
+    do {                                                    \
+        fflush(stdout);                                     \
+        rb_funcall(rb_mKernel, rb_intern("puts"), 1, v);    \
+    } while (0)
+
 #if RUBY_API_VERSION_CODE < 3*10000+4*100 // 3.4.0
 static inline void rb_thread_lock_native_thread(void)
 {
@@ -154,6 +161,7 @@ static VALUE platform_init_error;
 static VALUE context_disposed_error;
 static VALUE parse_error;
 static VALUE memory_error;
+static VALUE script_error;
 static VALUE runtime_error;
 static VALUE internal_error;
 static VALUE snapshot_error;
@@ -482,12 +490,36 @@ static void des_object_ref(void *arg, uint32_t id)
 
 static void des_error_begin(void *arg)
 {
-    push(arg, rb_class_new_instance(0, NULL, rb_eRuntimeError));
+    push(arg, rb_ary_new());
 }
 
 static void des_error_end(void *arg)
 {
-    pop(arg);
+    VALUE *a, h, message, stack, cause, newline;
+    DesCtx *c;
+
+    c = arg;
+    if (*c->err)
+        return;
+    if (c->tos == c->stack) {
+        snprintf(c->err, sizeof(c->err), "stack underflow");
+        return;
+    }
+    a = &c->tos->a;
+    h = rb_ary_pop(*a);
+    message = rb_hash_aref(h, rb_str_new_cstr("message"));
+    stack = rb_hash_aref(h, rb_str_new_cstr("stack"));
+    cause = rb_hash_aref(h, rb_str_new_cstr("cause"));
+    if (NIL_P(message))
+        message = rb_str_new_cstr("JS exception");
+    if (!NIL_P(stack)) {
+        newline = rb_str_new_cstr("\n");
+        message = rb_funcall(message, rb_intern("concat"), 2, newline, stack);
+    }
+    *a = rb_class_new_instance(1, &message, script_error);
+    if (!NIL_P(cause))
+        rb_iv_set(*a, "@cause", cause);
+    pop(c);
 }
 
 static int collect(VALUE k, VALUE v, VALUE a)
@@ -894,6 +926,7 @@ static VALUE rendezvous_callback_do(VALUE arg)
 static void *rendezvous_callback(void *arg)
 {
     struct rendezvous_nogvl *a;
+    const char *err;
     Context *c;
     int exc;
     VALUE r;
@@ -917,7 +950,12 @@ static void *rendezvous_callback(void *arg)
     buf_move(&s.b, a->req);
     return NULL;
 fail:
-    ser_init1(&s, 'e'); // exception pending
+    ser_init0(&s);   // ruby exception pending
+    w_byte(&s, 'e'); // send ruby error message to v8 thread
+    r = rb_funcall(c->exception, rb_intern("to_s"), 0);
+    err = StringValueCStr(r);
+    if (err)
+        w(&s, err, strlen(err));
     goto out;
 }
 
@@ -975,18 +1013,20 @@ static VALUE rendezvous1(Context *c, Buf *req, DesCtx *d)
     int exc;
 
     rendezvous_no_des(c, req, &res); // takes ownership of |req|
+    r = c->exception;
+    c->exception = Qnil;
+    // if js land didn't handle exception from ruby callback, re-raise it now
+    if (res.len == 1 && *res.buf == 'e') {
+        assert(!NIL_P(r));
+        rb_exc_raise(r);
+    }
     r = rb_protect(deserialize, (VALUE)&(struct rendezvous_des){d, &res}, &exc);
     buf_reset(&res);
     if (exc) {
         r = rb_errinfo();
         rb_set_errinfo(Qnil);
         rb_exc_raise(r);
     }
-    if (!NIL_P(c->exception)) {
-        r = c->exception;
-        c->exception = Qnil;
-        rb_exc_raise(r);
-    }
     return r;
 }
 
@@ -1634,6 +1674,11 @@ static VALUE snapshot_size0(VALUE self)
     return LONG2FIX(RSTRING_LENINT(ss->blob));
 }
 
+static VALUE script_error_cause(VALUE self)
+{
+    return rb_iv_get(self, "@cause");
+}
+
 __attribute__((visibility("default")))
 void Init_mini_racer_extension(void)
 {
@@ -1648,10 +1693,13 @@ void Init_mini_racer_extension(void)
     c = rb_define_class_under(m, "EvalError", c);
     parse_error = rb_define_class_under(m, "ParseError", c);
     memory_error = rb_define_class_under(m, "V8OutOfMemoryError", c);
+    script_error = rb_define_class_under(m, "ScriptError", c);
     runtime_error = rb_define_class_under(m, "RuntimeError", c);
     internal_error = rb_define_class_under(m, "InternalError", c);
     terminated_error = rb_define_class_under(m, "ScriptTerminatedError", c);
 
+    rb_define_method(script_error, "cause", script_error_cause, 0);
+
     c = context_class = rb_define_class_under(m, "Context", rb_cObject);
     rb_define_method(c, "initialize", context_initialize, -1);
     rb_define_method(c, "attach", context_attach, 2);

ext/mini_racer_extension/mini_racer_v8.cc

@@ -86,6 +86,7 @@ struct State
     v8::Persistent<v8::Context> persistent_context;         // single-thread mode only
     v8::Persistent<v8::Context> persistent_safe_context;    // single-thread mode only
     v8::Persistent<v8::Function> persistent_safe_context_function;  // single-thread mode only
+    v8::Persistent<v8::Value> ruby_exception;
     Context *ruby_context;
     int64_t max_memory;
     int err_reason;
@@ -122,6 +123,20 @@ struct Serialized
     }
 };
 
+bool bubble_up_ruby_exception(State& st, v8::TryCatch *try_catch)
+{
+    auto exception = try_catch->Exception();
+    if (exception.IsEmpty()) return false;
+    auto ruby_exception = v8::Local<v8::Value>::New(st.isolate, st.ruby_exception);
+    if (ruby_exception.IsEmpty()) return false;
+    if (!ruby_exception->SameValue(exception)) return false;
+    // signal that the ruby thread should reraise the exception
+    // that it caught earlier when executing a js->ruby callback
+    uint8_t c = 'e';
+    v8_reply(st.ruby_context, &c, 1);
+    return true;
+}
+
 // throws JS exception on serialization error
 bool reply(State& st, v8::Local<v8::Value> v)
 {
@@ -388,8 +403,17 @@ 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
+            v8::Local<v8::String> message;
+            auto type = v8::NewStringType::kNormal;
+            if (!v8::String::NewFromOneByte(st.isolate, p+1, type, n-1).ToLocal(&message)) {
+                message = v8::String::NewFromUtf8Literal(st.isolate, "Ruby exception");
+            }
+            auto exception = v8::Exception::Error(message);
+            st.ruby_exception.Reset(st.isolate, exception);
+            st.isolate->ThrowException(exception);
+            return;
+        }
         v8_dispatch(st.ruby_context);
     }
     v8::ValueDeserializer des(st.isolate, p+1, n-1);
@@ -523,6 +547,7 @@ extern "C" void v8_call(State *pst, const uint8_t *p, size_t n)
         cause = st.err_reason ? st.err_reason : TERMINATED_ERROR;
         st.err_reason = NO_ERROR;
     }
+    if (bubble_up_ruby_exception(st, &try_catch)) return;
     if (!cause && try_catch.HasCaught()) cause = RUNTIME_ERROR;
     if (cause) result = v8::Undefined(st.isolate);
     auto err = to_error(st, &try_catch, cause);
@@ -571,6 +596,7 @@ extern "C" void v8_eval(State *pst, const uint8_t *p, size_t n)
         cause = st.err_reason ? st.err_reason : TERMINATED_ERROR;
         st.err_reason = NO_ERROR;
     }
+    if (bubble_up_ruby_exception(st, &try_catch)) return;
     if (!cause && try_catch.HasCaught()) cause = RUNTIME_ERROR;
     if (cause) result = v8::Undefined(st.isolate);
     auto err = to_error(st, &try_catch, cause);
@@ -895,6 +921,7 @@ State::~State()
         v8::Isolate::Scope isolate_scope(isolate);
         persistent_safe_context.Reset();
         persistent_context.Reset();
+        ruby_exception.Reset();
     }
     isolate->Dispose();
     for (Callback *cb : callbacks)

ext/mini_racer_extension/serde.c

@@ -194,17 +194,21 @@ static inline int r_zigzag(const uint8_t **p, const uint8_t *pe, int64_t *r)
     return 0;
 }
 
-static inline void ser_init(Ser *s)
+static void ser_init0(Ser *s)
 {
     memset(s, 0, sizeof(*s));
     buf_init(&s->b);
+}
+
+static inline void ser_init(Ser *s)
+{
+    ser_init0(s);
     w(s, "\xFF\x0F", 2);
 }
 
 static void ser_init1(Ser *s, uint8_t c)
 {
-    memset(s, 0, sizeof(*s));
-    buf_init(&s->b);
+    ser_init0(s);
     w_byte(s, c);
     w(s, "\xFF\x0F", 2);
 }

lib/mini_racer.rb

@@ -51,6 +51,19 @@ def backtrace
     end
   end
 
+  class ScriptError < EvalError
+    def initialize(message)
+      message, *@frames = message.split("\n")
+      @frames.map! { "JavaScript #{_1.strip}" }
+      super(message)
+    end
+
+    def backtrace
+      frames = super || []
+      @frames + frames
+    end
+  end
+
   class SnapshotError < Error
     def initialize(message)
       message, *@frames = message.split("\n")

lib/mini_racer/truffleruby.rb

@@ -246,6 +246,10 @@ def convert_js_to_ruby(value)
           js_map_to_hash(value)
         elsif map_iterator?(value)
           value.map { |e| convert_js_to_ruby(e) }
+        elsif Polyglot::ForeignException === value
+          exc = MiniRacer::ScriptError.new(value.message)
+          exc.set_backtrace(value.backtrace)
+          exc
         else
           object = value
           h = {}

test/mini_racer_test.rb

@@ -1149,6 +1149,21 @@ def test_termination_exception
     b.kill
   end
 
+  def test_ruby_exception
+    context = MiniRacer::Context.new
+    context.attach("test", proc { raise "boom" })
+    line = __LINE__ - 1
+    actual = context.eval("try { test() } catch (e) { e }")
+    assert_equal(actual.class, MiniRacer::ScriptError)
+    assert_equal(actual.message, "boom")
+    if RUBY_ENGINE == "truffleruby"
+      assert_includes(actual.backtrace[0], "#{__FILE__}:#{line}")
+      assert_includes(actual.backtrace[0], "block in MiniRacerTest#test_ruby_exception")
+    else
+      assert_equal(actual.backtrace, ["JavaScript Error: boom", "JavaScript at <eval>:1:7"])
+    end
+  end
+
   def test_large_integer
     [10_000_000_001, -2**63, 2**63-1].each { |big_int|
       context = MiniRacer::Context.new