@samitouri / QOS-React / commits / 8d30cc968b

[DevTools] Type EventEmitter error handling (#37048)

Tightens `EventEmitter` listener types and fixes error handling so the first thrown value is preserved while subsequent errors are reported instead of swallowed. Adds regression coverage for listener failures.

Ruslan Lesiutin committed Jul 23, 2026 at 10:39 UTC 8d30cc968b6715b4d2aa8b5aca1503b5e2854c6d
2 files changed +52 -10
packages/react-devtools-shared/src/__tests__/events-test.js
+40 -7
@@ -16,12 +16,10 @@ describe('events', () => {
16 dispatcher = new EventEmitter();
17 });
18
19 - // @reactVersion >=16
19 it('can dispatch an event with no listeners', () => {
20 dispatcher.emit('event', 123);
21 });
22
24 - // @reactVersion >=16
23 it('handles a listener being attached multiple times', () => {
24 const callback = jest.fn();
25
@@ -33,7 +31,6 @@ describe('events', () => {
31 expect(callback).toHaveBeenCalledWith(123);
32 });
33
36 - // @reactVersion >=16
34 it('notifies all attached listeners of events', () => {
35 const callback1 = jest.fn();
36 const callback2 = jest.fn();
@@ -51,7 +48,6 @@ describe('events', () => {
48 expect(callback3).not.toHaveBeenCalled();
49 });
50
54 - // @reactVersion >= 16.0
51 it('calls later listeners before re-throwing if an earlier one throws', () => {
52 const callbackThatThrows = jest.fn(() => {
53 throw Error('expected');
@@ -71,7 +67,46 @@ describe('events', () => {
67 expect(callback).toHaveBeenCalledWith(123);
68 });
69
74 - // @reactVersion >= 16.0
70 + it('preserves the first thrown value and reports later errors', () => {
71 + const laterError = new Error('later error');
72 + const errorHandler = jest.fn(event => {
73 + event.preventDefault();
74 + });
75 + const firstCallback = jest.fn(() => {
76 + // This verifies that the emitter preserves any legal thrown value.
77 + // eslint-disable-next-line no-throw-literal
78 + throw null;
79 + });
80 + const secondCallback = jest.fn(() => {
81 + throw laterError;
82 + });
83 + const thirdCallback = jest.fn();
84 +
85 + dispatcher.addListener('event', firstCallback);
86 + dispatcher.addListener('event', secondCallback);
87 + dispatcher.addListener('event', thirdCallback);
88 +
89 + let caughtValue = undefined;
90 + window.addEventListener('error', errorHandler);
91 + try {
92 + dispatcher.emit('event', 123);
93 + } catch (error) {
94 + caughtValue = error;
95 + } finally {
96 + window.removeEventListener('error', errorHandler);
97 + }
98 +
99 + expect(caughtValue).toBe(null);
100 + expect(errorHandler).toHaveBeenCalledTimes(1);
101 + expect(errorHandler.mock.calls[0][0]).toEqual(
102 + expect.objectContaining({
103 + error: laterError,
104 + message: 'later error',
105 + }),
106 + );
107 + expect(thirdCallback).toHaveBeenCalledWith(123);
108 + });
109 +
110 it('removes attached listeners', () => {
111 const callback1 = jest.fn();
112 const callback2 = jest.fn();
@@ -86,7 +121,6 @@ describe('events', () => {
121 expect(callback2).toHaveBeenCalledWith(123);
122 });
123
89 - // @reactVersion >= 16.0
124 it('removes all listeners', () => {
125 const callback1 = jest.fn();
126 const callback2 = jest.fn();
@@ -104,7 +138,6 @@ describe('events', () => {
138 expect(callback3).not.toHaveBeenCalled();
139 });
140
107 - // @reactVersion >= 16.0
141 it('should call the initial listeners even if others are added or removed during a dispatch', () => {
142 const callback1 = jest.fn(() => {
143 dispatcher.removeListener('event', callback2);
packages/react-devtools-shared/src/events.js
+12 -3
@@ -7,12 +7,14 @@
7 * @flow
8 */
9
10 +import reportGlobalError from 'shared/reportGlobalError';
11 +
12 export default class EventEmitter<Events: Object> {
13 listenersMap: Map<string, Array<Function>> = new Map();
14
15 addListener<Event: $Keys<Events>>(
16 event: Event,
15 - listener: (...Events[Event]) => any,
17 + listener: (...Events[Event]) => mixed,
18 ): void {
19 const listeners = this.listenersMap.get(event);
20 if (listeners === undefined) {
@@ -42,9 +44,13 @@ export default class EventEmitter<Events: Object> {
44 try {
45 listener.apply(null, args);
46 } catch (error) {
45 - if (caughtError === null) {
47 + if (!didThrow) {
48 didThrow = true;
49 caughtError = error;
50 + } else {
51 + // Continue notifying the remaining listeners, but do not hide
52 + // additional failures behind the first one.
53 + reportGlobalError(error);
54 }
55 }
56 }
@@ -60,7 +66,10 @@ export default class EventEmitter<Events: Object> {
66 this.listenersMap.clear();
67 }
68
63 - removeListener(event: $Keys<Events>, listener: Function): void {
69 + removeListener<Event: $Keys<Events>>(
70 + event: Event,
71 + listener: (...Events[Event]) => mixed,
72 + ): void {
73 const listeners = this.listenersMap.get(event);
74 if (listeners !== undefined) {
75 const index = listeners.indexOf(listener);