Skip to content

Commit 1ea6ffc

Browse files
sebmarkbagesebmarkbage
committed
Set the current fiber to the source of the error during error reporting
This lets us expose the component stack to the error reporting that happens here as console.error patching, but more importantly for "owner stacks" this will be able to set the native async stacks to the original fiber. We now use the normal console.error management to add a component stack to the end. # Conflicts: # packages/shared/consoleWithStackDev.js
1 parent c1533e5 commit 1ea6ffc

12 files changed

+147
-86
lines changed

‎packages/react-devtools-shared/src/__tests__/treeContext-test.js‎

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2586,16 +2586,14 @@ describe('TreeListContext', () => {
25862586
utils.act(() => TestRenderer.create(<Contexts />));
25872587

25882588
expect(store).toMatchInlineSnapshot(`
2589-
✕ 1, ⚠ 0
25902589
[root]
2591-
<ErrorBoundary> ✕
2590+
<ErrorBoundary>
25922591
`);
25932592

25942593
selectNextErrorOrWarning();
25952594
expect(state).toMatchInlineSnapshot(`
2596-
✕ 1, ⚠ 0
25972595
[root]
2598-
→ <ErrorBoundary> ✕
2596+
<ErrorBoundary>
25992597
`);
26002598

26012599
utils.act(() => unmount());
@@ -2648,16 +2646,14 @@ describe('TreeListContext', () => {
26482646
utils.act(() => TestRenderer.create(<Contexts />));
26492647

26502648
expect(store).toMatchInlineSnapshot(`
2651-
✕ 1, ⚠ 0
26522649
[root]
2653-
<ErrorBoundary> ✕
2650+
<ErrorBoundary>
26542651
`);
26552652

26562653
selectNextErrorOrWarning();
26572654
expect(state).toMatchInlineSnapshot(`
2658-
✕ 1, ⚠ 0
26592655
[root]
2660-
→ <ErrorBoundary> ✕
2656+
<ErrorBoundary>
26612657
`);
26622658

26632659
utils.act(() => unmount());
@@ -2705,18 +2701,18 @@ describe('TreeListContext', () => {
27052701
utils.act(() => TestRenderer.create(<Contexts />));
27062702

27072703
expect(store).toMatchInlineSnapshot(`
2708-
✕ 2, ⚠ 0
2704+
✕ 1, ⚠ 0
27092705
[root]
2710-
▾ <ErrorBoundary> ✕
2706+
▾ <ErrorBoundary>
27112707
<Child> ✕
27122708
`);
27132709

27142710
selectNextErrorOrWarning();
27152711
expect(state).toMatchInlineSnapshot(`
2716-
✕ 2, ⚠ 0
2712+
✕ 1, ⚠ 0
27172713
[root]
2718-
→ ▾ <ErrorBoundary> ✕
2719-
<Child> ✕
2714+
▾ <ErrorBoundary>
2715+
→ <Child> ✕
27202716
`);
27212717
});
27222718
});

‎packages/react-dom/src/__tests__/ReactDOMConsoleErrorReporting-test.js‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -142,8 +142,8 @@ describe('ReactDOMConsoleErrorReporting', () => {
142142
// Addendum by React:
143143
expect.stringContaining('%s'),
144144
expect.stringContaining('An error occurred in the <Foo> component'),
145-
expect.stringContaining('Foo'),
146145
expect.stringContaining('Consider adding an error boundary'),
146+
expect.stringContaining('Foo'),
147147
],
148148
]);
149149
} else {
@@ -207,8 +207,8 @@ describe('ReactDOMConsoleErrorReporting', () => {
207207
expect.stringContaining(
208208
'The above error occurred in the <Foo> component',
209209
),
210-
expect.stringContaining('Foo'),
211210
expect.stringContaining('ErrorBoundary'),
211+
expect.stringContaining('Foo'),
212212
],
213213
]);
214214
} else {
@@ -273,8 +273,8 @@ describe('ReactDOMConsoleErrorReporting', () => {
273273
// Addendum by React:
274274
expect.stringContaining('%s'),
275275
expect.stringContaining('An error occurred in the <Foo> component'),
276-
expect.stringContaining('Foo'),
277276
expect.stringContaining('Consider adding an error boundary'),
277+
expect.stringContaining('Foo'),
278278
],
279279
]);
280280
} else {
@@ -343,8 +343,8 @@ describe('ReactDOMConsoleErrorReporting', () => {
343343
expect.stringContaining(
344344
'The above error occurred in the <Foo> component',
345345
),
346-
expect.stringContaining('Foo'),
347346
expect.stringContaining('ErrorBoundary'),
347+
expect.stringContaining('Foo'),
348348
],
349349
]);
350350
} else {
@@ -409,8 +409,8 @@ describe('ReactDOMConsoleErrorReporting', () => {
409409
// Addendum by React:
410410
expect.stringContaining('%s'),
411411
expect.stringContaining('An error occurred in the <Foo> component'),
412-
expect.stringContaining('Foo'),
413412
expect.stringContaining('Consider adding an error boundary'),
413+
expect.stringContaining('Foo'),
414414
],
415415
]);
416416
} else {
@@ -477,8 +477,8 @@ describe('ReactDOMConsoleErrorReporting', () => {
477477
expect.stringContaining(
478478
'The above error occurred in the <Foo> component',
479479
),
480-
expect.stringContaining('Foo'),
481480
expect.stringContaining('ErrorBoundary'),
481+
expect.stringContaining('Foo'),
482482
],
483483
]);
484484
} else {

‎packages/react-dom/src/__tests__/ReactDOMConsoleErrorReportingLegacy-test.js‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -161,8 +161,8 @@ describe('ReactDOMConsoleErrorReporting', () => {
161161
expect.stringContaining('%s'),
162162
// Addendum by React:
163163
expect.stringContaining('An error occurred in the <Foo> component'),
164-
expect.stringContaining('Foo'),
165164
expect.stringContaining('Consider adding an error boundary'),
165+
expect.stringContaining('Foo'),
166166
],
167167
]);
168168

