Skip to content

Commit 4ec0582

Browse files
committed
[Fiber] Don't Rethrow Errors at the Root (#28627)
Stacked on top of #28498 for test fixes. ### Don't Rethrow When we started React it was 1:1 setState calls a series of renders and if they error, it errors where the setState was called. Simple. However, then batching came and the error actually got thrown somewhere else. With concurrent mode, it's not even possible to get setState itself to throw anymore. In fact, all APIs that can rethrow out of React are executed either at the root of the scheduler or inside a DOM event handler. If you throw inside a React.startTransition callback that's sync, then that will bubble out of the startTransition but if you throw inside an async callback or a useTransition we now need to handle it at the hook site. So in 19 we need to make all React.startTransition swallow the error (and report them to reportError). The only one remaining that can throw is flushSync but it doesn't really make sense for it to throw at the callsite neither because batching. Just because something rendered in this flush doesn't mean it was rendered due to what was just scheduled and doesn't mean that it should abort any of the remaining code afterwards. setState is fire and forget. It's send an instruction elsewhere, it's not part of the current imperative code. Error boundaries never rethrow. Since you should really always have error boundaries, most of the time, it wouldn't rethrow anyway. Rethrowing also actually currently drops errors on the floor since we can only rethrow the first error, so to avoid that we'd need to call reportError anyway. This happens in RN events. The other issue with rethrowing is that it logs an extra console.error. Since we're not sure that user code will actually log it anywhere we still log it too just like we do with errors inside error boundaries which leads all of these to log twice. The goal of this PR is to never rethrow out of React instead, errors outside of error boundaries get logged to reportError. Event system errors too. ### Breaking Changes The main thing this affects is testing where you want to inspect the errors thrown. To make it easier to port, if you're inside `act` we track the error into act in an aggregate error and then rethrow it at the root of `act`. Unlike before though, if you flush synchronously inside of act it'll still continue until the end of act before rethrowing. I expect most user code breakages would be to migrate from `flushSync` to `act` if you assert on throwing. However, in the React repo we also have `internalAct` and the `waitForThrow` helpers. Since these have to use public production implementations we track these using the global onerror or process uncaughtException. Unlike regular act, includes both event handler errors and onRecoverableError by default too. Not just render/commit errors. So I had to account for that in our tests. We restore logging an extra log for uncaught errors after the main log with the component stack in it. We use `console.warn`. This is not yet ignorable if you preventDefault to the main error event. To avoid confusion if you don't end up logging the error to console I just added `An error occurred`. ### Polyfill All browsers we support really supports `reportError` but not all test and server environments do, so I implemented a polyfill for browser and node in `shared/reportGlobalError`. I don't love that this is included in all builds and gets duplicated into isomorphic even though it's not actually needed in production. Maybe in the future we can require a polyfill for this. ### Follow Ups In a follow up, I'll make caught vs uncaught error handling be configurable too. --------- Co-authored-by: Ricky Hanlon <[email protected]> DiffTrain build for commit 6786563.
1 parent f3180a2 commit 4ec0582

File tree

13 files changed

+1903
-1740
lines changed

13 files changed

+1903
-1740
lines changed

compiled-rn/facebook-fbsource/xplat/js/RKJSModules/vendor/react-test-renderer/cjs/ReactTestRenderer-dev.js

Lines changed: 135 additions & 120 deletions
Large diffs are not rendered by default.

compiled-rn/facebook-fbsource/xplat/js/RKJSModules/vendor/react-test-renderer/cjs/ReactTestRenderer-prod.js

Lines changed: 203 additions & 207 deletions
Large diffs are not rendered by default.

compiled-rn/facebook-fbsource/xplat/js/RKJSModules/vendor/react-test-renderer/cjs/ReactTestRenderer-profiling.js

Lines changed: 237 additions & 241 deletions
Large diffs are not rendered by default.

compiled-rn/facebook-fbsource/xplat/js/RKJSModules/vendor/react/cjs/React-dev.js

Lines changed: 103 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* @noflow
88
* @nolint
99
* @preventMunge
10-
* @generated SignedSource<<389738660a4f2ab62588a2951e5a2988>>
10+
* @generated SignedSource<<ba8e03c28352119146c3eb29b5683ded>>
1111
*/
1212

1313
"use strict";
@@ -26,7 +26,7 @@ if (__DEV__) {
2626
}
2727
var dynamicFlagsUntyped = require("ReactNativeInternalFeatureFlags");
2828

