Skip to content

Commit b19476f

Browse files
ursmclaude
andcommitted
fix(events): report a throwing event listener instead of swallowing it (−2)
Per DOM "inner invoke", when an event listener callback throws, the exception is REPORTED (HTML "report the exception" — a window `error` event + legacy `onerror`), not propagated to the dispatchEvent() caller, and dispatch continues to the next listener. `fireListeners` was catching a callback throw with `logThrew` (logged + swallowed), so `window.onerror` never saw it. Route the throw through `__csimReportCallbackError` (report on the callback's own realm, WebIDL "invoke a callback") — same as the already-correct handleEvent-Get path just above. The legacy `window.onerror` property handler is invoked with the string message, as the test expects. Add a re-entrancy guard to `reportError`: an `error` handler (`window.onerror` or an `error` listener) that ITSELF throws is reported too, but firing another `error` event for it would recurse unboundedly (a click → throwing listener → report → error event → throwing error handler → report → …). While already reporting, skip the event and just log — matching browsers, where error reporting is not re-entrant. (Before this commit `logThrew` broke the cycle; now that listener throws are reported, the guard is required.) Clears Event-dispatch-throwing.html (single + multiple listeners). The inline `on*`-attribute handler throw (line ~366) still uses `logThrew` — also reportable per spec, but untested and out of scope here. Gate 660/0/15, gem 1586/0/35, 0 WPT regressions. All five apps green on a clean box: Redmine 122/0, Avo 396/0 (3m08s, baseline), Forem, Mastodon 26/0, Discourse 2052/0/67 — confirming the report-the-exception behaviour change (now firing `error` events for throwing listeners) doesn't regress any app. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 2b66c40 commit b19476f

4 files changed

Lines changed: 36 additions & 8 deletions

File tree

lib/capybara/simulated/js/bridge.bundle.js

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3342,7 +3342,10 @@
33423342
cb.call(entry.handler, event);
33433343
fired = true;
33443344
} catch (e) {
3345-
logThrew("listener (event=" + event.type + ", tag=" + (node && node._tag) + ")", e);
3345+
try {
3346+
globalThis.__csimReportCallbackError(cb, e);
3347+
} catch (_) {
3348+
}
33463349
} finally {
33473350
event._inPassiveListener = false;
33483351
}
@@ -3351,7 +3354,10 @@
33513354
entry.handler.call(node, event);
33523355
fired = true;
33533356
} catch (e) {
3354-
logThrew("listener (event=" + event.type + ", tag=" + (node && node._tag) + ")", e);
3357+
try {
3358+
globalThis.__csimReportCallbackError(entry.handler, e);
3359+
} catch (_) {
3360+
}
33553361
} finally {
33563362
event._inPassiveListener = false;
33573363
}
@@ -17011,8 +17017,17 @@
1701117017
globalThis.structuredClone = /* @__PURE__ */ __name(function structuredClone(v) {
1701217018
return cloneInto(v, /* @__PURE__ */ new Map());
1701317019
}, "structuredClone");
17020+
var __csimReportingError = false;
1701417021
globalThis.reportError = /* @__PURE__ */ __name(function reportError(e) {
17022+
if (__csimReportingError) {
17023+
try {
17024+
console.error(e && e.stack ? e.stack : String(e));
17025+
} catch (_) {
17026+
}
17027+
return;
17028+
}
1701517029
let cancelled = false;
17030+
__csimReportingError = true;
1701617031
try {
1701717032
if (typeof globalThis.ErrorEvent !== "undefined" && typeof globalThis.dispatchEvent === "function") {
1701817033
const ev = new globalThis.ErrorEvent("error", {
@@ -17026,6 +17041,8 @@
1702617041
cancelled = globalThis.dispatchEvent(ev) === false;
1702717042
}
1702817043
} catch (_) {
17044+
} finally {
17045+
__csimReportingError = false;
1702917046
}
1703017047
if (!cancelled) {
1703117048
try {

lib/capybara/simulated/js/src/dispatch.js

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -396,12 +396,16 @@ function fireListeners(node, event, capture) {
396396
try { globalThis.reportError(e); } catch (_) {}
397397
continue;
398398
}
399+
// A callback throw is REPORTED (HTML "report the exception" — a window
400+
// `error` event + legacy `onerror`), NOT propagated to dispatchEvent's
401+
// caller, and dispatch continues to the next listener. Report on the
402+
// callback's own realm (WebIDL "invoke a callback"), same as the Get path.
399403
try { cb.call(entry.handler, event); fired = true; }
400-
catch (e) { logThrew('listener (event=' + event.type + ', tag=' + (node && node._tag) + ')', e); }
404+
catch (e) { try { globalThis.__csimReportCallbackError(cb, e); } catch (_) {} }
401405
finally { event._inPassiveListener = false; }
402406
} else {
403407
try { entry.handler.call(node, event); fired = true; }
404-
catch (e) { logThrew('listener (event=' + event.type + ', tag=' + (node && node._tag) + ')', e); }
408+
catch (e) { try { globalThis.__csimReportCallbackError(entry.handler, e); } catch (_) {} }
405409
finally { event._inPassiveListener = false; }
406410
}
407411
}

lib/capybara/simulated/js/src/platform-globals.js

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -359,8 +359,18 @@ globalThis.structuredClone = function structuredClone(v) { return cloneInto(v, n
359359
// console. This is also the channel a throwing event-loop callback surfaces
360360
// through (e.g. queueMicrotask), so it must fire the `error` event — NOT the
361361
// promise-rejection channel — to match real-browser behavior.
362+
let __csimReportingError = false;
362363
globalThis.reportError = function reportError(e) {
364+
// Re-entrancy guard: an `error` handler (`window.onerror` / an `error`
365+
// listener) that itself throws is reported too — but firing ANOTHER `error`
366+
// event for it would recurse unboundedly. While already reporting, skip the
367+
// event and just log, matching browsers (error reporting is not re-entrant).
368+
if (__csimReportingError) {
369+
try { console.error(e && e.stack ? e.stack : String(e)); } catch (_) {}
370+
return;
371+
}
363372
let cancelled = false;
373+
__csimReportingError = true;
364374
try {
365375
if (typeof globalThis.ErrorEvent !== 'undefined' && typeof globalThis.dispatchEvent === 'function') {
366376
const ev = new globalThis.ErrorEvent('error', {
@@ -374,7 +384,7 @@ globalThis.reportError = function reportError(e) {
374384
// dispatchEvent returns false iff a listener called preventDefault.
375385
cancelled = globalThis.dispatchEvent(ev) === false;
376386
}
377-
} catch (_) {}
387+
} catch (_) {} finally { __csimReportingError = false; }
378388
if (!cancelled) {
379389
try { console.error(e && e.stack ? e.stack : String(e)); } catch (_) {}
380390
}

spec/support/wpt_expected_failures.yml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,6 @@ dom/events/Event-dispatch-throwing-multiple-globals.html:
4242
global
4343
- exception thrown in event listener interface object should result in error event
4444
on listener's global
45-
dom/events/Event-dispatch-throwing.html:
46-
- Throwing in event listener with a single listeners
47-
- Throwing in event listener with multiple listeners
4845
dom/events/Event-timestamp-high-resolution.https.html:
4946
- Constructed GamepadEvent timestamp should be high resolution and have the same time
5047
origin as performance.now()

0 commit comments

Comments
 (0)