From d40b8940aabd22f67bf2167a602371f28456dfb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Norte?= Date: Tue, 16 Jun 2026 02:41:09 -0700 Subject: [PATCH] Rethrow event listener errors synchronously for native event dispatch (#57207) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: Under the EventTarget-based event dispatch path, an error thrown by an event handler (e.g. `onPress`, `onScroll`) was deferred to a new task via `setTimeout(0)` in `reportListenerError`. This diverged from the legacy plugin path, which collected the first listener error and rethrew it synchronously at the end of the dispatch (React's `runEventsInBatch` + `rethrowCaughtError`). As a result, handler errors escaped the synchronous dispatch flow — they were no longer catchable by React error boundaries or the native event call, and instead surfaced as deferred, uncaught global errors. This restores the legacy contract for native event dispatch: - `dispatchTrustedEvent` gains an opt-in `rethrowListenerErrors` argument. `dispatch()` threads a per-dispatch error holder through `invoke`/`invokeListeners`; when the flag is set it records the first listener error and rethrows it synchronously after the event has been fully cleaned up. - The renderer's native event dispatch (`dispatchNativeEvent`) opts in, so listener errors propagate synchronously again. The responder lifecycle `rethrowCaughtError()` is moved into a `finally` so a pending responder error can never leak into a later dispatch if the normal dispatch throws. - The public `dispatchEvent` API and other `EventTarget` consumers (e.g. `XMLHttpRequest`) are unchanged: they keep the DOM contract of reporting listener errors to the global error handler without throwing. - Re-enabled the previously skipped event-dispatch error-handling integration tests so they run in both dispatch modes. Changelog: [Internal] Reviewed By: javache Differential Revision: D108622141 --- .../__tests__/EventTargetDispatching-itest.js | 10 +- .../renderer/events/dispatchNativeEvent.js | 117 ++++++++++-------- .../private/webapis/dom/events/EventTarget.js | 83 +++++++++++-- .../events/internals/EventTargetInternals.js | 14 ++- 4 files changed, 148 insertions(+), 76 deletions(-) diff --git a/packages/react-native/src/private/renderer/core/__tests__/EventTargetDispatching-itest.js b/packages/react-native/src/private/renderer/core/__tests__/EventTargetDispatching-itest.js index 76de52d98711..8336899f4c5f 100644 --- a/packages/react-native/src/private/renderer/core/__tests__/EventTargetDispatching-itest.js +++ b/packages/react-native/src/private/renderer/core/__tests__/EventTargetDispatching-itest.js @@ -1327,15 +1327,7 @@ const {isOSS} = Fantom.getConstants(); expect(order).toEqual(['parent-capture']); }); - // When enableNativeEventTargetEventDispatching is true, EventTarget.js - // defers handler errors via setTimeout(0) in reportListenerError. This - // leaves a pending callback that Fantom's validateEmptyMessageQueue - // catches, and the error leaks into subsequent tests. Skip in that - // configuration until the error propagation mechanism is made - // synchronous (matching the legacy rethrowCaughtError pattern). - (ReactNativeFeatureFlags.enableNativeEventTargetEventDispatching() - ? describe.skip - : describe)('error handling', () => { + describe('error handling', () => { it('error in event handler does not break dispatch to subsequent listeners', () => { const root = Fantom.createRoot(); const childRef = React.createRef>(); diff --git a/packages/react-native/src/private/renderer/events/dispatchNativeEvent.js b/packages/react-native/src/private/renderer/events/dispatchNativeEvent.js index 1013ad98687f..69efbb30fea4 100644 --- a/packages/react-native/src/private/renderer/events/dispatchNativeEvent.js +++ b/packages/react-native/src/private/renderer/events/dispatchNativeEvent.js @@ -44,64 +44,75 @@ export default function dispatchNativeEvent( // Process responder events before normal event dispatch. processResponderEvent(type, target, payload); - // Normal EventTarget dispatch - const bubbleConfig = customBubblingEventTypes[type]; - const directConfig = customDirectEventTypes[type]; + try { + // Normal EventTarget dispatch + const bubbleConfig = customBubblingEventTypes[type]; + const directConfig = customDirectEventTypes[type]; - // Skip events that are not registered in the view config - if (bubbleConfig != null || directConfig != null) { - // Honor `skipBubbling` declared in the view config: when set, the bubble - // phase only fires on the target itself (matching the legacy renderer's - // behavior). The synthesized event reports `bubbles: false`, which causes - // the EventTarget bubble loop to short-circuit after dispatching to the - // target. Capture-phase listeners are unaffected. - const bubbles = - bubbleConfig != null && - bubbleConfig.phasedRegistrationNames.skipBubbling !== true; + // Skip events that are not registered in the view config + if (bubbleConfig != null || directConfig != null) { + // Honor `skipBubbling` declared in the view config: when set, the bubble + // phase only fires on the target itself (matching the legacy renderer's + // behavior). The synthesized event reports `bubbles: false`, which causes + // the EventTarget bubble loop to short-circuit after dispatching to the + // target. Capture-phase listeners are unaffected. + const bubbles = + bubbleConfig != null && + bubbleConfig.phasedRegistrationNames.skipBubbling !== true; - const eventType = topLevelTypeToEventType(type); - const options: {bubbles: boolean, cancelable: boolean} = { - bubbles, - cancelable: true, - }; + const eventType = topLevelTypeToEventType(type); + const options: {bubbles: boolean, cancelable: boolean} = { + bubbles, + cancelable: true, + }; - // Preserve the native event timestamp for backwards compatibility. - const nativeTimestamp = payload.timeStamp ?? payload.timestamp; - if (typeof nativeTimestamp === 'number') { - setEventInitTimeStamp(options, nativeTimestamp); - } - - const syntheticEvent = new LegacySyntheticEvent( - eventType, - options, - payload, - bubbleConfig ?? directConfig, - ); + // Preserve the native event timestamp for backwards compatibility. + const nativeTimestamp = payload.timeStamp ?? payload.timestamp; + if (typeof nativeTimestamp === 'number') { + setEventInitTimeStamp(options, nativeTimestamp); + } - // Pre-resolve the React prop names ("onFoo" / "onFooCapture") once per - // dispatch and stash them on the event so per-ancestor - // `EVENT_TARGET_GET_DECLARATIVE_LISTENER_KEY` lookups can read them - // directly, avoiding the per-call `getEventTypePropName` hash lookup. - if (bubbleConfig != null) { - const phasedRegistrationNames = bubbleConfig.phasedRegistrationNames; - setBubbledPropName( - syntheticEvent, - phasedRegistrationNames.bubbled ?? null, + const syntheticEvent = new LegacySyntheticEvent( + eventType, + options, + payload, + bubbleConfig ?? directConfig, ); - setCapturedPropName( - syntheticEvent, - phasedRegistrationNames.captured ?? null, - ); - } else if (directConfig != null) { - setBubbledPropName(syntheticEvent, directConfig.registrationName ?? null); - setCapturedPropName(syntheticEvent, null); - } - dispatchTrustedEvent(target, syntheticEvent); - } + // Pre-resolve the React prop names ("onFoo" / "onFooCapture") once per + // dispatch and stash them on the event so per-ancestor + // `EVENT_TARGET_GET_DECLARATIVE_LISTENER_KEY` lookups can read them + // directly, avoiding the per-call `getEventTypePropName` hash lookup. + if (bubbleConfig != null) { + const phasedRegistrationNames = bubbleConfig.phasedRegistrationNames; + setBubbledPropName( + syntheticEvent, + phasedRegistrationNames.bubbled ?? null, + ); + setCapturedPropName( + syntheticEvent, + phasedRegistrationNames.captured ?? null, + ); + } else if (directConfig != null) { + setBubbledPropName( + syntheticEvent, + directConfig.registrationName ?? null, + ); + setCapturedPropName(syntheticEvent, null); + } - // Rethrow the first error caught during responder lifecycle dispatch, - // after all dispatching is complete. This matches the old system's - // runEventsInBatch → rethrowCaughtError pattern. - rethrowCaughtError(); + // Pass `rethrowListenerErrors: true` so the first listener error is + // rethrown synchronously (matching the legacy plugin path) rather than + // deferred to a new task, keeping it catchable by React error boundaries + // and the native event call. + dispatchTrustedEvent(target, syntheticEvent, true); + } + } finally { + // Rethrow the first error caught during responder lifecycle dispatch, + // after all dispatching is complete. This matches the old system's + // runEventsInBatch → rethrowCaughtError pattern. Running it in a `finally` + // ensures a pending responder error is never left to leak into a later + // dispatch even if the normal dispatch above threw synchronously. + rethrowCaughtError(); + } } diff --git a/packages/react-native/src/private/webapis/dom/events/EventTarget.js b/packages/react-native/src/private/webapis/dom/events/EventTarget.js index e794efcc3379..51a16621617c 100644 --- a/packages/react-native/src/private/webapis/dom/events/EventTarget.js +++ b/packages/react-native/src/private/webapis/dom/events/EventTarget.js @@ -207,7 +207,7 @@ export default class EventTarget { setIsTrusted(event, false); - dispatch(this, event); + dispatch(this, event, false); return !event.defaultPrevented; } @@ -249,8 +249,11 @@ export default class EventTarget { * canceled (i.e. `event.defaultPrevented`), otherwise `true`. */ // $FlowExpectedError[unsupported-syntax] - [INTERNAL_DISPATCH_METHOD_KEY](event: Event): boolean { - dispatch(this, event); + [INTERNAL_DISPATCH_METHOD_KEY]( + event: Event, + rethrowListenerErrors?: boolean, + ): boolean { + dispatch(this, event, rethrowListenerErrors === true); return !event.defaultPrevented; } } @@ -280,13 +283,26 @@ function getDefaultPassiveValue( * Implements the "event dispatch" concept * (see https://dom.spec.whatwg.org/#concept-event-dispatch). */ -function dispatch(eventTarget: EventTarget, event: Event): void { +function dispatch( + eventTarget: EventTarget, + event: Event, + rethrowErrors: boolean, +): void { setEventDispatchFlag(event, true); const eventPath = getEventPath(eventTarget, event); setComposedPath(event, eventPath); setTarget(event, eventTarget); + // When `rethrowErrors` is set (trusted dispatch of native UI events), collect + // the first listener error and rethrow it synchronously once the dispatch + // completes, matching the legacy plugin system's `rethrowCaughtError` + // behavior. Otherwise (the public `dispatchEvent` API, XHR, etc.) listener + // errors are reported to the global error handler per the DOM spec. + const errorState: ListenerErrorState | null = rethrowErrors + ? {hasError: false, error: undefined} + : null; + for (let i = eventPath.length - 1; i >= 0; i--) { if (getStopPropagationFlag(event)) { break; @@ -297,7 +313,7 @@ function dispatch(eventTarget: EventTarget, event: Event): void { event, target === eventTarget ? Event.AT_TARGET : Event.CAPTURING_PHASE, ); - invoke(target, event, Event.CAPTURING_PHASE); + invoke(target, event, Event.CAPTURING_PHASE, errorState); } for (const target of eventPath) { @@ -315,7 +331,7 @@ function dispatch(eventTarget: EventTarget, event: Event): void { event, target === eventTarget ? Event.AT_TARGET : Event.BUBBLING_PHASE, ); - invoke(target, event, Event.BUBBLING_PHASE); + invoke(target, event, Event.BUBBLING_PHASE, errorState); } setEventPhase(event, Event.NONE); @@ -325,6 +341,12 @@ function dispatch(eventTarget: EventTarget, event: Event): void { setEventDispatchFlag(event, false); setStopImmediatePropagationFlag(event, false); setStopPropagationFlag(event, false); + + // Trusted dispatch: surface the first listener error synchronously, after the + // event has been fully cleaned up. + if (errorState != null && errorState.hasError) { + throw errorState.error; + } } /** @@ -356,6 +378,7 @@ function invoke( eventTarget: EventTarget, event: Event, eventPhase: EventPhase, + errorState: ListenerErrorState | null, ) { const isCapture = eventPhase === Event.CAPTURING_PHASE; @@ -385,7 +408,7 @@ function invoke( try { propListener.call(eventTarget, event); } catch (error) { - reportListenerError(error); + handleListenerError(error, errorState); } global.event = currentEvent; return; @@ -404,7 +427,7 @@ function invoke( for (const registration of maybeListeners.values()) { listeners.push(registration); } - invokeListeners(eventTarget, event, listeners, isCapture); + invokeListeners(eventTarget, event, listeners, isCapture, errorState); return; } @@ -419,6 +442,7 @@ function invoke( event, Array.from(maybeListeners.values()), isCapture, + errorState, ); } @@ -427,6 +451,7 @@ function invokeListeners( event: Event, listeners: Array, isCapture: boolean, + errorState: ListenerErrorState | null, ): void { for (const listener of listeners) { if (listener.removed) { @@ -454,7 +479,7 @@ function invokeListeners( callback.handleEvent(event); } } catch (error) { - reportListenerError(error); + handleListenerError(error, errorState); } if (listener.passive) { @@ -509,12 +534,44 @@ function setEventDispatchFlag(event: Event, value: boolean): void { event[EVENT_DISPATCH_FLAG] = value; } +type ListenerErrorState = {hasError: boolean, error: unknown}; + +/** + * Handle an error thrown by an event listener without aborting the rest of the + * dispatch. + * + * For trusted dispatch of native UI events (`errorState` is non-null), the + * first error is recorded so `dispatch` can rethrow it synchronously once the + * dispatch completes, matching the legacy plugin path (React's + * runEventsInBatch + `rethrowCaughtError`). This keeps listener errors + * catchable by React error boundaries and the native event call, instead of + * escaping as deferred uncaught exceptions. + * + * Otherwise (`errorState` is null: the public `dispatchEvent` API, XHR, etc.) + * the DOM spec requires reporting the exception to the global error handler + * without throwing, so it is deferred via `reportListenerError`. + */ +function handleListenerError( + error: unknown, + errorState: ListenerErrorState | null, +): void { + if (errorState != null) { + if (!errorState.hasError) { + errorState.hasError = true; + errorState.error = error; + } + return; + } + + reportListenerError(error); +} + /** * Surface a listener error to the global error handler without aborting the - * rest of the dispatch. Throws in a new task so the error becomes an - * uncaught exception (matching the legacy plugin path's behavior of - * propagating listener errors via React's runEventsInBatch + - * `rethrowCaughtError`, rather than swallowing them as a `console.error`). + * rest of the dispatch. Throws in a new task so the error becomes an uncaught + * exception. Used for dispatches that follow the DOM `dispatchEvent` contract + * (the public API, XHR, etc.), where errors are reported rather than thrown + * synchronously. * * `setTimeout(0)` schedules a new macrotask; the throw inside it has no * catcher above, so it bubbles up to the host's unhandled-error reporter. diff --git a/packages/react-native/src/private/webapis/dom/events/internals/EventTargetInternals.js b/packages/react-native/src/private/webapis/dom/events/internals/EventTargetInternals.js index 93bf4803feaf..87322c62d1fe 100644 --- a/packages/react-native/src/private/webapis/dom/events/internals/EventTargetInternals.js +++ b/packages/react-native/src/private/webapis/dom/events/internals/EventTargetInternals.js @@ -75,13 +75,25 @@ export function getEventTargetParent(target: EventTarget): EventTarget | null { * * This should only be used by the runtime to dispatch native events to * JavaScript. + * + * When `rethrowListenerErrors` is `true`, the first error thrown by a listener + * is rethrown synchronously once dispatch completes (matching the legacy + * plugin system's `rethrowCaughtError` behavior). This is used by the renderer + * for native UI events so listener errors stay catchable by React error + * boundaries and the native event call. When omitted/`false`, listener errors + * are reported to the global error handler per the DOM spec (used by XHR and + * other web API event targets). */ export function dispatchTrustedEvent( eventTarget: EventTarget, event: Event, + rethrowListenerErrors?: boolean, ): boolean { setIsTrusted(event, true); // $FlowExpectedError[prop-missing] - return eventTarget[INTERNAL_DISPATCH_METHOD_KEY](event); + return eventTarget[INTERNAL_DISPATCH_METHOD_KEY]( + event, + rethrowListenerErrors, + ); }