@@ -238,8 +238,8 @@ describe('ReactDOMConsoleErrorReporting', () => {
238238
expect.stringContaining(
239239
'The above error occurred in the <Foo> component',
240240
),
241-
expect.stringContaining('Foo'),
242241
expect.stringContaining('ErrorBoundary'),
242+
expect.stringContaining('Foo'),
243243
],
244244
]);
245245
} else {
@@ -307,11 +307,9 @@ describe('ReactDOMConsoleErrorReporting', () => {
307307
expect.stringContaining('%s'),
308308

309309
// Addendum by React:
310-
expect.stringContaining(
311-
'An error occurred in the <Foo> component:',
312-
),
313-
expect.stringContaining('Foo'),
310+
expect.stringContaining('An error occurred in the <Foo> component'),
314311
expect.stringContaining('Consider adding an error boundary'),
312+
expect.stringContaining('Foo'),
315313
],
316314
]);
317315

@@ -391,8 +389,8 @@ describe('ReactDOMConsoleErrorReporting', () => {
391389
expect.stringContaining(
392390
'The above error occurred in the <Foo> component',
393391
),
394-
expect.stringContaining('Foo'),
395392
expect.stringContaining('ErrorBoundary'),
393+
expect.stringContaining('Foo'),
396394
],
397395
]);
398396
} else {
@@ -461,8 +459,8 @@ describe('ReactDOMConsoleErrorReporting', () => {
461459

462460
// Addendum by React:
463461
expect.stringContaining('An error occurred in the <Foo> component'),
464-
expect.stringContaining('Foo'),
465462
expect.stringContaining('Consider adding an error boundary'),
463+
expect.stringContaining('Foo'),
466464
],
467465
]);
468466

