From b573e44fe1ea231bae936dc25e7df3724cc440c7 Mon Sep 17 00:00:00 2001 From: Navid EMAD Date: Tue, 5 May 2026 18:16:52 +0200 Subject: [PATCH 1/2] events: report listener exceptions instead of halting dispatch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per DOM §2.9 step 4 substep 8 ("Inner invoke"), an exception thrown by a listener callback must be reported to the realm's global, not propagated out of `dispatchEvent`. Subsequent listeners on the same target and the rest of the capture/bubble walk must still run. Both `EventManager.dispatchPhase` (DOM targets) and `EventManagerBase.dispatchDirect` (Window/XHR/AbortSignal/etc.) wrapped the V8 callback invocation in raw `try`, so a listener throw aborted the whole dispatch and surfaced as an uncaught exception at the `Runtime.evaluate` boundary. The inline-handler invocation a few lines above already used `catch |err| log.warn(...)`; this just extends the same shape to the listener switch. Closes #2367 --- src/browser/EventManager.zig | 23 ++++++-- src/browser/EventManagerBase.zig | 22 ++++++-- src/browser/tests/events.html | 92 ++++++++++++++++++++++++++++++++ 3 files changed, 129 insertions(+), 8 deletions(-) diff --git a/src/browser/EventManager.zig b/src/browser/EventManager.zig index 77cb47b0..cd44a7e8 100644 --- a/src/browser/EventManager.zig +++ b/src/browser/EventManager.zig @@ -388,16 +388,31 @@ fn dispatchPhase(self: *EventManager, list: *std.DoublyLinkedList, current_targe event._target = getAdjustedTarget(original_target, current_target); } + // Per DOM §2.9 step 4 substep 8 ("Inner invoke"), a listener callback that + // throws must have its exception *reported* to the global error handler, + // not propagated to the dispatch caller — subsequent listeners on the same + // target and the rest of the propagation path must still run. Mirrors the + // catch on the inline-handler invocation in dispatchDirect. switch (listener.function) { - .value => |value| try local.toLocal(value).callWithThis(void, current_target, .{event}), + .value => |value| local.toLocal(value).callWithThis(void, current_target, .{event}) catch |err| { + log.warn(.event, "listener", .{ .err = err }); + }, .string => |string| { const str = try frame.call_arena.dupeZ(u8, string.str()); - try local.eval(str, null); + local.eval(str, null) catch |err| { + log.warn(.event, "listener string", .{ .err = err }); + }; }, .object => |obj_global| { const obj = local.toLocal(obj_global); - if (try obj.getFunction("handleEvent")) |handleEvent| { - try handleEvent.callWithThis(void, obj, .{event}); + const handle_event = obj.getFunction("handleEvent") catch |err| blk: { + log.warn(.event, "listener handleEvent", .{ .err = err }); + break :blk null; + }; + if (handle_event) |handleEvent| { + handleEvent.callWithThis(void, obj, .{event}) catch |err| { + log.warn(.event, "listener object", .{ .err = err }); + }; } }, } diff --git a/src/browser/EventManagerBase.zig b/src/browser/EventManagerBase.zig index 8e13ecd5..7ac622ae 100644 --- a/src/browser/EventManagerBase.zig +++ b/src/browser/EventManagerBase.zig @@ -305,16 +305,30 @@ pub fn dispatchDirect( event._current_target = target; + // Per DOM §2.9 step 4 substep 8 ("Inner invoke"), a listener callback that + // throws must have its exception *reported*, not propagated to the dispatch + // caller — subsequent listeners must still run. Same shape as the catch on + // the property-handler invocation above and on EventManager.dispatchPhase. switch (listener.function) { - .value => |value| try ls.local.toLocal(value).callWithThis(void, target, .{event}), + .value => |value| ls.local.toLocal(value).callWithThis(void, target, .{event}) catch |err| { + log.warn(.event, opts.context, .{ .err = err }); + }, .string => |string| { const str = try arena.dupeZ(u8, string.str()); - try ls.local.eval(str, null); + ls.local.eval(str, null) catch |err| { + log.warn(.event, opts.context, .{ .err = err }); + }; }, .object => |obj_global| { const obj = ls.local.toLocal(obj_global); - if (try obj.getFunction("handleEvent")) |handleEvent| { - try handleEvent.callWithThis(void, obj, .{event}); + const handle_event = obj.getFunction("handleEvent") catch |err| blk: { + log.warn(.event, opts.context, .{ .err = err }); + break :blk null; + }; + if (handle_event) |handleEvent| { + handleEvent.callWithThis(void, obj, .{event}) catch |err| { + log.warn(.event, opts.context, .{ .err = err }); + }; } }, } diff --git a/src/browser/tests/events.html b/src/browser/tests/events.html index 80d41707..eae7f883 100644 --- a/src/browser/tests/events.html +++ b/src/browser/tests/events.html @@ -762,3 +762,95 @@ testing.expectEqual(2, calls.length); } + +
+ + +
+ + + From bf7cac2b6807855e49abebd45ba222f24ed31b03 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Wed, 6 May 2026 09:07:38 +0800 Subject: [PATCH 2/2] Extract repeated listener switch/execution into helper --- src/browser/EventManager.zig | 35 +++------------- src/browser/EventManagerBase.zig | 68 +++++++++++++++++++------------- 2 files changed, 47 insertions(+), 56 deletions(-) diff --git a/src/browser/EventManager.zig b/src/browser/EventManager.zig index cd44a7e8..fa698403 100644 --- a/src/browser/EventManager.zig +++ b/src/browser/EventManager.zig @@ -266,7 +266,11 @@ fn dispatchNode(self: *EventManager, target: *Node, event: *Event, comptime opts was_handled = true; event._current_target = target_et; - try ls.toLocal(inline_handler).callWithThis(void, target_et, .{event}); + // Inline handlers (e.g. onclick property) follow the same "report, + // don't propagate" rule as addEventListener listeners — see Listener.run. + ls.toLocal(inline_handler).callWithThis(void, target_et, .{event}) catch |err| { + log.warn(.event, "inline handler", .{ .err = err }); + }; if (event._stop_propagation) { return; @@ -388,34 +392,7 @@ fn dispatchPhase(self: *EventManager, list: *std.DoublyLinkedList, current_targe event._target = getAdjustedTarget(original_target, current_target); } - // Per DOM §2.9 step 4 substep 8 ("Inner invoke"), a listener callback that - // throws must have its exception *reported* to the global error handler, - // not propagated to the dispatch caller — subsequent listeners on the same - // target and the rest of the propagation path must still run. Mirrors the - // catch on the inline-handler invocation in dispatchDirect. - switch (listener.function) { - .value => |value| local.toLocal(value).callWithThis(void, current_target, .{event}) catch |err| { - log.warn(.event, "listener", .{ .err = err }); - }, - .string => |string| { - const str = try frame.call_arena.dupeZ(u8, string.str()); - local.eval(str, null) catch |err| { - log.warn(.event, "listener string", .{ .err = err }); - }; - }, - .object => |obj_global| { - const obj = local.toLocal(obj_global); - const handle_event = obj.getFunction("handleEvent") catch |err| blk: { - log.warn(.event, "listener handleEvent", .{ .err = err }); - break :blk null; - }; - if (handle_event) |handleEvent| { - handleEvent.callWithThis(void, obj, .{event}) catch |err| { - log.warn(.event, "listener object", .{ .err = err }); - }; - } - }, - } + try listener.run(frame.call_arena, local, event, "listener"); // Restore original target (only if we changed it) if (event._needs_retargeting) { diff --git a/src/browser/EventManagerBase.zig b/src/browser/EventManagerBase.zig index 7ac622ae..03bb6d15 100644 --- a/src/browser/EventManagerBase.zig +++ b/src/browser/EventManagerBase.zig @@ -305,33 +305,7 @@ pub fn dispatchDirect( event._current_target = target; - // Per DOM §2.9 step 4 substep 8 ("Inner invoke"), a listener callback that - // throws must have its exception *reported*, not propagated to the dispatch - // caller — subsequent listeners must still run. Same shape as the catch on - // the property-handler invocation above and on EventManager.dispatchPhase. - switch (listener.function) { - .value => |value| ls.local.toLocal(value).callWithThis(void, target, .{event}) catch |err| { - log.warn(.event, opts.context, .{ .err = err }); - }, - .string => |string| { - const str = try arena.dupeZ(u8, string.str()); - ls.local.eval(str, null) catch |err| { - log.warn(.event, opts.context, .{ .err = err }); - }; - }, - .object => |obj_global| { - const obj = ls.local.toLocal(obj_global); - const handle_event = obj.getFunction("handleEvent") catch |err| blk: { - log.warn(.event, opts.context, .{ .err = err }); - break :blk null; - }; - if (handle_event) |handleEvent| { - handleEvent.callWithThis(void, obj, .{event}) catch |err| { - log.warn(.event, opts.context, .{ .err = err }); - }; - } - }, - } + try listener.run(arena, &ls.local, event, opts.context); if (event._stop_immediate_propagation) { return; @@ -406,6 +380,46 @@ pub const Listener = struct { signal: ?*@import("webapi/AbortSignal.zig") = null, node: std.DoublyLinkedList.Node, removed: bool = false, + + // Per DOM §2.9 step 4 substep 8 ("Inner invoke"), a listener callback that + // throws must have its exception *reported* to the global error handler, + // not propagated to the dispatch caller — subsequent listeners on the same + // target and the rest of the propagation path must still run. + // + // Caller must set `event._current_target` before invoking — the function- + // listener variant uses it as `this`, matching the spec contract that a + // listener sees its current target via both `event.currentTarget` and `this`. + pub fn run( + self: *const Listener, + arena: Allocator, + local: *const js.Local, + event: *Event, + comptime context: []const u8, + ) error{OutOfMemory}!void { + switch (self.function) { + .value => |value| local.toLocal(value).callWithThis(void, event._current_target.?, .{event}) catch |err| { + log.warn(.event, context, .{ .err = err }); + }, + .string => |string| { + const str = try arena.dupeZ(u8, string.str()); + local.eval(str, null) catch |err| { + log.warn(.event, context, .{ .err = err }); + }; + }, + .object => |obj_global| { + const obj = local.toLocal(obj_global); + const handle_event = obj.getFunction("handleEvent") catch |err| blk: { + log.warn(.event, context, .{ .err = err }); + break :blk null; + }; + if (handle_event) |handleEvent| { + handleEvent.callWithThis(void, obj, .{event}) catch |err| { + log.warn(.event, context, .{ .err = err }); + }; + } + }, + } + } }; pub const Function = union(enum) {