Log and show error overlay for commit phase errors (#21723)
* Enable skipped tests from #21723 * Report uncaught errors in DEV * Clear caught error This is not necessary (as proven by tests) because next invokeGuardedCallback clears it anyway. But I'll keep it for consistency with other calls.
Dan Abramov committed
Jun 24, 2021 at 15:48 UTC
7fec38041faf7f4691f503feccbaf5af901367e2
3 files changed
+86
-32
packages/react-dom/src/__tests__/ReactDOMConsoleErrorReporting-test.js
+8
-16
@@ -313,8 +313,7 @@ describe('ReactDOMConsoleErrorReporting', () => {
313
}
314
});
315
316
- // TODO: this is broken due to https://github.com/facebook/react/issues/21712.
317
- xit('logs layout effect errors without an error boundary', () => {
316
+ it('logs layout effect errors without an error boundary', () => {
317
spyOnDevAndProd(console, 'error');
318
319
function Foo() {
@@ -382,8 +381,7 @@ describe('ReactDOMConsoleErrorReporting', () => {
381
}
382
});
383
385
- // TODO: this is broken due to https://github.com/facebook/react/issues/21712.
386
- xit('logs layout effect errors with an error boundary', () => {
384
+ it('logs layout effect errors with an error boundary', () => {
385
spyOnDevAndProd(console, 'error');
386
387
function Foo() {
@@ -453,8 +451,7 @@ describe('ReactDOMConsoleErrorReporting', () => {
451
}
452
});
453
456
- // TODO: this is broken due to https://github.com/facebook/react/issues/21712.
457
- xit('logs passive effect errors without an error boundary', () => {
454
+ it('logs passive effect errors without an error boundary', () => {
455
spyOnDevAndProd(console, 'error');
456
457
function Foo() {
@@ -522,8 +519,7 @@ describe('ReactDOMConsoleErrorReporting', () => {
519
}
520
});
521
525
- // TODO: this is broken due to https://github.com/facebook/react/issues/21712.
526
- xit('logs passive effect errors with an error boundary', () => {
522
+ it('logs passive effect errors with an error boundary', () => {
523
spyOnDevAndProd(console, 'error');
524
525
function Foo() {
@@ -827,8 +823,7 @@ describe('ReactDOMConsoleErrorReporting', () => {
823
}
824
});
825
830
- // TODO: this is broken due to https://github.com/facebook/react/issues/21712.
831
- xit('logs layout effect errors without an error boundary', () => {
826
+ it('logs layout effect errors without an error boundary', () => {
827
spyOnDevAndProd(console, 'error');
828
829
function Foo() {
@@ -898,8 +893,7 @@ describe('ReactDOMConsoleErrorReporting', () => {
893
}
894
});
895
901
- // TODO: this is broken due to https://github.com/facebook/react/issues/21712.
902
- xit('logs layout effect errors with an error boundary', () => {
896
+ it('logs layout effect errors with an error boundary', () => {
897
spyOnDevAndProd(console, 'error');
898
899
function Foo() {
@@ -972,8 +966,7 @@ describe('ReactDOMConsoleErrorReporting', () => {
966
}
967
});
968
975
- // TODO: this is broken due to https://github.com/facebook/react/issues/21712.
976
- xit('logs passive effect errors without an error boundary', () => {
969
+ it('logs passive effect errors without an error boundary', () => {
970
spyOnDevAndProd(console, 'error');
971
972
function Foo() {
@@ -1043,8 +1036,7 @@ describe('ReactDOMConsoleErrorReporting', () => {
1036
}
1037
});
1038
1046
- // TODO: this is broken due to https://github.com/facebook/react/issues/21712.
1047
- xit('logs passive effect errors with an error boundary', () => {
1039
+ it('logs passive effect errors with an error boundary', () => {
1040
spyOnDevAndProd(console, 'error');
1041
1042
function Foo() {
packages/react-reconciler/src/ReactFiberCommitWork.new.js
+39
-8
@@ -140,6 +140,7 @@ import {
140
} from './ReactHookEffectTags';
141
import {didWarnAboutReassigningProps} from './ReactFiberBeginWork.new';
142
import {doesFiberContain} from './ReactFiberTreeReflection';
143
+import {invokeGuardedCallback, clearCaughtError} from 'shared/ReactErrorUtils';
144
145
let didWarnAboutUndefinedSnapshotBeforeUpdate: Set<mixed> | null = null;
146
if (__DEV__) {
@@ -160,6 +161,20 @@ let nextEffect: Fiber | null = null;
161
let inProgressLanes: Lanes | null = null;
162
let inProgressRoot: FiberRoot | null = null;
163
164
+function reportUncaughtErrorInDEV(error) {
165
+ // Wrapping each small part of the commit phase into a guarded
166
+ // callback is a bit too slow (https://github.com/facebook/react/pull/21666).
167
+ // But we rely on it to surface errors to DEV tools like overlays
168
+ // (https://github.com/facebook/react/issues/21712).
169
+ // As a compromise, rethrow only caught errors in a guard.
170
+ if (__DEV__) {
171
+ invokeGuardedCallback(null, () => {
172
+ throw error;
173
+ });
174
+ clearCaughtError();
175
+ }
176
+}
177
+
178
const callComponentWillUnmountWithTimer = function(current, instance) {
179
instance.props = current.memoizedProps;
180
instance.state = current.memoizedState;
@@ -186,8 +201,9 @@ function safelyCallCommitHookLayoutEffectListMount(
201
) {
202
try {
203
commitHookEffectListMount(HookLayout, current);
189
- } catch (unmountError) {
190
- captureCommitPhaseError(current, nearestMountedAncestor, unmountError);
204
+ } catch (error) {
205
+ reportUncaughtErrorInDEV(error);
206
+ captureCommitPhaseError(current, nearestMountedAncestor, error);
207
}
208
}
209
@@ -199,8 +215,9 @@ function safelyCallComponentWillUnmount(
215
) {
216
try {
217
callComponentWillUnmountWithTimer(current, instance);
202
- } catch (unmountError) {
203
- captureCommitPhaseError(current, nearestMountedAncestor, unmountError);
218
+ } catch (error) {
219
+ reportUncaughtErrorInDEV(error);
220
+ captureCommitPhaseError(current, nearestMountedAncestor, error);
221
}
222
}
223
@@ -212,8 +229,9 @@ function safelyCallComponentDidMount(
229
) {
230
try {
231
instance.componentDidMount();
215
- } catch (unmountError) {
216
- captureCommitPhaseError(current, nearestMountedAncestor, unmountError);
232
+ } catch (error) {
233
+ reportUncaughtErrorInDEV(error);
234
+ captureCommitPhaseError(current, nearestMountedAncestor, error);
235
}
236
}
237
@@ -221,8 +239,9 @@ function safelyCallComponentDidMount(
239
function safelyAttachRef(current: Fiber, nearestMountedAncestor: Fiber | null) {
240
try {
241
commitAttachRef(current);
224
- } catch (unmountError) {
225
- captureCommitPhaseError(current, nearestMountedAncestor, unmountError);
242
+ } catch (error) {
243
+ reportUncaughtErrorInDEV(error);
244
+ captureCommitPhaseError(current, nearestMountedAncestor, error);
245
}
246
}
247
@@ -246,6 +265,7 @@ function safelyDetachRef(current: Fiber, nearestMountedAncestor: Fiber | null) {
265
ref(null);
266
}
267
} catch (error) {
268
+ reportUncaughtErrorInDEV(error);
269
captureCommitPhaseError(current, nearestMountedAncestor, error);
270
}
271
} else {
@@ -262,6 +282,7 @@ function safelyCallDestroy(
282
try {
283
destroy();
284
} catch (error) {
285
+ reportUncaughtErrorInDEV(error);
286
captureCommitPhaseError(current, nearestMountedAncestor, error);
287
}
288
}
@@ -323,6 +344,7 @@ function commitBeforeMutationEffects_complete() {
344
try {
345
commitBeforeMutationEffectsOnFiber(fiber);
346
} catch (error) {
347
+ reportUncaughtErrorInDEV(error);
348
captureCommitPhaseError(fiber, fiber.return, error);
349
}
350
resetCurrentDebugFiberInDEV();
@@ -2065,6 +2087,7 @@ function commitMutationEffects_begin(root: FiberRoot) {
2087
try {
2088
commitDeletion(root, childToDelete, fiber);
2089
} catch (error) {
2090
+ reportUncaughtErrorInDEV(error);
2091
captureCommitPhaseError(childToDelete, fiber, error);
2092
}
2093
}
@@ -2087,6 +2110,7 @@ function commitMutationEffects_complete(root: FiberRoot) {
2110
try {
2111
commitMutationEffectsOnFiber(fiber, root);
2112
} catch (error) {
2113
+ reportUncaughtErrorInDEV(error);
2114
captureCommitPhaseError(fiber, fiber.return, error);
2115
}
2116
resetCurrentDebugFiberInDEV();
@@ -2329,6 +2353,7 @@ function commitLayoutMountEffects_complete(
2353
try {
2354
commitLayoutEffectOnFiber(root, current, fiber, committedLanes);
2355
} catch (error) {
2356
+ reportUncaughtErrorInDEV(error);
2357
captureCommitPhaseError(fiber, fiber.return, error);
2358
}
2359
resetCurrentDebugFiberInDEV();
@@ -2382,6 +2407,7 @@ function commitPassiveMountEffects_complete(
2407
try {
2408
commitPassiveMountOnFiber(root, fiber);
2409
} catch (error) {
2410
+ reportUncaughtErrorInDEV(error);
2411
captureCommitPhaseError(fiber, fiber.return, error);
2412
}
2413
resetCurrentDebugFiberInDEV();
@@ -2664,6 +2690,7 @@ function invokeLayoutEffectMountInDEV(fiber: Fiber): void {
2690
try {
2691
commitHookEffectListMount(HookLayout | HookHasEffect, fiber);
2692
} catch (error) {
2693
+ reportUncaughtErrorInDEV(error);
2694
captureCommitPhaseError(fiber, fiber.return, error);
2695
}
2696
break;
@@ -2673,6 +2700,7 @@ function invokeLayoutEffectMountInDEV(fiber: Fiber): void {
2700
try {
2701
instance.componentDidMount();
2702
} catch (error) {
2703
+ reportUncaughtErrorInDEV(error);
2704
captureCommitPhaseError(fiber, fiber.return, error);
2705
}
2706
break;
@@ -2692,6 +2720,7 @@ function invokePassiveEffectMountInDEV(fiber: Fiber): void {
2720
try {
2721
commitHookEffectListMount(HookPassive | HookHasEffect, fiber);
2722
} catch (error) {
2723
+ reportUncaughtErrorInDEV(error);
2724
captureCommitPhaseError(fiber, fiber.return, error);
2725
}
2726
break;
@@ -2715,6 +2744,7 @@ function invokeLayoutEffectUnmountInDEV(fiber: Fiber): void {
2744
fiber.return,
2745
);
2746
} catch (error) {
2747
+ reportUncaughtErrorInDEV(error);
2748
captureCommitPhaseError(fiber, fiber.return, error);
2749
}
2750
break;
@@ -2745,6 +2775,7 @@ function invokePassiveEffectUnmountInDEV(fiber: Fiber): void {
2775
fiber.return,
2776
);
2777
} catch (error) {
2778
+ reportUncaughtErrorInDEV(error);
2779
captureCommitPhaseError(fiber, fiber.return, error);
2780
}
2781
}
packages/react-reconciler/src/ReactFiberCommitWork.old.js
+39
-8
@@ -140,6 +140,7 @@ import {
140
} from './ReactHookEffectTags';
141
import {didWarnAboutReassigningProps} from './ReactFiberBeginWork.old';
142
import {doesFiberContain} from './ReactFiberTreeReflection';
143
+import {invokeGuardedCallback, clearCaughtError} from 'shared/ReactErrorUtils';
144
145
let didWarnAboutUndefinedSnapshotBeforeUpdate: Set<mixed> | null = null;
146
if (__DEV__) {
@@ -160,6 +161,20 @@ let nextEffect: Fiber | null = null;
161
let inProgressLanes: Lanes | null = null;
162
let inProgressRoot: FiberRoot | null = null;
163
164
+function reportUncaughtErrorInDEV(error) {
165
+ // Wrapping each small part of the commit phase into a guarded
166
+ // callback is a bit too slow (https://github.com/facebook/react/pull/21666).
167
+ // But we rely on it to surface errors to DEV tools like overlays
168
+ // (https://github.com/facebook/react/issues/21712).
169
+ // As a compromise, rethrow only caught errors in a guard.
170
+ if (__DEV__) {
171
+ invokeGuardedCallback(null, () => {
172
+ throw error;
173
+ });
174
+ clearCaughtError();
175
+ }
176
+}
177
+
178
const callComponentWillUnmountWithTimer = function(current, instance) {
179
instance.props = current.memoizedProps;
180
instance.state = current.memoizedState;
@@ -186,8 +201,9 @@ function safelyCallCommitHookLayoutEffectListMount(
201
) {
202
try {
203
commitHookEffectListMount(HookLayout, current);
189
- } catch (unmountError) {
190
- captureCommitPhaseError(current, nearestMountedAncestor, unmountError);
204
+ } catch (error) {
205
+ reportUncaughtErrorInDEV(error);
206
+ captureCommitPhaseError(current, nearestMountedAncestor, error);
207
}
208
}
209
@@ -199,8 +215,9 @@ function safelyCallComponentWillUnmount(
215
) {
216
try {
217
callComponentWillUnmountWithTimer(current, instance);
202
- } catch (unmountError) {
203
- captureCommitPhaseError(current, nearestMountedAncestor, unmountError);
218
+ } catch (error) {
219
+ reportUncaughtErrorInDEV(error);
220
+ captureCommitPhaseError(current, nearestMountedAncestor, error);
221
}
222
}
223
@@ -212,8 +229,9 @@ function safelyCallComponentDidMount(
229
) {
230
try {
231
instance.componentDidMount();
215
- } catch (unmountError) {
216
- captureCommitPhaseError(current, nearestMountedAncestor, unmountError);
232
+ } catch (error) {
233
+ reportUncaughtErrorInDEV(error);
234
+ captureCommitPhaseError(current, nearestMountedAncestor, error);
235
}
236
}
237
@@ -221,8 +239,9 @@ function safelyCallComponentDidMount(
239
function safelyAttachRef(current: Fiber, nearestMountedAncestor: Fiber | null) {
240
try {
241
commitAttachRef(current);
224
- } catch (unmountError) {
225
- captureCommitPhaseError(current, nearestMountedAncestor, unmountError);
242
+ } catch (error) {
243
+ reportUncaughtErrorInDEV(error);
244
+ captureCommitPhaseError(current, nearestMountedAncestor, error);
245
}
246
}
247
@@ -246,6 +265,7 @@ function safelyDetachRef(current: Fiber, nearestMountedAncestor: Fiber | null) {
265
ref(null);
266
}
267
} catch (error) {
268
+ reportUncaughtErrorInDEV(error);
269
captureCommitPhaseError(current, nearestMountedAncestor, error);
270
}
271
} else {
@@ -262,6 +282,7 @@ function safelyCallDestroy(
282
try {
283
destroy();
284
} catch (error) {
285
+ reportUncaughtErrorInDEV(error);
286
captureCommitPhaseError(current, nearestMountedAncestor, error);
287
}
288
}
@@ -323,6 +344,7 @@ function commitBeforeMutationEffects_complete() {
344
try {
345
commitBeforeMutationEffectsOnFiber(fiber);
346
} catch (error) {
347
+ reportUncaughtErrorInDEV(error);
348
captureCommitPhaseError(fiber, fiber.return, error);
349
}
350
resetCurrentDebugFiberInDEV();
@@ -2065,6 +2087,7 @@ function commitMutationEffects_begin(root: FiberRoot) {
2087
try {
2088
commitDeletion(root, childToDelete, fiber);
2089
} catch (error) {
2090
+ reportUncaughtErrorInDEV(error);
2091
captureCommitPhaseError(childToDelete, fiber, error);
2092
}
2093
}
@@ -2087,6 +2110,7 @@ function commitMutationEffects_complete(root: FiberRoot) {
2110
try {
2111
commitMutationEffectsOnFiber(fiber, root);
2112
} catch (error) {
2113
+ reportUncaughtErrorInDEV(error);
2114
captureCommitPhaseError(fiber, fiber.return, error);
2115
}
2116
resetCurrentDebugFiberInDEV();
@@ -2329,6 +2353,7 @@ function commitLayoutMountEffects_complete(
2353
try {
2354
commitLayoutEffectOnFiber(root, current, fiber, committedLanes);
2355
} catch (error) {
2356
+ reportUncaughtErrorInDEV(error);
2357
captureCommitPhaseError(fiber, fiber.return, error);
2358
}
2359
resetCurrentDebugFiberInDEV();
@@ -2382,6 +2407,7 @@ function commitPassiveMountEffects_complete(
2407
try {
2408
commitPassiveMountOnFiber(root, fiber);
2409
} catch (error) {
2410
+ reportUncaughtErrorInDEV(error);
2411
captureCommitPhaseError(fiber, fiber.return, error);
2412
}
2413
resetCurrentDebugFiberInDEV();
@@ -2664,6 +2690,7 @@ function invokeLayoutEffectMountInDEV(fiber: Fiber): void {
2690
try {
2691
commitHookEffectListMount(HookLayout | HookHasEffect, fiber);
2692
} catch (error) {
2693
+ reportUncaughtErrorInDEV(error);
2694
captureCommitPhaseError(fiber, fiber.return, error);
2695
}
2696
break;
@@ -2673,6 +2700,7 @@ function invokeLayoutEffectMountInDEV(fiber: Fiber): void {
2700
try {
2701
instance.componentDidMount();
2702
} catch (error) {
2703
+ reportUncaughtErrorInDEV(error);
2704
captureCommitPhaseError(fiber, fiber.return, error);
2705
}
2706
break;
@@ -2692,6 +2720,7 @@ function invokePassiveEffectMountInDEV(fiber: Fiber): void {
2720
try {
2721
commitHookEffectListMount(HookPassive | HookHasEffect, fiber);
2722
} catch (error) {
2723
+ reportUncaughtErrorInDEV(error);
2724
captureCommitPhaseError(fiber, fiber.return, error);
2725
}
2726
break;
@@ -2715,6 +2744,7 @@ function invokeLayoutEffectUnmountInDEV(fiber: Fiber): void {
2744
fiber.return,
2745
);
2746
} catch (error) {
2747
+ reportUncaughtErrorInDEV(error);
2748
captureCommitPhaseError(fiber, fiber.return, error);
2749
}
2750
break;
@@ -2745,6 +2775,7 @@ function invokePassiveEffectUnmountInDEV(fiber: Fiber): void {
2775
fiber.return,
2776
);
2777
} catch (error) {
2778
+ reportUncaughtErrorInDEV(error);
2779
captureCommitPhaseError(fiber, fiber.return, error);
2780
}
2781
}