From 5440e3f625f1f68cd690e053d4367c3d1ae52199 Mon Sep 17 00:00:00 2001 From: Sam Date: Wed, 11 May 2016 17:58:33 +1000 Subject: [PATCH] FIX: rethrow ruby exceptions that you catch from js callbacks --- README.md | 23 +---------- ext/mini_racer_extension/extconf.rb | 4 +- .../mini_racer_extension.cc | 40 ++++++++++++++----- lib/mini_racer.rb | 1 + test/mini_racer_test.rb | 18 ++++++++- 5 files changed, 50 insertions(+), 36 deletions(-) diff --git a/README.md b/README.md index f4a06bb..d7a5be5 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,7 @@ Minimal, modern embedded V8 for Ruby. MiniRacer provides a minimal two way bridge between the V8 JavaScript engine and Ruby. -It was created as an alternative to the excellent [therubyracer](https://github.com/cowboyd/therubyracer). Unlike therubyracer, mini_racer only implements a minimal (yet complete) bridge. This reduces the surface area making upgrading v8 much simpler and exahustive testing simpler. +It was created as an alternative to the excellent [therubyracer](https://github.com/cowboyd/therubyracer). Unlike therubyracer, mini_racer only implements a minimal bridge. This reduces the surface area making upgrading v8 much simpler and exahustive testing simpler. ## Features @@ -100,57 +100,36 @@ Or install it yourself as: ###therubyracer - https://github.com/cowboyd/therubyracer - - Most comprehensive bridge available - - Provides the ability to "eval" JavaScript - - Provides the ability to invoke Ruby code from JavaScript - - Hold refrences to JavaScript objects and methods in your Ruby code - - Hold refrences to Ruby objects and methods in JavaScript code - - Uses libv8, so installation is fast - - Supports timeouts for JavaScript execution - - Does not release global interpreter lock, so performance is constrained to a single thread - - Currently (May 2016) only supports v8 version 3.16.14 (Released approx November 2013), plans to upgrade by July 2016 ###v8eval - https://github.com/sony/v8eval - - Provides the ability to "eval" JavaScript using the latest V8 engine - - Does not depend on the [libv8](https://github.com/cowboyd/libv8) gem, installation can take 10-20 mins as V8 needs to be downloaded and compiled. - - Does not release global interpreter lock when executing JavaScript - - Does not allow you to invoke Ruby code from JavaScript - - Multi runtime support due to SWIG based bindings - - Supports a JavaScript debugger - - Does not support timeouts for JavaScript execution ###therubyrhino - https://github.com/cowboyd/therubyrhino - - API compatible with therubyracer - - Uses Mozilla's Rhino engine https://github.com/mozilla/rhino - - Requires JRuby - - Support for timeouts for JavaScript execution - - Concurrent cause .... JRuby diff --git a/ext/mini_racer_extension/extconf.rb b/ext/mini_racer_extension/extconf.rb index 90e5eba..0956379 100644 --- a/ext/mini_racer_extension/extconf.rb +++ b/ext/mini_racer_extension/extconf.rb @@ -34,8 +34,8 @@ LIBV8_COMPATIBILITY = '~> 5.0.71.35.0' # # Libv8.configure_makefile -#NODE_PATH = "/home/sam/Source/libv8" -NODE_PATH = "/Users/sam/Source/libv8" +NODE_PATH = "/home/sam/Source/libv8" +#NODE_PATH = "/Users/sam/Source/libv8" # NODE_LIBS = NODE_PATH + "/vendor/v8/out/x64.release" NODE_INCLUDE = NODE_PATH + "/vendor/v8/include" diff --git a/ext/mini_racer_extension/mini_racer_extension.cc b/ext/mini_racer_extension/mini_racer_extension.cc index 5ef4d0c..11dbe20 100644 --- a/ext/mini_racer_extension/mini_racer_extension.cc +++ b/ext/mini_racer_extension/mini_racer_extension.cc @@ -240,13 +240,19 @@ static VALUE rb_context_eval_unsafe(VALUE self, VALUE str) { } if (!eval_result.executed) { - // exception report about what happened - if(TYPE(message) == T_STRING && TYPE(backtrace) == T_STRING) { - rb_raise(rb_eJavaScriptError, "%s/n%s", RSTRING_PTR(message), RSTRING_PTR(backtrace)); - } else if(TYPE(message) == T_STRING) { - rb_raise(rb_eJavaScriptError, "%s", RSTRING_PTR(message)); + + VALUE ruby_exception = rb_iv_get(self, "@current_exception"); + if (ruby_exception == Qnil) { + // exception report about what happened + if(TYPE(message) == T_STRING && TYPE(backtrace) == T_STRING) { + rb_raise(rb_eJavaScriptError, "%s/n%s", RSTRING_PTR(message), RSTRING_PTR(backtrace)); + } else if(TYPE(message) == T_STRING) { + rb_raise(rb_eJavaScriptError, "%s", RSTRING_PTR(message)); + } else { + rb_raise(rb_eJavaScriptError, "Unknown JavaScript Error during execution"); + } } else { - rb_raise(rb_eJavaScriptError, "Unknown JavaScript Error during execution"); + rb_raise(CLASS_OF(ruby_exception), RSTRING_PTR(rb_funcall(ruby_exception, rb_intern("to_s"), 0))); } } @@ -270,8 +276,10 @@ typedef struct { VALUE callback; int length; VALUE* args; + bool failed; } protected_callback_data; +static VALUE protected_callback(VALUE rdata) { protected_callback_data* data = (protected_callback_data*)rdata; VALUE result; @@ -284,6 +292,13 @@ VALUE protected_callback(VALUE rdata) { return result; } +static +VALUE rescue_callback(VALUE rdata, VALUE exception) { + protected_callback_data* data = (protected_callback_data*)rdata; + data->failed = true; + return exception; +} + void* gvl_ruby_callback(void* data) { @@ -292,13 +307,15 @@ gvl_ruby_callback(void* data) { int length = args->Length(); VALUE callback; VALUE result; + VALUE self; { HandleScope scope(args->GetIsolate()); Handle external = Handle::Cast(args->Data()); VALUE* self_pointer = (VALUE*)(external->Value()); - callback = rb_iv_get(*self_pointer, "@callback"); + self = *self_pointer; + callback = rb_iv_get(self, "@callback"); if (length > 0) { ruby_args = ALLOC_N(VALUE, length); @@ -312,15 +329,18 @@ gvl_ruby_callback(void* data) { } // may raise exception stay clear of handle scope - int state = 0; protected_callback_data callback_data; callback_data.length = length; callback_data.callback = callback; callback_data.args = ruby_args; + callback_data.failed = false; - result = rb_protect(protected_callback, (VALUE)(&callback_data), &state); + result = rb_rescue(protected_callback, (VALUE)(&callback_data), + rescue_callback, (VALUE)(&callback_data)); - if(state) { + if(callback_data.failed) { + VALUE parent = rb_iv_get(self, "@parent"); + rb_iv_set(parent, "@current_exception", result); args->GetIsolate()->ThrowException(String::NewFromUtf8(args->GetIsolate(), "Ruby exception")); } else { diff --git a/lib/mini_racer.rb b/lib/mini_racer.rb index 23d1026..a7cc554 100644 --- a/lib/mini_racer.rb +++ b/lib/mini_racer.rb @@ -48,6 +48,7 @@ module MiniRacer def eval(str) @lock.synchronize do + @current_exception = nil eval_unsafe(str) end end diff --git a/test/mini_racer_test.rb b/test/mini_racer_test.rb index f5c4140..b1c89e7 100644 --- a/test/mini_racer_test.rb +++ b/test/mini_racer_test.rb @@ -105,11 +105,25 @@ class MiniRacerTest < Minitest::Test assert_equal 10, context.eval("counter") end + class FooError < StandardError + def initialize(message) + super(message) + end + end + def test_attached_exceptions context = MiniRacer::Context.new - context.attach("adder", proc{raise StandardError}) + context.attach("adder", proc{ raise FooError, "I like foos" }) assert_raises do - context.eval('adder(1,2,3)') + begin +raise FooError, "I like foos" + context.eval('adder()') + rescue => e + assert_equal FooError, e.class + assert_match( /I like foos/, e.message) + # TODO backtrace splicing so js frames are injected + raise + end end end