From f3d896ac61773f47d37c56038f9ad7088572ba9b Mon Sep 17 00:00:00 2001 From: Dmitry Vorotilin Date: Fri, 23 Oct 2026 10:06:26 +0300 Subject: [PATCH] fix(runtime): raise JavaScriptError for any thrown value, not only Error instances handle_error looked at the result's subtype instead of exceptionDetails, so throwing a string or an object returned it as the result, while returning an Error object crashed with NoMethodError. JavaScriptError's message includes the thrown primitive, e.g. "Uncaught smth" instead of just "Uncaught". --- CHANGELOG.md | 2 ++ lib/ferrum/errors.rb | 2 +- lib/ferrum/frame/runtime.rb | 8 ++++---- spec/frame/runtime_spec.rb | 35 +++++++++++++++++++++++++++++++++++ 4 files changed, 42 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9aae1162..22c5b281 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,8 @@ `429 Too Many Requests` for a full remote browser service, and exposes it as `#status` ### Fixed +- `evaluate`, `execute` and friends raise `JavaScriptError` when JS throws something other than an `Error`, e.g. + `throw "smth"`, instead of returning it. Returning an `Error` object no longer crashes with `NoMethodError` - `workers` skips detached workers, `target.worker` with no session raises `NoSuchTargetError` instead of a CDP error - `attach_target` reuses a target's session and returns the target, instead of opening one more that reruns extensions - Service workers registered by a page stayed attached and alive for the browser's lifetime, now Chrome can stop them diff --git a/lib/ferrum/errors.rb b/lib/ferrum/errors.rb index 82e35ba9..298db890 100644 --- a/lib/ferrum/errors.rb +++ b/lib/ferrum/errors.rb @@ -216,7 +216,7 @@ class JavaScriptError < BrowserError def initialize(response) @class_name, @message = response["exception"]&.values_at("className", "description") - @message ||= response["text"] + @message ||= [response["text"], response.dig("exception", "value")].compact.join(" ") @stack_trace = response["stackTrace"] super(response.merge("message" => @message)) end diff --git a/lib/ferrum/frame/runtime.rb b/lib/ferrum/frame/runtime.rb index 535e9123..993a92f2 100644 --- a/lib/ferrum/frame/runtime.rb +++ b/lib/ferrum/frame/runtime.rb @@ -171,14 +171,14 @@ def call(expression:, arguments: [], on: nil, wait: 0, handle: true, **options) # FIXME: We should have a central place to handle all type of errors def handle_error(response) - result = response["result"] - return if result["subtype"] != "error" + details = response["exceptionDetails"] + return unless details - case result["description"] + case details.dig("exception", "description") when /\AError: timed out promise/ raise ScriptTimeoutError else - raise JavaScriptError, response["exceptionDetails"] + raise JavaScriptError, details end end diff --git a/spec/frame/runtime_spec.rb b/spec/frame/runtime_spec.rb index 2cdde169..4b41718d 100644 --- a/spec/frame/runtime_spec.rb +++ b/spec/frame/runtime_spec.rb @@ -2,6 +2,10 @@ describe Ferrum::Frame::Runtime do describe "#execute" do + it "raises when a string is thrown" do + expect { browser.execute(%(throw "smth")) }.to raise_error(Ferrum::JavaScriptError, "Uncaught smth") + end + it "executes multiple lines of javascript" do browser.execute <<-JS var a = 1 @@ -128,6 +132,22 @@ end.to raise_error(Ferrum::JavaScriptError) end + it "raises when a string is thrown" do + expect do + browser.evaluate(%((() => { throw "smth" })())) + end.to raise_error(Ferrum::JavaScriptError, "Uncaught smth") + end + + it "raises when an object is thrown" do + expect do + browser.evaluate("(() => { throw { code: 1 } })()") + end.to raise_error(Ferrum::JavaScriptError) { |e| expect(e.class_name).to eq("Object") } + end + + it "returns an Error object instead of raising" do + expect(browser.evaluate(%(new Error("smth")))).to eq({}) + end + context "when cyclic structure is returned" do context "ignores seen" do let(:code) do @@ -219,9 +239,24 @@ browser.evaluate_async("var callback=arguments[0]; setTimeout(function(){callback(true)}, 4000)", 1) end.to raise_error(Ferrum::ScriptTimeoutError) end + + it "raises when a string is thrown" do + expect do + browser.evaluate_async(%(throw "smth"), 5) + end.to raise_error(Ferrum::JavaScriptError, "Uncaught (in promise) smth") + end end describe "#evaluate_on" do + it "raises when a string is thrown" do + browser.go_to("/table") + node = browser.at_xpath(".//td") + + expect do + browser.evaluate_on(node: node, expression: %((() => { throw "smth" })())) + end.to raise_error(Ferrum::JavaScriptError, "Uncaught smth") + end + it "does not retry if node is not around anymore" do stub_const("Ferrum::Frame::Runtime::INTERMITTENT_SLEEP", 5)