@@ -541,8 +539,8 @@ describe('ReactDOMConsoleErrorReporting', () => {
541539
expect.stringContaining(
542540
'The above error occurred in the <Foo> component',
543541
),
544-
expect.stringContaining('Foo'),
545542
expect.stringContaining('ErrorBoundary'),
543+
expect.stringContaining('Foo'),
546544
],
547545
]);
548546
} else {

‎packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -727,7 +727,7 @@ describe('ReactErrorBoundaries', () => {
727727
if (__DEV__) {
728728
expect(console.error).toHaveBeenCalledTimes(1);
729729
expect(console.error.mock.calls[0][2]).toContain(
730-
'The above error occurred in the <BrokenRender> component:',
730+
'The above error occurred in the <BrokenRender> component',
731731
);
732732
}
733733

‎packages/react-dom/src/__tests__/ReactLegacyErrorBoundaries-test.internal.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -705,7 +705,7 @@ describe('ReactLegacyErrorBoundaries', () => {
705705
'ReactDOM.render has not been supported since React 18',
706706
);
707707
expect(console.error.mock.calls[1][2]).toContain(
708-
'The above error occurred in the <BrokenRender> component:',
708+
'The above error occurred in the <BrokenRender> component',
709709
);
710710
}
711711

‎packages/react-reconciler/src/ReactFiberErrorLogger.js‎

Lines changed: 61 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ import reportGlobalError from 'shared/reportGlobalError';
1818

1919
import ReactSharedInternals from 'shared/ReactSharedInternals';
2020

21+
import {enableOwnerStacks} from 'shared/ReactFeatureFlags';
22+
2123
// Side-channel since I'm not sure we want to make this part of the public API
2224
let componentName: null | string = null;
2325
let errorBoundaryName: null | string = null;
@@ -34,20 +36,36 @@ export function defaultOnUncaughtError(
3436
// So we add those into a separate console.warn.
3537
reportGlobalError(error);
3638
if (__DEV__) {
37-
const componentStack =
38-
errorInfo.componentStack != null ? errorInfo.componentStack : '';
39-
4039
const componentNameMessage = componentName
41-
? `An error occurred in the <${componentName}> component:`
42-
: 'An error occurred in one of your React components:';
40+
? `An error occurred in the <${componentName}> component.`
41+
: 'An error occurred in one of your React components.';
4342

44-
console['warn'](
45-
'%s\n%s\n\n%s',
46-
componentNameMessage,
47-
componentStack || '',
43+
const errorBoundaryMessage =
4844
'Consider adding an error boundary to your tree to customize error handling behavior.\n' +
49-
'Visit https://react.dev/link/error-boundaries to learn more about error boundaries.',
50-
);
45+
'Visit https://react.dev/link/error-boundaries to learn more about error boundaries.';
46+
47+
if (enableOwnerStacks) {
48+
console.warn(
49+
'%s\n\n%s\n',
50+
componentNameMessage,
51+
errorBoundaryMessage,
52+
// We let our console.error wrapper add the component stack to the end.
53+
);
54+
} else {
55+
// The current Fiber is disconnected at this point which means that console printing
56+
// cannot add a component stack since it terminates at the deletion node. This is not
57+
// a problem for owner stacks which are not disconnected but for the parent component
58+
// stacks we need to use the snapshot we've previously extracted.
59+
const componentStack =
60+
errorInfo.componentStack != null ? errorInfo.componentStack : '';
61+
// Don't transform to our wrapper
62+
console['warn'](
63+
'%s\n\n%s\n%s',
64+
componentNameMessage,
65+
errorBoundaryMessage,
66+
componentStack,
67+
);
68+
}
5169
}
5270
}
5371

@@ -63,31 +81,47 @@ export function defaultOnCaughtError(
6381

6482
// Caught by error boundary
6583
if (__DEV__) {
66-
const componentStack =
67-
errorInfo.componentStack != null ? errorInfo.componentStack : '';
68-
6984
const componentNameMessage = componentName
70-
? `The above error occurred in the <${componentName}> component:`
71-
: 'The above error occurred in one of your React components:';
85+
? `The above error occurred in the <${componentName}> component.`
86+
: 'The above error occurred in one of your React components.';
7287

7388
// In development, we provide our own message which includes the component stack
7489
// in addition to the error.
75-
// Don't transform to our wrapper
76-
console['error'](
77-
'%o\n\n%s\n%s\n\n%s',
78-
error,
79-
componentNameMessage,
80-
componentStack,
90+
const recreateMessage =
8191
`React will try to recreate this component tree from scratch ` +
82-
`using the error boundary you provided, ${
83-
errorBoundaryName || 'Anonymous'
84-
}.`,
85-
);
92+
`using the error boundary you provided, ${
93+
errorBoundaryName || 'Anonymous'
94+
}.`;
95+
96+
if (enableOwnerStacks) {
97+
console.error(
98+
'%o\n\n%s\n\n%s\n',
99+
error,
100+
componentNameMessage,
101+
recreateMessage,
102+
// We let our consoleWithStackDev wrapper add the component stack to the end.
103+
);
104+
} else {
105+
// The current Fiber is disconnected at this point which means that console printing
106+
// cannot add a component stack since it terminates at the deletion node. This is not
107+
// a problem for owner stacks which are not disconnected but for the parent component
108+
// stacks we need to use the snapshot we've previously extracted.
109+
const componentStack =
110+
errorInfo.componentStack != null ? errorInfo.componentStack : '';
111+
// Don't transform to our wrapper
112+
console['error'](
113+
'%o\n\n%s\n\n%s\n%s',
114+
error,
115+
componentNameMessage,
116+
recreateMessage,
117+
componentStack,
118+
);
119+
}
86120
} else {
87121
// In production, we print the error directly.
88122
// This will include the message, the JS stack, and anything the browser wants to show.
89123
// We pass the error object instead of custom message so that the browser displays the error natively.
90-
console['error'](error); // Don't transform to our wrapper
124+
console['error'](error); // Don't transform to our wrapper, however, React DevTools can still add a stack.
91125
}
92126
}
93127

‎packages/react-reconciler/src/ReactFiberThrow.js‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,10 @@ import {
8787
import {ConcurrentRoot} from './ReactRootTags';
8888
import {noopSuspenseyCommitThenable} from './ReactFiberThenable';
8989
import {REACT_POSTPONE_TYPE} from 'shared/ReactSymbols';
90+
import {
91+
setCurrentDebugFiberInDEV,
92+
getCurrentFiber as getCurrentDebugFiberInDEV,
93+
} from './ReactCurrentFiber';
9094

9195
function createRootErrorUpdate(
9296
root: FiberRoot,
@@ -100,7 +104,10 @@ function createRootErrorUpdate(
100104
// being called "element".
101105
update.payload = {element: null};
102106
update.callback = () => {
107+
const prevFiber = getCurrentDebugFiberInDEV(); // should just be the root
108+
setCurrentDebugFiberInDEV(errorInfo.source);
103109
logUncaughtError(root, errorInfo);
110+
setCurrentDebugFiberInDEV(prevFiber);
104111
};
105112
return update;
106113
}
@@ -127,7 +134,10 @@ function initializeClassErrorUpdate(
127134
if (__DEV__) {
128135
markFailedErrorBoundaryForHotReloading(fiber);
129136
}
137+
const prevFiber = getCurrentDebugFiberInDEV(); // should be the error boundary
138+
setCurrentDebugFiberInDEV(errorInfo.source);
130139
logCaughtError(root, fiber, errorInfo);
140+
setCurrentDebugFiberInDEV(prevFiber);
131141
};
132142
}
133143

@@ -138,7 +148,10 @@ function initializeClassErrorUpdate(
138148
if (__DEV__) {
139149
markFailedErrorBoundaryForHotReloading(fiber);
140150
}
151+
const prevFiber = getCurrentDebugFiberInDEV(); // should be the error boundary
152+
setCurrentDebugFiberInDEV(errorInfo.source);
141153
logCaughtError(root, fiber, errorInfo);
154+
setCurrentDebugFiberInDEV(prevFiber);
142155
if (typeof getDerivedStateFromError !== 'function') {
143156
// To preserve the preexisting retry behavior of error boundaries,
144157
// we keep track of which ones already failed during this batch.

‎packages/react-reconciler/src/ReactFiberWorkLoop.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3029,7 +3029,9 @@ function commitRootImpl(
30293029
for (let i = 0; i < recoverableErrors.length; i++) {
30303030
const recoverableError = recoverableErrors[i];
30313031
const errorInfo = makeErrorInfo(recoverableError.stack);
3032+
setCurrentDebugFiberInDEV(recoverableError.source);
30323033
onRecoverableError(recoverableError.value, errorInfo);
3034+
resetCurrentDebugFiberInDEV();
30333035
}
30343036
}
30353037

‎packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1512,7 +1512,7 @@ describe('ReactIncrementalErrorHandling', () => {
15121512
expect(console.error).toHaveBeenCalledTimes(1);
15131513
expect(console.error.mock.calls[0][1]).toBe(notAnError);
15141514
expect(console.error.mock.calls[0][2]).toContain(
1515-
'The above error occurred in the <BadRender> component:',
1515+
'The above error occurred in the <BadRender> component',
15161516
);
15171517
} else {
15181518
expect(console.error).toHaveBeenCalledTimes(1);

0 commit comments

Comments
 (0)