Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: (WIP) Improve error marshaling from JS to Ruby #37

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ sudo: false
before_install:
- if [ "$EXECJS_RUNTIME" == "V8" ]; then brew update; fi
- if [ "$EXECJS_RUNTIME" == "V8" ]; then brew install v8; fi
script: bundle exec ruby test/test_execjs.rb
script:
- node --version
- bundle exec ruby test/test_execjs.rb

matrix:
include:
Expand Down
1 change: 1 addition & 0 deletions lib/execjs/duktape_runtime.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ def wrap_error(e)
re = / \(line (\d+)\)$/
lineno = e.message[re, 1] || 1
error = klass.new(e.message.sub(re, ""))
error.metadata = {} # FIXME: has to be available upstream
error.set_backtrace(["(execjs):#{lineno}"] + e.backtrace)
error
else
Expand Down
16 changes: 12 additions & 4 deletions lib/execjs/external_runtime.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,12 @@ def write_to_tempfile(contents)
end

def extract_result(output, filename)
status, value, stack = output.empty? ? [] : ::JSON.parse(output, create_additions: false)
status, value= output.empty? ? [] : ::JSON.parse(output, create_additions: false)
if status == "ok"
value
else
stack ||= ""
demarshaled_err = value
stack = demarshaled_err['stack'] || ""
real_filename = File.realpath(filename)
stack = stack.split("\n").map do |line|
line.sub(" at ", "")
Expand All @@ -79,8 +80,11 @@ def extract_result(output, filename)
end
stack.reject! { |line| ["eval code", "eval@[native code]"].include?(line) }
stack.shift unless stack[0].to_s.include?("(execjs)")
error_class = value =~ /SyntaxError:/ ? RuntimeError : ProgramError
error = error_class.new(value)

error_class = demarshaled_err['name'].to_s =~ /SyntaxError/ ? RuntimeError : ProgramError

error = error_class.new(demarshaled_err.delete('message').to_s)
error.metadata = demarshaled_err
error.set_backtrace(stack + caller)
raise error
end
Expand Down Expand Up @@ -156,6 +160,10 @@ def json2_source
@json2_source ||= IO.read(ExecJS.root + "/support/json2.js")
end

def encode_error_source
@encode_error_source ||= IO.read(ExecJS.root + "/support/encode_error.js")
end

def encode_source(source)
encoded_source = encode_unicode_codepoints(source)
::JSON.generate("(function(){ #{encoded_source} })()", quirks_mode: true)
Expand Down
13 changes: 11 additions & 2 deletions lib/execjs/module.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,17 @@

module ExecJS
class Error < ::StandardError; end
class RuntimeError < Error; end
class ProgramError < Error; end

class RuntimeError < Error
# Stores the unmarshaled JavaScript error information if it was available (type, message etc.)
attr_accessor :metadata
end

class ProgramError < Error
# Stores the unmarshaled JavaScript error information if it was available (type, message etc.)
attr_accessor :metadata
end

class RuntimeUnavailable < RuntimeError; end

