From f1bdf9baa6458984c146cedfa7dfb2fa47315a28 Mon Sep 17 00:00:00 2001 From: Francis Bouvier Date: Fri, 10 Jul 2026 19:18:07 +0200 Subject: [PATCH] webapi: report listener exceptions to window.onerror MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes both failing tests in WPT /dom/events/Event-dispatch-throwing.html ("Throwing in event listener with a single/multiple listeners"): an exception thrown by an event listener must be reported to the global — firing window.onerror and an "error" event — while the dispatch continues with the remaining listeners. Lightpanda only logged the exception internally, so window.onerror never fired. - js.Function gains callWithThisRethrow, built on the existing callRethrow machinery (_tryCallWithThis with .rethrow = true): a thrown JS exception is surfaced as error.TryCatchRethrow past the internal TryCatch so an enclosing TryCatch of the caller can observe the exception value. js.TryCatch gains exceptionValue() returning the raw caught value. - Listener.run wraps function and handleEvent callbacks in a TryCatch and routes the caught exception value to Window.reportError (the existing "report the exception" implementation, which invokes onerror with the 5-argument form and dispatches the error event). This merges with the pre-existing _listeners_did_throw tracking on Event (used by IndexedDB): the reporting paths also set the flag. Worker globals are left as-is for now. - Window.reportError now implements the spec's "in error reporting mode" guard: an exception thrown while reporting one (e.g. by a throwing "error" listener) is not reported again, which would recurse without bound. Coverage: /dom/events/Event-dispatch-throwing.html 0/2 -> 2/2. No regressions across /dom/events (verified against a baseline build). Co-Authored-By: Claude Fable 5 --- src/browser/EventManagerBase.zig | 47 +++++++++++++++++++++++--------- src/browser/js/Function.zig | 8 ++++++ src/browser/js/TryCatch.zig | 6 ++++ src/browser/webapi/Window.zig | 10 +++++++ 4 files changed, 58 insertions(+), 13 deletions(-) diff --git a/src/browser/EventManagerBase.zig b/src/browser/EventManagerBase.zig index 60de091f6..0cd4c24ce 100644 --- a/src/browser/EventManagerBase.zig +++ b/src/browser/EventManagerBase.zig @@ -434,14 +434,20 @@ pub const Listener = struct { event: *Event, comptime context: []const u8, ) error{OutOfMemory}!void { - var caught: js.TryCatch.Caught = undefined; switch (self.function) { - .value => |value| local.toLocal(value).tryCallWithThis(void, event._current_target.?, .{event}, &caught) catch |err| { - if (err == error.JsException) { - event._listeners_did_throw = true; - } else { - log.warn(.event, context, .{ .err = err, .caught = caught }); - } + .value => |value| { + var try_catch: js.TryCatch = undefined; + try_catch.init(local); + defer try_catch.deinit(); + local.toLocal(value).callWithThisRethrow(void, event._current_target.?, .{event}) catch |err| switch (err) { + // The rethrow variant surfaces a thrown JS exception as + // TryCatchRethrow so our enclosing TryCatch holds it. + error.JsException, error.TryCatchRethrow => { + event._listeners_did_throw = true; + reportException(&try_catch, local); + }, + else => log.warn(.event, context, .{ .err = err }), + }; }, .string => |string| { const str = try arena.dupeZ(u8, string.str()); @@ -449,7 +455,7 @@ pub const Listener = struct { if (err == error.JsException) { event._listeners_did_throw = true; } else { - log.warn(.event, context, .{ .err = err, .caught = caught }); + log.warn(.event, context, .{ .err = err }); } }; }, @@ -465,12 +471,15 @@ pub const Listener = struct { break :blk null; }; if (handle_event) |handleEvent| { - handleEvent.tryCallWithThis(void, obj, .{event}, &caught) catch |err| { - if (err == error.JsException) { + var try_catch: js.TryCatch = undefined; + try_catch.init(local); + defer try_catch.deinit(); + handleEvent.callWithThisRethrow(void, obj, .{event}) catch |err| switch (err) { + error.JsException, error.TryCatchRethrow => { event._listeners_did_throw = true; - } else { - log.warn(.event, context, .{ .err = err, .caught = caught }); - } + reportException(&try_catch, local); + }, + else => log.warn(.event, context, .{ .err = err }), }; } else { // The listener was an object without the handleEvent function @@ -480,6 +489,18 @@ pub const Listener = struct { }, } } + + // Reports a listener exception to the relevant global (firing + // window.onerror / an "error" event) without stopping the dispatch. + fn reportException(try_catch: *js.TryCatch, local: *const js.Local) void { + const exc = try_catch.exceptionValue() orelse return; + switch (local.ctx.global) { + .frame => |frame| frame.window.reportError(exc, frame) catch |err| { + log.warn(.event, "listener report error", .{ .err = err }); + }, + .worker => {}, + } + } }; pub const Function = union(enum) { diff --git a/src/browser/js/Function.zig b/src/browser/js/Function.zig index ee9f2f606..49bf7101c 100644 --- a/src/browser/js/Function.zig +++ b/src/browser/js/Function.zig @@ -112,6 +112,14 @@ pub fn callWithThis(self: *const Function, comptime T: type, this: anytype, args }; } +// Like callWithThis, but a thrown JS exception is rethrown past the internal +// TryCatch, so an enclosing TryCatch of the caller can observe the exception +// value itself (e.g. to report it to window.onerror). +pub fn callWithThisRethrow(self: *const Function, comptime T: type, this: anytype, args: anytype) !T { + var caught: js.TryCatch.Caught = undefined; + return self._tryCallWithThis(T, this, args, &caught, .{ .rethrow = true }); +} + pub fn tryCall(self: *const Function, comptime T: type, args: anytype, caught: *js.TryCatch.Caught) !T { return self._tryCallWithThis(T, self.getThis(), args, caught, .{}); } diff --git a/src/browser/js/TryCatch.zig b/src/browser/js/TryCatch.zig index 0222e3c36..73681ed41 100644 --- a/src/browser/js/TryCatch.zig +++ b/src/browser/js/TryCatch.zig @@ -46,6 +46,12 @@ pub fn rethrow(self: *TryCatch) void { _ = v8.v8__TryCatch__ReThrow(&self.handle); } +// The raw caught exception value, e.g. to report it to a global's onerror. +pub fn exceptionValue(self: TryCatch) ?js.Value { + const handle = v8.v8__TryCatch__Exception(&self.handle) orelse return null; + return .{ .local = self.local, .handle = handle }; +} + pub fn caught(self: TryCatch, allocator: Allocator) ?Caught { if (self.hasCaught() == false) { return null; diff --git a/src/browser/webapi/Window.zig b/src/browser/webapi/Window.zig index f8f227516..e374d0168 100644 --- a/src/browser/webapi/Window.zig +++ b/src/browser/webapi/Window.zig @@ -89,6 +89,7 @@ _on_scroll: ?js.Function.Global = null, _on_message: ?js.Function.Global = null, _on_rejection_handled: ?js.Function.Global = null, _on_unhandled_rejection: ?js.Function.Global = null, +_reporting_error: bool = false, _current_event: ?*Event = null, _location: *Location, _timers: Timers = .{}, @@ -556,6 +557,15 @@ pub fn cancelIdleCallback(self: *Window, id: u32) void { } pub fn reportError(self: *Window, err: js.Value, frame: *Frame) !void { + // Per spec's "in error reporting mode": an exception thrown while an + // error is being reported (e.g. by an "error" listener) is not reported + // again, which would otherwise recurse without bound. + if (self._reporting_error) { + return; + } + self._reporting_error = true; + defer self._reporting_error = false; + const error_event = try ErrorEvent.initTrusted(comptime .wrap("error"), .{ .@"error" = try err.persist(), .message = err.toStringSlice() catch "Unknown error",