Skip to content

Commit fd082b9

Browse files
rubennortemeta-codesync[bot]
authored andcommitted
Re-throw errors from tasks and microtasks synchronously (#56513)
Summary: Pull Request resolved: #56513 Changelog: [internal] Fantom previously did not propagate errors thrown inside `runTask` callbacks (or microtasks queued by them) to Jest, so failing assertions inside tasks were silently swallowed and only logged via `console.error`. Tests had to work around this with manual `try`/`catch` blocks and a couple of `it.skip`s with TODOs. This installs a Fantom-specific global error handler (via `ErrorUtils.setGlobalHandler`) that captures the first error reported during the current work loop into `pendingError`. After `flushMessageQueue()` returns, `runWorkLoop()` re-throws the captured error so it becomes observable as a Jest failure. Only the first error in a work loop is re-thrown (subsequent ones are typically follow-on noise), and `pendingError` is cleared at the start of each work loop so errors captured during module setup do not spuriously fail later loops. The post-loop `runLogBoxCheck()` continues to take precedence, since LogBox diagnostics are more actionable. With this in place: - The 'should throw when running a task inside another task' test can use `expect(...).toThrow(...)` directly instead of manual try/catch. - The two previously-skipped tests for re-throwing errors from tasks and microtasks are now enabled. NOTE: this will cause some tests to start failing, but they're legit failures Reviewed By: javache Differential Revision: D101647822 fbshipit-source-id: b9cba056d46cc94a4f6dad5e3665266df62a0211
1 parent f2f9209 commit fd082b9

2 files changed

Lines changed: 50 additions & 28 deletions

File tree

private/react-native-fantom/src/__tests__/Fantom-itest.js

Lines changed: 15 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -116,47 +116,35 @@ describe('Fantom', () => {
116116
LogBox.uninstall();
117117
});
118118

119-
// TODO: T223804378 when error handling is fixed, this should verify using `toThrow`
120119
it('should throw when running a task inside another task', () => {
121-
let threw = false;
122-
123-
Fantom.runTask(() => {
124-
// TODO replace with expect(() => { ... }).toThrow() when error handling is fixed
125-
try {
120+
expect(() => {
121+
Fantom.runTask(() => {
126122
Fantom.runTask(() => {});
127-
} catch {
128-
threw = true;
129-
}
130-
});
131-
expect(threw).toBe(true);
132-
133-
threw = false;
123+
});
124+
}).toThrow(
125+
'Nested `runTask` calls are not allowed. If you want to schedule a task from inside another task, use `scheduleTask` instead.',
126+
);
134127

135-
Fantom.runTask(() => {
136-
queueMicrotask(() => {
137-
try {
128+
expect(() => {
129+
Fantom.runTask(() => {
130+
queueMicrotask(() => {
138131
Fantom.runTask(() => {});
139-
} catch {
140-
threw = true;
141-
}
132+
});
142133
});
143-
});
144-
expect(threw).toBe(true);
134+
}).toThrow(
135+
'Nested `runTask` calls are not allowed. If you want to schedule a task from inside another task, use `scheduleTask` instead.',
136+
);
145137
});
146138

147-
// TODO: fix error handling and make this pass
148-
// eslint-disable-next-line jest/no-disabled-tests
149-
it.skip('should re-throw errors from the task synchronously', () => {
139+
it('should re-throw errors from the task synchronously', () => {
150140
expect(() => {
151141
Fantom.runTask(() => {
152142
throw new Error('test error');
153143
});
154144
}).toThrow('test error');
155145
});
156146

157-
// TODO: fix error handling and make this pass
158-
// eslint-disable-next-line jest/no-disabled-tests
159-
it.skip('should re-throw errors from microtasks synchronously', () => {
147+
it('should re-throw errors from microtasks synchronously', () => {
160148
expect(() => {
161149
Fantom.runTask(() => {
162150
queueMicrotask(() => {

private/react-native-fantom/src/index.js

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import * as Benchmark from './Benchmark';
2020
import {getConstants} from './Constants';
2121
import getFantomRenderedOutput from './getFantomRenderedOutput';
2222
import {LogBox} from 'react-native';
23+
import ErrorUtils from 'react-native/Libraries/vendor/core/ErrorUtils';
2324
import NativeFantom, {
2425
NativeEventCategory,
2526
} from 'react-native/src/private/testing/fantom/specs/NativeFantom';
@@ -164,6 +165,24 @@ export function scheduleTask(task: () => void | Promise<void>) {
164165

165166
let flushingQueue = false;
166167
let isLogBoxCheckEnabled = true;
168+
let pendingError: unknown = null;
169+
170+
// Install a Fantom-specific global error handler that captures the first error
171+
// reported during the current work loop into `pendingError`. `runWorkLoop()`
172+
// re-throws the captured error after `flushMessageQueue()` returns, so errors
173+
// thrown inside `runTask` callbacks (or microtasks queued by them) propagate
174+
// out and become observable as Jest test failures.
175+
//
176+
// This overrides the handler installed by `setUpErrorHandling.js`, which in
177+
// Fantom (where LogBox is not installed) only `console.error`s the error.
178+
// Subsequent errors during the same work loop are ignored: only the first one
179+
// is re-thrown, since it is typically the most informative; subsequent errors
180+
// are usually follow-on noise.
181+
ErrorUtils.setGlobalHandler((error: unknown, _isFatal: boolean) => {
182+
if (pendingError == null) {
183+
pendingError = error;
184+
}
185+
});
167186

168187
/**
169188
* Runs a task on the event loop.
@@ -331,6 +350,10 @@ export function runWorkLoop(): void {
331350
runLogBoxCheck();
332351
}
333352

353+
// Clear any error captured outside of a work loop (e.g., during module setup)
354+
// so it does not spuriously fail the next work loop.
355+
pendingError = null;
356+
334357
try {
335358
flushingQueue = true;
336359
NativeFantom.flushMessageQueue();
@@ -340,9 +363,20 @@ export function runWorkLoop(): void {
340363

341364
if (__DEV__) {
342365
// We also do it after because a task might trigger the initialization of the environment that enables LogBox,
343-
// which could be equally dangerous.
366+
// which could be equally dangerous. If LogBox is now installed, this throws
367+
// and intentionally takes precedence over any captured task error: the
368+
// LogBox diagnostic is more actionable.
344369
runLogBoxCheck();
345370
}
371+
372+
// Re-throw the first error captured by the global handler during this work
373+
// loop, so the failure propagates out of `runTask` / `runWorkLoop` and Jest
374+
// marks the test as failed with the original error.
375+
const errorToThrow = pendingError;
376+
pendingError = null;
377+
if (errorToThrow != null) {
378+
throw errorToThrow;
379+
}
346380
}
347381

348382
/**

0 commit comments

Comments
 (0)