class << self
Expand Down
12 changes: 12 additions & 0 deletions lib/execjs/support/encode_error.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
function encodeError(err) {
var errMeta = {type: typeof(err), name: null, message: '' + err, stack: null};
if (typeof(err) === 'object') {
// Copy "message" and "name" since those are standard attributes for an Error
if (typeof(err.name) !== 'undefined') errMeta.name = err.name;
if (typeof(err.message) !== 'undefined') errMeta.message = err.message;
if (typeof(err.stack) !== 'undefined') errMeta.stack = err.stack;
// Copy all of the other properties that are tacked on the Error itself
if (typeof(Object.assign) === 'function') Object.assign(errMeta, err);
}
return errMeta;
};
5 changes: 3 additions & 2 deletions lib/execjs/support/jsc_runner.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
(function(program, execJS) { execJS(program) })(function() { #{source}
}, function(program) {
#{encode_error_source}
var output;
try {
result = program();
Expand All @@ -9,10 +10,10 @@
try {
print(JSON.stringify(['ok', result]));
} catch (err) {
print(JSON.stringify(['err', '' + err, err.stack]));
print(JSON.stringify(['err', encodeError(err)]));
}
}
} catch (err) {
print(JSON.stringify(['err', '' + err, err.stack]));
print(JSON.stringify(['err', encodeError(err)]));
}
});
5 changes: 3 additions & 2 deletions lib/execjs/support/jscript_runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
return eval(#{encode_source(source)});
}, function(program) {
#{json2_source}
#{encode_error_source}
var output, print = function(string) {
WScript.Echo(string);
};
Expand All @@ -13,10 +14,10 @@
try {
print(JSON.stringify(['ok', result]));
} catch (err) {
print(JSON.stringify(['err', err.name + ': ' + err.message, err.stack]));
print(JSON.stringify(['err', encodeError(err)]));
}
}
} catch (err) {
print(JSON.stringify(['err', err.name + ': ' + err.message, err.stack]));
print(JSON.stringify(['err', encodeError(err)]));
}
});
5 changes: 3 additions & 2 deletions lib/execjs/support/node_runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
var output, print = function(string) {
process.stdout.write('' + string);
};
#{encode_error_source}
try {
result = program();
if (typeof result == 'undefined' && result !== null) {
Expand All @@ -11,10 +12,10 @@
try {
print(JSON.stringify(['ok', result]));
} catch (err) {
print(JSON.stringify(['err', '' + err, err.stack]));
print(JSON.stringify(['err', encodeError(err)]));
}
}
} catch (err) {
print(JSON.stringify(['err', '' + err, err.stack]));
print(JSON.stringify(['err', encodeError(err)]));
}
});
5 changes: 3 additions & 2 deletions lib/execjs/support/spidermonkey_runner.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
(function(program, execJS) { execJS(program) })(function() { #{source}
}, function(program) {
#{encode_error_source}
var output;
try {
result = program();
Expand All @@ -9,10 +10,10 @@
try {
print(JSON.stringify(['ok', result]));
} catch (err) {
print(JSON.stringify(['err', '' + err, err.stack]));
print(JSON.stringify(['err', encodeError(err)]));
}
}
} catch (err) {
print(JSON.stringify(['err', '' + err, err.stack]));
print(JSON.stringify(['err', encodeError(err)]));
}
});
5 changes: 3 additions & 2 deletions lib/execjs/support/v8_runner.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
(function(program, execJS) { execJS(program) })(function() { #{source}
}, function(program) {
#{encode_error_source}
var output;
try {
result = program();
Expand All @@ -9,10 +10,10 @@
try {
print(JSON.stringify(['ok', result]));
} catch (err) {
print(JSON.stringify(['err', '' + err, err.stack]));
print(JSON.stringify(['err', encodeError(err)]));
}
}
} catch (err) {
print(JSON.stringify(['err', '' + err, err.stack]));
print(JSON.stringify(['err', encodeError(err)]));
}
});
24 changes: 24 additions & 0 deletions test/test_execjs.rb
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,30 @@ def test_exec_syntax_error
end
end

def test_exec_custom_js_error
begin
ExecJS.exec('CustomError = function(message, line, column) {
this.name = "CustomError";
this.message = message;
this.line = line;
this.column = column;
this.specialProperty = 123;
};
CustomError.prototype = Error.prototype;
throw new CustomError("this has failed", 10, 15)
')
flunk
rescue ExecJS::ProgramError => e
assert e
assert_match /this has failed/, e.message

meta = e.metadata
assert_equal 'object', meta['type']
assert_equal 'CustomError', meta['name']
assert_equal 123, meta['specialProperty']
end
end

def test_eval_syntax_error
begin
ExecJS.eval(")")
Expand Down