From d1890c4fd2bf22dd7aab92be43e1a937e1a9cad9 Mon Sep 17 00:00:00 2001 From: Arpit Jain Date: Sat, 6 Jun 2026 16:28:25 +0900 Subject: [PATCH 1/2] Include runtime stderr in error messages 5cce03a stopped folding stderr into stdout to keep Bun's .env loading noise out of the parsed output (#130). A side effect was that when the runtime exits non-zero, the diagnostic it writes to stderr (parse errors, stack traces) is dropped, so the raised ExecJS::RuntimeError carries no useful detail (#142). Capture stderr separately by redirecting the child's stderr to a temp file (a file, not a pipe, so there is no risk of a deadlock between draining stdout and stderr), and include it in the error only on the failure path. The success path still reads stdout exactly as before, so #130 stays fixed. exec_runtime_error takes an optional stderr argument and is backwards compatible, so the Windows branch (which uses its own redirect) is unaffected. The two IO.popen branches that 5cce03a changed (default and JRuby) are the ones updated here. Adds a regression test that a non-zero runtime exit surfaces its stderr. Fixes #142 Signed-off-by: Arpit Jain --- lib/execjs/external_runtime.rb | 48 ++++++++++++++++++++++++---------- test/test_execjs.rb | 12 +++++++++ 2 files changed, 46 insertions(+), 14 deletions(-) diff --git a/lib/execjs/external_runtime.rb b/lib/execjs/external_runtime.rb index a0325f4..47b50b9 100644 --- a/lib/execjs/external_runtime.rb +++ b/lib/execjs/external_runtime.rb @@ -194,36 +194,56 @@ def shell_escape(*args) require 'shellwords' def exec_runtime(filename) - command = "#{Shellwords.join(binary.split(' ') << filename)}" - io = IO.popen(command, **@popen_options) - output = io.read - io.close + stderr_path = Dir::Tmpname.create(['execjs', 'stderr']) {} + begin + command = "#{Shellwords.join(binary.split(' ') << filename)}" + io = IO.popen(command, err: stderr_path, **@popen_options) + output = io.read + io.close + status = $? + stderr = File.file?(stderr_path) ? File.read(stderr_path) : "" + ensure + File.unlink(stderr_path) if stderr_path && File.exist?(stderr_path) + end - if $?.success? + if status.success? output else - raise exec_runtime_error(output) + raise exec_runtime_error(output, stderr) end end else def exec_runtime(filename) - io = IO.popen(binary.split(' ') << filename, **@popen_options) - output = io.read - io.close + stderr_path = Dir::Tmpname.create(['execjs', 'stderr']) {} + begin + io = IO.popen(binary.split(' ') << filename, err: stderr_path, **@popen_options) + output = io.read + io.close + status = $? + stderr = File.file?(stderr_path) ? File.read(stderr_path) : "" + ensure + File.unlink(stderr_path) if stderr_path && File.exist?(stderr_path) + end - if $?.success? + if status.success? output else - raise exec_runtime_error(output) + raise exec_runtime_error(output, stderr) end end end # Internally exposed for Context. public :exec_runtime - def exec_runtime_error(output) - error = RuntimeError.new(output) - lines = output.split("\n") + def exec_runtime_error(output, stderr = nil) + message = output.to_s + unless stderr.nil? || stderr.empty? + extra = stderr.dup.force_encoding(message.encoding) + extra = extra.scrub if extra.respond_to?(:scrub) && !extra.valid_encoding? + message = message.empty? ? extra : "#{message}\n#{extra}" + end + error = RuntimeError.new(message) + lines = message.split("\n") lineno = lines[0][/:(\d+)$/, 1] if lines[0] lineno ||= 1 error.set_backtrace(["(execjs):#{lineno}"] + caller) diff --git a/test/test_execjs.rb b/test/test_execjs.rb index f2a5e23..64b51cb 100644 --- a/test/test_execjs.rb +++ b/test/test_execjs.rb @@ -67,6 +67,18 @@ def test_call_with_env_file end end + def test_runtime_error_includes_stderr + # Regression for #142: when the runtime exits non-zero it writes its + # diagnostic to stderr (here, a parse error). 5cce03a stopped capturing + # stderr, so the raised error lost that trace; it should be surfaced again. + # The diagnostic text is Node-style, so guard to Node-family runtimes. + skip unless ExecJS.runtime.name.to_s =~ /Node|Bun|Deno/i + err = assert_raises(ExecJS::RuntimeError) do + ExecJS.exec("?? not valid javascript ??") + end + assert err.message =~ /SyntaxError/, err.message + end + def test_call_with_this # Known bug: https://github.com/cowboyd/therubyrhino/issues/39 skip if ExecJS.runtime.is_a?(ExecJS::RubyRhinoRuntime) From 25a87b671aac4edc62276862a55d5eba6d35c6e8 Mon Sep 17 00:00:00 2001 From: Arpit Jain Date: Wed, 10 Jun 2026 10:02:42 +0900 Subject: [PATCH 2/2] test: limit stderr regression test to Node (Bun formats parse errors differently) Signed-off-by: Arpit Jain --- test/test_execjs.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/test_execjs.rb b/test/test_execjs.rb index 64b51cb..703710f 100644 --- a/test/test_execjs.rb +++ b/test/test_execjs.rb @@ -71,8 +71,10 @@ def test_runtime_error_includes_stderr # Regression for #142: when the runtime exits non-zero it writes its # diagnostic to stderr (here, a parse error). 5cce03a stopped capturing # stderr, so the raised error lost that trace; it should be surfaced again. - # The diagnostic text is Node-style, so guard to Node-family runtimes. - skip unless ExecJS.runtime.name.to_s =~ /Node|Bun|Deno/i + # The stderr-capture fix is runtime-agnostic, but the diagnostic wording + # varies per engine (Node says "SyntaxError", Bun differs), so this asserts + # on Node where the format is stable. + skip unless ExecJS.runtime.name.to_s =~ /Node/i err = assert_raises(ExecJS::RuntimeError) do ExecJS.exec("?? not valid javascript ??") end