29-
var ReactVersion = "19.0.0-canary-4322682e";
29+
var ReactVersion = "19.0.0-canary-66632b76";
3030

3131
// ATTENTION
3232
// When adding new symbols to this file,
@@ -100,7 +100,9 @@ if (__DEV__) {
100100
// Tracks whether something called `use` during the current batch of work.
101101
// Determines whether we should yield to microtasks to unwrap already resolved
102102
// promises without suspending.
103-
didUsePromise: false
103+
didUsePromise: false,
104+
// Track first uncaught error within this act
105+
thrownErrors: []
104106
};
105107

106108
/**
@@ -3133,6 +3135,45 @@ if (__DEV__) {
31333135
}
31343136
}
31353137

3138+
var reportGlobalError =
3139+
typeof reportError === "function" // In modern browsers, reportError will dispatch an error event,
3140+
? // emulating an uncaught JavaScript error.
3141+
reportError
3142+
: function (error) {
3143+
if (
3144+
typeof window === "object" &&
3145+
typeof window.ErrorEvent === "function"
3146+
) {
3147+
// Browser Polyfill
3148+
var message =
3149+
typeof error === "object" &&
3150+
error !== null &&
3151+
typeof error.message === "string" // eslint-disable-next-line react-internal/safe-string-coercion
3152+
? String(error.message) // eslint-disable-next-line react-internal/safe-string-coercion
3153+
: String(error);
3154+
var event = new window.ErrorEvent("error", {
3155+
bubbles: true,
3156+
cancelable: true,
3157+
message: message,
3158+
error: error
3159+
});
3160+
var shouldLog = window.dispatchEvent(event);
3161+
3162+
if (!shouldLog) {
3163+
return;
3164+
}
3165+
} else if (
3166+
typeof process === "object" && // $FlowFixMe[method-unbinding]
3167+
typeof process.emit === "function"
3168+
) {
3169+
// Node Polyfill
3170+
process.emit("uncaughtException", error);
3171+
return;
3172+
} // eslint-disable-next-line react-internal/no-production-logging
3173+
3174+
console["error"](error);
3175+
};
3176+
31363177
function startTransition(scope, options) {
31373178
var prevTransition = ReactCurrentBatchConfig.transition; // Each renderer registers a callback to receive the return value of
31383179
// the scope function. This is used to implement async actions.
@@ -3160,10 +3201,10 @@ if (__DEV__) {
31603201
callbacks.forEach(function (callback) {
31613202
return callback(currentTransition, returnValue);
31623203
});
3163-
returnValue.then(noop, onError);
3204+
returnValue.then(noop, reportGlobalError);
31643205
}
31653206
} catch (error) {
3166-
onError(error);
3207+
reportGlobalError(error);
31673208
} finally {
31683209
warnAboutTransitionSubscriptions(prevTransition, currentTransition);
31693210
ReactCurrentBatchConfig.transition = prevTransition;
@@ -3201,18 +3242,7 @@ if (__DEV__) {
32013242
}
32023243
}
32033244

3204-
function noop() {} // Use reportError, if it exists. Otherwise console.error. This is the same as
3205-
// the default for onRecoverableError.
3206-
3207-
var onError =
3208-
typeof reportError === "function" // In modern browsers, reportError will dispatch an error event,
3209-
? // emulating an uncaught JavaScript error.
3210-
reportError
3211-
: function (error) {
3212-
// In older browsers and test environments, fallback to console.error.
3213-
// eslint-disable-next-line react-internal/no-production-logging
3214-
console["error"](error);
3215-
};
3245+
function noop() {}
32163246

32173247
var didWarnAboutMessageChannel = false;
32183248
var enqueueTaskImpl = null;
@@ -3261,6 +3291,16 @@ if (__DEV__) {
32613291
var actScopeDepth = 0; // We only warn the first time you neglect to await an async `act` scope.
32623292

32633293
var didWarnNoAwaitAct = false;
3294+
3295+
function aggregateErrors(errors) {
3296+
if (errors.length > 1 && typeof AggregateError === "function") {
3297+
// eslint-disable-next-line no-undef
3298+
return new AggregateError(errors);
3299+
}
3300+
3301+
return errors[0];
3302+
}
3303+
32643304
function act(callback) {
32653305
{
32663306
// When ReactCurrentActQueue.current is not null, it signals to React that
@@ -3312,9 +3352,15 @@ if (__DEV__) {
33123352
// one used to track `act` scopes. Why, you may be wondering? Because
33133353
// that's how it worked before version 18. Yes, it's confusing! We should
33143354
// delete legacy mode!!
3355+
ReactCurrentActQueue.thrownErrors.push(error);
3356+
}
3357+
3358+
if (ReactCurrentActQueue.thrownErrors.length > 0) {
33153359
ReactCurrentActQueue.isBatchingLegacy = prevIsBatchingLegacy;
33163360
popActScope(prevActQueue, prevActScopeDepth);
3317-
throw error;
3361+
var thrownError = aggregateErrors(ReactCurrentActQueue.thrownErrors);
3362+
ReactCurrentActQueue.thrownErrors.length = 0;
3363+
throw thrownError;
33183364
}
33193365

33203366
if (
@@ -3369,15 +3415,34 @@ if (__DEV__) {
33693415
// `thenable` might not be a real promise, and `flushActQueue`
33703416
// might throw, so we need to wrap `flushActQueue` in a
33713417
// try/catch.
3372-
reject(error);
3418+
ReactCurrentActQueue.thrownErrors.push(error);
3419+
}
3420+
3421+
if (ReactCurrentActQueue.thrownErrors.length > 0) {
3422+
var _thrownError = aggregateErrors(
3423+
ReactCurrentActQueue.thrownErrors
3424+
);
3425+
3426+
ReactCurrentActQueue.thrownErrors.length = 0;
3427+
reject(_thrownError);
33733428
}
33743429
} else {
33753430
resolve(returnValue);
33763431
}
33773432
},
33783433
function (error) {
33793434
popActScope(prevActQueue, prevActScopeDepth);
3380-
reject(error);
3435+
3436+
if (ReactCurrentActQueue.thrownErrors.length > 0) {
3437+
var _thrownError2 = aggregateErrors(
3438+
ReactCurrentActQueue.thrownErrors
3439+
);
3440+
3441+
ReactCurrentActQueue.thrownErrors.length = 0;
3442+
reject(_thrownError2);
3443+
} else {
3444+
reject(error);
3445+
}
33813446
}
33823447
);
33833448
}
@@ -3430,6 +3495,15 @@ if (__DEV__) {
34303495
ReactCurrentActQueue.current = null;
34313496
}
34323497

3498+
if (ReactCurrentActQueue.thrownErrors.length > 0) {
3499+
var _thrownError3 = aggregateErrors(
3500+
ReactCurrentActQueue.thrownErrors
3501+
);
3502+
3503+
ReactCurrentActQueue.thrownErrors.length = 0;
3504+
throw _thrownError3;
3505+
}
3506+
34333507
return {
34343508
then: function (resolve, reject) {
34353509
didAwaitActCall = true;
@@ -3486,15 +3560,21 @@ if (__DEV__) {
34863560
reject
34873561
);
34883562
});
3563+
return;
34893564
} catch (error) {
34903565
// Leave remaining tasks on the queue if something throws.
3491-
reject(error);
3566+
ReactCurrentActQueue.thrownErrors.push(error);
34923567
}
34933568
} else {
34943569
// The queue is empty. We can finish.
34953570
ReactCurrentActQueue.current = null;
3496-
resolve(returnValue);
34973571
}
3572+
}
3573+
3574+
if (ReactCurrentActQueue.thrownErrors.length > 0) {
3575+
var thrownError = aggregateErrors(ReactCurrentActQueue.thrownErrors);
3576+
ReactCurrentActQueue.thrownErrors.length = 0;
3577+
reject(thrownError);
34983578
} else {
34993579
resolve(returnValue);
35003580
}
@@ -3539,7 +3619,7 @@ if (__DEV__) {
35393619
} catch (error) {
35403620
// If something throws, leave the remaining callbacks on the queue.
35413621
queue.splice(0, i + 1);
3542-
throw error;
3622+
ReactCurrentActQueue.thrownErrors.push(error);
35433623
} finally {
35443624
isFlushing = false;
35453625
}

compiled-rn/facebook-fbsource/xplat/js/RKJSModules/vendor/react/cjs/React-prod.js

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* @noflow
88
* @nolint
99
* @preventMunge
10-
* @generated SignedSource<<ccb8a31f6eb2aabad35c7b0717209352>>
10+
* @generated SignedSource<<ac6e1c721cd9e734454c18870d36b509>>
1111
*/
1212

