Skip to content

Commit 7f2e28a

Browse files
gnoffAndyPengc12
gnoff
authored and
AndyPengc12
committed
[Fizz] Add Component Stacks to onError and onPostpone when in dev mode or during prerenders in prod mode (facebook#27761)
Historically React would produce component stacks for dev builds only. There is a cost to tracking component stacks and given the prod builds try to optimize runtime performance these stacks were left out. More recently React added production component stacks to Fiber in because it can be immensely helpful in tracking down hard to debug production issues. Fizz was not updated to have a similar behavior. With the advent of prerendering however stacks for production in Fizz are more relevant because prerendering is not really a dev-time task. If you want the ability to reason about errors or postpones that happen during a prerender having component stacks to interrogate is helpful and these component stacks need to be available in production otherwise you are really never going to see them. (it is possible that you could do dev-mode prerenders but we don't expect this to be a common dev mode workflow) To better support the prerender use case and to make error logging in Fizz more useful the following changes have been made 1. `onPostpone` now accepts a second `postponeInfo` argument which will contain a componentStack. Postpones always originate from a component render so the stack should be consistently available. The type however will indicate the stack is optional so we can remove them in the future if we decide the overhead is the wrong tradeoff in certain cases 2. `onError` now accepts a second `errorInfo` argument which may contain a componentStack. If an error originated from a component a stack will be included in the following cases. This change entails tracking the component hierarchy in prod builds now. While this isn't cost free it is implemented in a relatively lean manner. Deferring the most expensive work (reifying the stack) until we are actually in an error pathway. In the course of implementing this change a number of simplifications were made to the code which should make the stack tracking more resilient. We no longer use a module global to curry the stack up to some handler. This was delicate because you needed to always reset it properly. We now curry the stack on the task itself. Another change made was to track the component stack on SuspenseBoundary instances so that we can provide the stack when aborting suspense boundaries to help you determine which ones were affected by an abort.
1 parent f5e1c3b commit 7f2e28a

File tree

9 files changed

+348
-294
lines changed

9 files changed

+348
-294
lines changed

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

Lines changed: 45 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -734,7 +734,7 @@ describe('ReactDOMFizzServer', () => {
734734

735735
const theError = new Error('Test');
736736
const loggedErrors = [];
737-
function onError(x) {
737+
function onError(x, errorInfo) {
738738
loggedErrors.push(x);
739739
return 'Hash of (' + x.message + ')';
740740
}
@@ -837,7 +837,7 @@ describe('ReactDOMFizzServer', () => {
837837

838838
const theError = new Error('Test');
839839
const loggedErrors = [];
840-
function onError(x) {
840+
function onError(x, errorInfo) {
841841
loggedErrors.push(x);
842842
return 'hash of (' + x.message + ')';
843843
}
@@ -898,7 +898,7 @@ describe('ReactDOMFizzServer', () => {
898898
[
899899
theError.message,
900900
expectedDigest,
901-
componentStack(['Suspense', 'div', 'App']),
901+
componentStack(['Lazy', 'Suspense', 'div', 'App']),
902902
],
903903
],
904904
[
@@ -936,7 +936,9 @@ describe('ReactDOMFizzServer', () => {
936936
return (
937937
<div>
938938
<Suspense fallback={<span>loading...</span>}>
939-
<Erroring isClient={isClient} />
939+
<Indirection level={2}>
940+
<Erroring isClient={isClient} />
941+
</Indirection>
940942
</Suspense>
941943
</div>
942944
);
@@ -979,7 +981,15 @@ describe('ReactDOMFizzServer', () => {
979981
[
980982
theError.message,
981983
expectedDigest,
982-
componentStack(['Erroring', 'Suspense', 'div', 'App']),
984+
componentStack([
985+
'Erroring',
986+
'Indirection',
987+
'Indirection',
988+
'Indirection',
989+
'Suspense',
990+
'div',
991+
'App',
992+
]),
983993
],
984994
],
985995
[
@@ -1330,6 +1340,11 @@ describe('ReactDOMFizzServer', () => {
13301340
<AsyncText text="Hello" />
13311341
</h1>
13321342
</Suspense>
1343+
<main>
1344+
<Suspense fallback="loading...">
1345+
<AsyncText text="World" />
1346+
</Suspense>
1347+
</main>
13331348
</div>
13341349
);
13351350
}
@@ -1359,7 +1374,11 @@ describe('ReactDOMFizzServer', () => {
13591374
await waitForAll([]);
13601375

13611376
// We're still loading because we're waiting for the server to stream more content.
1362-
expect(getVisibleChildren(container)).toEqual(<div>Loading...</div>);
1377+
expect(getVisibleChildren(container)).toEqual(
1378+
<div>
1379+
Loading...<main>loading...</main>
1380+
</div>,
1381+
);
13631382

13641383
// We abort the server response.
13651384
await act(() => {
@@ -1374,26 +1393,44 @@ describe('ReactDOMFizzServer', () => {
13741393
[
13751394
'The server did not finish this Suspense boundary: The render was aborted by the server without a reason.',
13761395
expectedDigest,
1396+
// We get the stack of the task when it was aborted which is why we see `h1`
13771397
componentStack(['h1', 'Suspense', 'div', 'App']),
13781398
],
1399+
[
1400+
'The server did not finish this Suspense boundary: The render was aborted by the server without a reason.',
1401+
expectedDigest,
1402+
componentStack(['Suspense', 'main', 'div', 'App']),
1403+
],
13791404
],
13801405
[
13811406
[
13821407
'The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.',
13831408
expectedDigest,
13841409
],
1410+
[
1411+
'The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.',
1412+
expectedDigest,
1413+
],
13851414
],
13861415
);
1387-
expect(getVisibleChildren(container)).toEqual(<div>Loading...</div>);
1416+
expect(getVisibleChildren(container)).toEqual(
1417+
<div>
1418+
Loading...<main>loading...</main>
1419+
</div>,
1420+
);
13881421

13891422
// We now resolve it on the client.
1390-
await clientAct(() => resolveText('Hello'));
1423+
await clientAct(() => {
1424+
resolveText('Hello');
1425+
resolveText('World');
1426+
});
13911427
assertLog([]);
13921428

13931429
// The client rendered HTML is now in place.
13941430
expect(getVisibleChildren(container)).toEqual(
13951431
<div>
13961432
<h1>Hello</h1>
1433+
<main>World</main>
13971434
</div>,
13981435
);
13991436
});

‎packages/react-dom/src/server/ReactDOMFizzServerBrowser.js‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,11 @@
77
* @flow
88
*/
99

10-
import type {PostponedState} from 'react-server/src/ReactFizzServer';
10+
import type {
11+
PostponedState,
12+
ErrorInfo,
13+
PostponeInfo,
14+
} from 'react-server/src/ReactFizzServer';
1115
import type {ReactNodeList, ReactFormState} from 'shared/ReactTypes';
1216
import type {
1317
BootstrapScriptDescriptor,
@@ -42,8 +46,8 @@ type Options = {
4246
bootstrapModules?: Array<string | BootstrapScriptDescriptor>,
4347
progressiveChunkSize?: number,
4448
signal?: AbortSignal,
45-
onError?: (error: mixed) => ?string,
46-
onPostpone?: (reason: string) => void,
49+
onError?: (error: mixed, errorInfo: ErrorInfo) => ?string,
50+
onPostpone?: (reason: string, postponeInfo: PostponeInfo) => void,
4751
unstable_externalRuntimeSrc?: string | BootstrapScriptDescriptor,
4852
importMap?: ImportMap,
4953
formState?: ReactFormState<any, any> | null,

‎packages/react-dom/src/server/ReactDOMFizzServerBun.js‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import type {
1313
HeadersDescriptor,
1414
} from 'react-dom-bindings/src/server/ReactFizzConfigDOM';
1515
import type {ImportMap} from '../shared/ReactDOMTypes';
16+
import type {ErrorInfo, PostponeInfo} from 'react-server/src/ReactFizzServer';
1617

1718
import ReactVersion from 'shared/ReactVersion';
1819

@@ -39,8 +40,8 @@ type Options = {
3940
bootstrapModules?: Array<string | BootstrapScriptDescriptor>,
4041
progressiveChunkSize?: number,
4142
signal?: AbortSignal,
42-
onError?: (error: mixed) => ?string,
43-
onPostpone?: (reason: string) => void,
43+
onError?: (error: mixed, errorInfo: ErrorInfo) => ?string,
44+
onPostpone?: (reason: string, postponeInfo: PostponeInfo) => void,
4445
unstable_externalRuntimeSrc?: string | BootstrapScriptDescriptor,
4546
importMap?: ImportMap,
4647
formState?: ReactFormState<any, any> | null,

‎packages/react-dom/src/server/ReactDOMFizzServerEdge.js‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,11 @@
77
* @flow
88
*/
99

10-
import type {PostponedState} from 'react-server/src/ReactFizzServer';
10+
import type {
11+
PostponedState,
12+
ErrorInfo,
13+
PostponeInfo,
14+
} from 'react-server/src/ReactFizzServer';
1115
import type {ReactNodeList, ReactFormState} from 'shared/ReactTypes';
1216
import type {
1317
BootstrapScriptDescriptor,
@@ -42,8 +46,8 @@ type Options = {
4246
bootstrapModules?: Array<string | BootstrapScriptDescriptor>,
4347
progressiveChunkSize?: number,
4448
signal?: AbortSignal,
45-
onError?: (error: mixed) => ?string,
46-
onPostpone?: (reason: string) => void,
49+
onError?: (error: mixed, errorInfo: ErrorInfo) => ?string,
50+
onPostpone?: (reason: string, postponeInfo: PostponeInfo) => void,
4751
unstable_externalRuntimeSrc?: string | BootstrapScriptDescriptor,
4852
importMap?: ImportMap,
4953
formState?: ReactFormState<any, any> | null,

‎packages/react-dom/src/server/ReactDOMFizzServerNode.js‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,12 @@
77
* @flow
88
*/
99

10-
import type {Request, PostponedState} from 'react-server/src/ReactFizzServer';
10+
import type {
11+
Request,
12+
PostponedState,
13+
ErrorInfo,
14+
PostponeInfo,
15+
} from 'react-server/src/ReactFizzServer';
1116
import type {ReactNodeList, ReactFormState} from 'shared/ReactTypes';
1217
import type {Writable} from 'stream';
1318
import type {
@@ -59,8 +64,8 @@ type Options = {
5964
onShellReady?: () => void,
6065
onShellError?: (error: mixed) => void,
6166
onAllReady?: () => void,
62-
onError?: (error: mixed) => ?string,
63-
onPostpone?: (reason: string) => void,
67+
onError?: (error: mixed, errorInfo: ErrorInfo) => ?string,
68+
onPostpone?: (reason: string, postponeInfo: PostponeInfo) => void,
6469
unstable_externalRuntimeSrc?: string | BootstrapScriptDescriptor,
6570
importMap?: ImportMap,
6671
formState?: ReactFormState<any, any> | null,
@@ -73,8 +78,8 @@ type ResumeOptions = {
7378
onShellReady?: () => void,
7479
onShellError?: (error: mixed) => void,
7580
onAllReady?: () => void,
76-
onError?: (error: mixed) => ?string,
77-
onPostpone?: (reason: string) => void,
81+
onError?: (error: mixed, errorInfo: ErrorInfo) => ?string,
82+
onPostpone?: (reason: string, postponeInfo: PostponeInfo) => void,
7883
};
7984

8085
type PipeableStream = {

‎packages/react-dom/src/server/ReactDOMFizzStaticBrowser.js‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,11 @@ import type {
1212
BootstrapScriptDescriptor,
1313
HeadersDescriptor,
1414
} from 'react-dom-bindings/src/server/ReactFizzConfigDOM';
15-
import type {PostponedState} from 'react-server/src/ReactFizzServer';
15+
import type {
16+
PostponedState,
17+
ErrorInfo,
18+
PostponeInfo,
19+
} from 'react-server/src/ReactFizzServer';
1620
import type {ImportMap} from '../shared/ReactDOMTypes';
1721

1822
import ReactVersion from 'shared/ReactVersion';
@@ -40,8 +44,8 @@ type Options = {
4044
bootstrapModules?: Array<string | BootstrapScriptDescriptor>,
4145
progressiveChunkSize?: number,
4246
signal?: AbortSignal,
43-
onError?: (error: mixed) => ?string,
44-
onPostpone?: (reason: string) => void,
47+
onError?: (error: mixed, errorInfo: ErrorInfo) => ?string,
48+
onPostpone?: (reason: string, postponeInfo: PostponeInfo) => void,
4549
unstable_externalRuntimeSrc?: string | BootstrapScriptDescriptor,
4650
importMap?: ImportMap,
4751
onHeaders?: (headers: Headers) => void,

‎packages/react-dom/src/server/ReactDOMFizzStaticEdge.js‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,11 @@ import type {
1212
BootstrapScriptDescriptor,
1313
HeadersDescriptor,
1414
} from 'react-dom-bindings/src/server/ReactFizzConfigDOM';
15-
import type {PostponedState} from 'react-server/src/ReactFizzServer';
15+
import type {
16+
PostponedState,
17+
ErrorInfo,
18+
PostponeInfo,
19+
} from 'react-server/src/ReactFizzServer';
1620
import type {ImportMap} from '../shared/ReactDOMTypes';
1721

1822
import ReactVersion from 'shared/ReactVersion';
@@ -40,8 +44,8 @@ type Options = {
4044
bootstrapModules?: Array<string | BootstrapScriptDescriptor>,
4145
progressiveChunkSize?: number,
4246
signal?: AbortSignal,
43-
onError?: (error: mixed) => ?string,
44-
onPostpone?: (reason: string) => void,
47+
onError?: (error: mixed, errorInfo: ErrorInfo) => ?string,
48+
onPostpone?: (reason: string, postponeInfo: PostponeInfo) => void,
4549
unstable_externalRuntimeSrc?: string | BootstrapScriptDescriptor,
4650
importMap?: ImportMap,
4751
onHeaders?: (headers: Headers) => void,

‎packages/react-dom/src/server/ReactDOMFizzStaticNode.js‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,11 @@ import type {
1212
BootstrapScriptDescriptor,
1313
HeadersDescriptor,
1414
} from 'react-dom-bindings/src/server/ReactFizzConfigDOM';
15-
import type {PostponedState} from 'react-server/src/ReactFizzServer';
15+
import type {
16+
PostponedState,
17+
ErrorInfo,
18+
PostponeInfo,
19+
} from 'react-server/src/ReactFizzServer';
1620
import type {ImportMap} from '../shared/ReactDOMTypes';
1721

1822
import {Writable, Readable} from 'stream';
@@ -41,8 +45,8 @@ type Options = {
4145
bootstrapModules?: Array<string | BootstrapScriptDescriptor>,
4246
progressiveChunkSize?: number,
4347
signal?: AbortSignal,
44-
onError?: (error: mixed) => ?string,
45-
onPostpone?: (reason: string) => void,
48+
onError?: (error: mixed, errorInfo: ErrorInfo) => ?string,
49+
onPostpone?: (reason: string, postponeInfo: PostponeInfo) => void,
4650
unstable_externalRuntimeSrc?: string | BootstrapScriptDescriptor,
4751
importMap?: ImportMap,
4852
onHeaders?: (headers: HeadersDescriptor) => void,

0 commit comments

Comments
 (0)