Skip to content

Commit aae244d

Browse files
sebmarkbagesebmarkbage
committed
Print the error in DEV with addendum
Since we no longer get the error printed by the browser automatically we have to manually print the error with an addendum which contains component stacks.
1 parent 944275e commit aae244d

11 files changed

+219
-122
lines changed

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

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,10 +130,17 @@ describe('ReactDOMConsoleErrorReporting', () => {
130130
expect(windowOnError.mock.calls).toEqual([]);
131131
expect(console.error.mock.calls).toEqual([
132132
[
133+
// Formatting
134+
expect.stringContaining('%o'),
135+
expect.objectContaining({
136+
message: 'Boom',
137+
}),
133138
// Addendum by React:
134139
expect.stringContaining(
135140
'The above error occurred in the <Foo> component',
136141
),
142+
expect.stringContaining('Foo'),
143+
expect.stringContaining('Consider adding an error boundary'),
137144
],
138145
]);
139146
} else {
@@ -183,10 +190,17 @@ describe('ReactDOMConsoleErrorReporting', () => {
183190
expect(windowOnError.mock.calls).toEqual([]);
184191
expect(console.error.mock.calls).toEqual([
185192
[
193+
// Formatting
194+
expect.stringContaining('%o'),
195+
expect.objectContaining({
196+
message: 'Boom',
197+
}),
186198
// Addendum by React:
187199
expect.stringContaining(
188200
'The above error occurred in the <Foo> component',
189201
),
202+
expect.stringContaining('Foo'),
203+
expect.stringContaining('ErrorBoundary'),
190204
],
191205
]);
192206
} else {
@@ -236,10 +250,17 @@ describe('ReactDOMConsoleErrorReporting', () => {
236250
expect(windowOnError.mock.calls).toEqual([]);
237251
expect(console.error.mock.calls).toEqual([
238252
[
253+
// Formatting
254+
expect.stringContaining('%o'),
255+
expect.objectContaining({
256+
message: 'Boom',
257+
}),
239258
// Addendum by React:
240259
expect.stringContaining(
241260
'The above error occurred in the <Foo> component',
242261
),
262+
expect.stringContaining('Foo'),
263+
expect.stringContaining('Consider adding an error boundary'),
243264
],
244265
]);
245266
} else {
@@ -292,10 +313,17 @@ describe('ReactDOMConsoleErrorReporting', () => {
292313
expect(windowOnError.mock.calls).toEqual([]);
293314
expect(console.error.mock.calls).toEqual([
294315
[
316+
// Formatting
317+
expect.stringContaining('%o'),
318+
expect.objectContaining({
319+
message: 'Boom',
320+
}),
295321
// Addendum by React:
296322
expect.stringContaining(
297323
'The above error occurred in the <Foo> component',
298324
),
325+
expect.stringContaining('Foo'),
326+
expect.stringContaining('ErrorBoundary'),
299327
],
300328
]);
301329
} else {
@@ -345,10 +373,17 @@ describe('ReactDOMConsoleErrorReporting', () => {
345373
expect(windowOnError.mock.calls).toEqual([]);
346374
expect(console.error.mock.calls).toEqual([
347375
[
376+
// Formatting
377+
expect.stringContaining('%o'),
378+
expect.objectContaining({
379+
message: 'Boom',
380+
}),
348381
// Addendum by React:
349382
expect.stringContaining(
350383
'The above error occurred in the <Foo> component',
351384
),
385+
expect.stringContaining('Foo'),
386+
expect.stringContaining('Consider adding an error boundary'),
352387
],
353388
]);
354389
} else {
@@ -401,10 +436,17 @@ describe('ReactDOMConsoleErrorReporting', () => {
401436
expect(windowOnError.mock.calls).toEqual([]);
402437
expect(console.error.mock.calls).toEqual([
403438
[
439+
// Formatting
440+
expect.stringContaining('%o'),
441+
expect.objectContaining({
442+
message: 'Boom',
443+
}),
404444
// Addendum by React:
405445
expect.stringContaining(
406446
'The above error occurred in the <Foo> component',
407447
),
448+
expect.stringContaining('Foo'),
449+
expect.stringContaining('ErrorBoundary'),
408450
],
409451
]);
410452
} else {

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

Lines changed: 42 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -159,10 +159,17 @@ describe('ReactDOMConsoleErrorReporting', () => {
159159
expect(console.error.mock.calls).toEqual([
160160
[expect.stringContaining('ReactDOM.render is no longer supported')],
161161
[
162+
// Formatting
163+
expect.stringContaining('%o'),
164+
expect.objectContaining({
165+
message: 'Boom',
166+
}),
162167
// Addendum by React:
163168
expect.stringContaining(
164169
'The above error occurred in the <Foo> component',
165170
),
171+
expect.stringContaining('Foo'),
172+
expect.stringContaining('Consider adding an error boundary'),
166173
],
167174
]);
168175
} else {
@@ -215,12 +222,18 @@ describe('ReactDOMConsoleErrorReporting', () => {
215222
expect(windowOnError.mock.calls).toEqual([]);
216223
expect(console.error.mock.calls).toEqual([
217224
[expect.stringContaining('ReactDOM.render is no longer supported')],
218-
// TODO: There is no log for this error now.
219225
[
226+
// Formatting
227+
expect.stringContaining('%o'),
228+
expect.objectContaining({
229+
message: 'Boom',
230+
}),
220231
// Addendum by React:
221232
expect.stringContaining(
222233
'The above error occurred in the <Foo> component',
223234
),
235+
expect.stringContaining('Foo'),
236+
expect.stringContaining('ErrorBoundary'),
224237
],
225238
]);
226239
} else {
@@ -271,12 +284,18 @@ describe('ReactDOMConsoleErrorReporting', () => {
271284
expect(windowOnError.mock.calls).toEqual([]);
272285
expect(console.error.mock.calls).toEqual([
273286
[expect.stringContaining('ReactDOM.render is no longer supported')],
274-
// TODO: There should be an error reported here.
275287
[
288+
// Formatting
289+
expect.stringContaining('%o'),
290+
expect.objectContaining({
291+
message: 'Boom',
292+
}),
276293
// Addendum by React:
277294
expect.stringContaining(
278295
'The above error occurred in the <Foo> component',
279296
),
297+
expect.stringContaining('Foo'),
298+
expect.stringContaining('Consider adding an error boundary'),
280299
],
281300
]);
282301
} else {
@@ -332,12 +351,18 @@ describe('ReactDOMConsoleErrorReporting', () => {
332351
expect(windowOnError.mock.calls).toEqual([]);
333352
expect(console.error.mock.calls).toEqual([
334353
[expect.stringContaining('ReactDOM.render is no longer supported')],
335-
// TODO: There should be a log here.
336354
[
355+
// Formatting
356+
expect.stringContaining('%o'),
357+
expect.objectContaining({
358+
message: 'Boom',
359+
}),
337360
// Addendum by React:
338361
expect.stringContaining(
339362
'The above error occurred in the <Foo> component',
340363
),
364+
expect.stringContaining('Foo'),
365+
expect.stringContaining('ErrorBoundary'),
341366
],
342367
]);
343368
} else {
@@ -389,12 +414,18 @@ describe('ReactDOMConsoleErrorReporting', () => {
389414
expect(windowOnError.mock.calls).toEqual([]);
390415
expect(console.error.mock.calls).toEqual([
391416
[expect.stringContaining('ReactDOM.render is no longer supported')],
392-
// TODO: We should have logged an error.
393417
[
418+
// Formatting
419+
expect.stringContaining('%o'),
420+
expect.objectContaining({
421+
message: 'Boom',
422+
}),
394423
// Addendum by React:
395424
expect.stringContaining(
396425
'The above error occurred in the <Foo> component',
397426
),
427+
expect.stringContaining('Foo'),
428+
expect.stringContaining('Consider adding an error boundary'),
398429
],
399430
]);
400431
} else {
@@ -450,12 +481,18 @@ describe('ReactDOMConsoleErrorReporting', () => {
450481
expect(windowOnError.mock.calls).toEqual([]);
451482
expect(console.error.mock.calls).toEqual([
452483
[expect.stringContaining('ReactDOM.render is no longer supported')],
453-
// TODO: There should be a log here.
454484
[
485+
// Formatting
486+
expect.stringContaining('%o'),
487+
expect.objectContaining({
488+
message: 'Boom',
489+
}),
455490
// Addendum by React:
456491
expect.stringContaining(
457492
'The above error occurred in the <Foo> component',
458493
),
494+
expect.stringContaining('Foo'),
495+
expect.stringContaining('ErrorBoundary'),
459496
],
460497
]);
461498
} else {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -707,7 +707,7 @@ describe('ReactErrorBoundaries', () => {
707707
});
708708
if (__DEV__) {
709709
expect(console.error).toHaveBeenCalledTimes(1);
710-
expect(console.error.mock.calls[0][0]).toContain(
710+
expect(console.error.mock.calls[0][2]).toContain(
711711
'The above error occurred in the <BrokenRender> component:',
712712
);
713713
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -686,7 +686,7 @@ describe('ReactLegacyErrorBoundaries', () => {
686686
expect(console.error.mock.calls[0][0]).toContain(
687687
'ReactDOM.render is no longer supported',
688688
);
689-
expect(console.error.mock.calls[1][0]).toContain(
689+
expect(console.error.mock.calls[1][2]).toContain(
690690
'The above error occurred in the <BrokenRender> component:',
691691
);
692692
}

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

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -243,21 +243,6 @@ function shouldProfile(current: Fiber): boolean {
243243
);
244244
}
245245

246-
export function reportUncaughtErrorInDEV(error: mixed) {
247-
// Wrapping each small part of the commit phase into a guarded
248-
// callback is a bit too slow (https://github.com/facebook/react/pull/21666).
249-
// But we rely on it to surface errors to DEV tools like overlays
250-
// (https://github.com/facebook/react/issues/21712).
251-
// As a compromise, rethrow only caught errors in a guard.
252-
if (__DEV__) {
253-
// TODO: This trick no longer works. Should probably use reportError maybe.
254-
// invokeGuardedCallback(null, () => {
255-
// throw error;
256-
// });
257-
// clearCaughtError();
258-
}
259-
}
260-
261246
function callComponentWillUnmountWithTimer(current: Fiber, instance: any) {
262247
instance.props = current.memoizedProps;
263248
instance.state = current.memoizedState;

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

Lines changed: 12 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import type {Fiber} from './ReactInternalTypes';
1111
import type {CapturedValue} from './ReactCapturedValue';
1212

1313
import {showErrorDialog} from './ReactFiberErrorDialog';
14-
import {ClassComponent} from './ReactWorkTags';
1514
import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber';
1615
import {HostRoot} from 'react-reconciler/src/ReactWorkTags';
1716

@@ -33,25 +32,8 @@ export function logCapturedError(
3332
const source = errorInfo.source;
3433
const stack = errorInfo.stack;
3534
const componentStack = stack !== null ? stack : '';
36-
// Browsers support silencing uncaught errors by calling
37-
// `preventDefault()` in window `error` handler.
38-
// We record this information as an expando on the error.
39-
// TODO: We no longer track this.
40-
if (error != null && error._suppressLogging) {
41-
if (boundary.tag === ClassComponent) {
42-
// The error is recoverable and was silenced.
43-
// Ignore it and don't print the stack addendum.
44-
// This is handy for testing error boundaries without noise.
45-
return;
46-
}
47-
// The error is fatal. Since the silencing might have
48-
// been accidental, we'll surface it anyway.
49-
// However, the browser would have silenced the original error
50-
// so we'll print it first, and then print the stack addendum.
51-
console['error'](error); // Don't transform to our wrapper
52-
// For a more detailed description of this block, see:
53-
// https://github.com/facebook/react/pull/13384
54-
}
35+
// TODO: There's no longer a way to silence these warnings e.g. for tests.
36+
// See https://github.com/facebook/react/pull/13384
5537

5638
const componentName = source ? getComponentNameFromFiber(source) : null;
5739
const componentNameMessage = componentName
@@ -70,16 +52,17 @@ export function logCapturedError(
7052
`React will try to recreate this component tree from scratch ` +
7153
`using the error boundary you provided, ${errorBoundaryName}.`;
7254
}
73-
const combinedMessage =
74-
`${componentNameMessage}\n${componentStack}\n\n` +
75-
`${errorBoundaryMessage}`;
7655

77-
// TODO: The error is no longer printed by the browser.
78-
// In development, we provide our own message with just the component stack.
79-
// We don't include the original error message and JS stack because the browser
80-
// has already printed it. Even if the application swallows the error, it is still
81-
// displayed by the browser thanks to the DEV-only fake event trick in ReactErrorUtils.
82-
console['error'](combinedMessage); // Don't transform to our wrapper
56+
// In development, we provide our own message which includes the component stack
57+
// in addition to the error.
58+
console['error'](
59+
// Don't transform to our wrapper
60+
'%o\n\n%s\n%s\n\n%s',
61+
error,
62+
componentNameMessage,
63+
componentStack,
64+
errorBoundaryMessage,
65+
);
8366
} else {
8467
// In production, we print the error directly.
8568
// This will include the message, the JS stack, and anything the browser wants to show.

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,6 @@ import {
186186
reconnectPassiveEffects,
187187
reappearLayoutEffects,
188188
disconnectPassiveEffect,
189-
reportUncaughtErrorInDEV,
190189
invokeLayoutEffectMountInDEV,
191190
invokePassiveEffectMountInDEV,
192191
invokeLayoutEffectUnmountInDEV,
@@ -3384,7 +3383,6 @@ export function captureCommitPhaseError(
33843383
error: mixed,
33853384
) {
33863385
if (__DEV__) {
3387-
reportUncaughtErrorInDEV(error);
33883386
setIsRunningInsertionEffect(false);
33893387
}
33903388
if (sourceFiber.tag === HostRoot) {

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1510,7 +1510,8 @@ describe('ReactIncrementalErrorHandling', () => {
15101510

15111511
if (__DEV__) {
15121512
expect(console.error).toHaveBeenCalledTimes(1);
1513-
expect(console.error.mock.calls[0][0]).toContain(
1513+
expect(console.error.mock.calls[0][1]).toBe(notAnError);
1514+
expect(console.error.mock.calls[0][2]).toContain(
15141515
'The above error occurred in the <BadRender> component:',
15151516
);
15161517
} else {
@@ -1911,7 +1912,7 @@ describe('ReactIncrementalErrorHandling', () => {
19111912
expect(console.error.mock.calls[0][0]).toContain(
19121913
'Cannot update a component (`%s`) while rendering a different component',
19131914
);
1914-
expect(console.error.mock.calls[1][0]).toContain(
1915+
expect(console.error.mock.calls[1][2]).toContain(
19151916
'The above error occurred in the <App> component',
19161917
);
19171918
}

0 commit comments

Comments
 (0)