1313
"use strict";
@@ -372,13 +372,36 @@ function lazyInitializer(payload) {
372372
if (1 === payload._status) return payload._result.default;
373373
throw payload._result;
374374
}
375-
function noop() {}
376-
var onError =
375+
var reportGlobalError =
377376
"function" === typeof reportError
378377
? reportError
379378
: function (error) {
379+
if (
380+
"object" === typeof window &&
381+
"function" === typeof window.ErrorEvent
382+
) {
383+
var event = new window.ErrorEvent("error", {
384+
bubbles: !0,
385+
cancelable: !0,
386+
message:
387+
"object" === typeof error &&
388+
null !== error &&
389+
"string" === typeof error.message
390+
? String(error.message)
391+
: String(error),
392+
error: error
393+
});
394+
if (!window.dispatchEvent(event)) return;
395+
} else if (
396+
"object" === typeof process &&
397+
"function" === typeof process.emit
398+
) {
399+
process.emit("uncaughtException", error);
400+
return;
401+
}
380402
console.error(error);
381403
};
404+
function noop() {}
382405
exports.Children = {
383406
map: mapChildren,
384407
forEach: function (children, forEachFunc, forEachContext) {
@@ -532,9 +555,9 @@ exports.startTransition = function (scope) {
532555
(callbacks.forEach(function (callback) {
533556
return callback(currentTransition, returnValue);
534557
}),
535-
returnValue.then(noop, onError));
558+
returnValue.then(noop, reportGlobalError));
536559
} catch (error) {
537-
onError(error);
560+
reportGlobalError(error);
538561
} finally {
539562
ReactCurrentBatchConfig.transition = prevTransition;
540563
}
@@ -640,4 +663,4 @@ exports.useSyncExternalStore = function (
640663
exports.useTransition = function () {
641664
return ReactCurrentDispatcher.current.useTransition();
642665
};
643-
exports.version = "19.0.0-canary-b3b89a7e";
666+
exports.version = "19.0.0-canary-bd294150";

compiled-rn/facebook-fbsource/xplat/js/RKJSModules/vendor/react/cjs/React-profiling.js

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* @noflow
88
* @nolint
99
* @preventMunge
10-
* @generated SignedSource<<111d124bf374c5bbae728fb7f4d17857>>
10+
* @generated SignedSource<<50a5378f60ee29fee7964f02f5243965>>
1111
*/
1212

1313
"use strict";
@@ -339,13 +339,36 @@ function lazyInitializer(payload) {
339339
if (1 === payload._status) return payload._result.default;
340340
throw payload._result;
341341
}
342-
function noop() {}
343-
var onError =
342+
var reportGlobalError =
344343
"function" === typeof reportError
345344
? reportError
346345
: function (error) {
346+
if (
347+
"object" === typeof window &&
348+
"function" === typeof window.ErrorEvent
349+
) {
350+
var event = new window.ErrorEvent("error", {
351+
bubbles: !0,
352+
cancelable: !0,
353+
message:
354+
"object" === typeof error &&
355+
null !== error &&
356+
"string" === typeof error.message
357+
? String(error.message)
358+
: String(error),
359+
error: error
360+
});
361+
if (!window.dispatchEvent(event)) return;
362+
} else if (
363+
"object" === typeof process &&
364+
"function" === typeof process.emit
365+
) {
366+
process.emit("uncaughtException", error);
367+
return;
368+
}
347369
console.error(error);
348370
};
371+
function noop() {}
349372
exports.Children = {
350373
map: mapChildren,
351374
forEach: function (children, forEachFunc, forEachContext) {
@@ -529,9 +552,9 @@ exports.startTransition = function (scope) {
529552
(callbacks.forEach(function (callback) {
530553
return callback(currentTransition, returnValue);
531554
}),
532-
returnValue.then(noop, onError));
555+
returnValue.then(noop, reportGlobalError));
533556
} catch (error) {
534-
onError(error);
557+
reportGlobalError(error);
535558
} finally {
536559
ReactCurrentBatchConfig.transition = prevTransition;
537560
}
@@ -636,7 +659,7 @@ exports.useSyncExternalStore = function (
636659
exports.useTransition = function () {
637660
return ReactCurrentDispatcher.current.useTransition();
638661
};
639-
exports.version = "19.0.0-canary-4b9ba4bc";
662+
exports.version = "19.0.0-canary-5f0b765f";
640663
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
641664
"function" ===
642665
typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop &&
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
2ec2aaea98588178525f83495669e11e96815a00
1+
6786563f3cbbc9b16d5a8187207b5bd904386e53

0 commit comments

Comments
 (0)