[Fizz] handle errors in `onHeaders` (#27712)
`onHeaders` can throw however for now we can assume that headers are optimistic values since the only things we produce for them are preload links. This is a pragmatic decision because React could concievably have headers in the future which were not optimistic and thus non-optional however it is hard to imagine what these headers might be in practice. If we need to change this behavior to be fatal in the future it would be a breaking change. This commit adds error logging when `onHeaders` throws and ensures the request can continue to render successfully.
Josh Story committed
Nov 15, 2023 at 12:53 UTC
ee68446ff198755bd38202ac9139275b657968b0
4 files changed
+70
-9
packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js
+4
-1
@@ -6131,6 +6131,10 @@ export function emitEarlyPreloads(
6131
if (onHeaders) {
6132
const headers = renderState.headers;
6133
if (headers) {
6134
+ // Even if onHeaders throws we don't want to call this again so
6135
+ // we drop the headers state from this point onwards.
6136
+ renderState.headers = null;
6137
+
6138
let linkHeader = headers.preconnects;
6139
if (headers.fontPreloads) {
6140
if (linkHeader) {
@@ -6205,7 +6209,6 @@ export function emitEarlyPreloads(
6209
// it React will not provide any headers
6210
onHeaders({});
6211
}
6208
- renderState.headers = null;
6212
return;
6213
}
6214
}
packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
+24
@@ -3841,6 +3841,30 @@ describe('ReactDOMFizzServer', () => {
3841
});
3842
});
3843
3844
+ it('logs an error if onHeaders throws but continues the render', async () => {
3845
+ const errors = [];
3846
+ function onError(error) {
3847
+ errors.push(error.message);
3848
+ }
3849
+
3850
+ function onHeaders(x) {
3851
+ throw new Error('bad onHeaders');
3852
+ }
3853
+
3854
+ let pipe;
3855
+ await act(() => {
3856
+ ({pipe} = renderToPipeableStream(<div>hello</div>, {onHeaders, onError}));
3857
+ });
3858
+
3859
+ expect(errors).toEqual(['bad onHeaders']);
3860
+
3861
+ await act(() => {
3862
+ pipe(writable);
3863
+ });
3864
+
3865
+ expect(getVisibleChildren(container)).toEqual(<div>hello</div>);
3866
+ });
3867
+
3868
describe('error escaping', () => {
3869
it('escapes error hash, message, and component stack values in directly flushed errors (html escaping)', async () => {
3870
window.__outlet = {};
packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js
+22
@@ -1420,6 +1420,28 @@ describe('ReactDOMFizzStaticBrowser', () => {
1420
);
1421
});
1422
1423
+ // @gate experimental
1424
+ it('logs an error if onHeaders throws but continues the prerender', async () => {
1425
+ const errors = [];
1426
+ function onError(error) {
1427
+ errors.push(error.message);
1428
+ }
1429
+
1430
+ function onHeaders(x) {
1431
+ throw new Error('bad onHeaders');
1432
+ }
1433
+
1434
+ const prerendered = await ReactDOMFizzStatic.prerender(<div>hello</div>, {
1435
+ onHeaders,
1436
+ onError,
1437
+ });
1438
+ expect(prerendered.postponed).toBe(null);
1439
+ expect(errors).toEqual(['bad onHeaders']);
1440
+
1441
+ await readIntoContainer(prerendered.prelude);
1442
+ expect(getVisibleChildren(container)).toEqual(<div>hello</div>);
1443
+ });
1444
+
1445
// @gate enablePostpone
1446
it('does not bootstrap again in a resume if it bootstraps', async () => {
1447
let prerendering = true;
packages/react-server/src/ReactFizzServer.js
+20
-8
@@ -3220,6 +3220,22 @@ function abortTask(task: Task, request: Request, error: mixed): void {
3220
}
3221
}
3222
3223
+function safelyEmitEarlyPreloads(
3224
+ request: Request,
3225
+ shellComplete: boolean,
3226
+): void {
3227
+ try {
3228
+ emitEarlyPreloads(
3229
+ request.renderState,
3230
+ request.resumableState,
3231
+ shellComplete,
3232
+ );
3233
+ } catch (error) {
3234
+ // We assume preloads are optimistic and thus non-fatal if errored.
3235
+ logRecoverableError(request, error);
3236
+ }
3237
+}
3238
+
3239
// I extracted this function out because we want to ensure we consistently emit preloads before
3240
// transitioning to the next request stage and this transition can happen in multiple places in this
3241
// implementation.
@@ -3232,11 +3248,7 @@ function completeShell(request: Request) {
3248
// we should only be calling completeShell when the shell is complete so we
3249
// just use a literal here
3250
const shellComplete = true;
3235
- emitEarlyPreloads(
3236
- request.renderState,
3237
- request.resumableState,
3238
- shellComplete,
3239
- );
3251
+ safelyEmitEarlyPreloads(request, shellComplete);
3252
}
3253
// We have completed the shell so the shell can't error anymore.
3254
request.onShellError = noop;
@@ -3259,7 +3271,7 @@ function completeAll(request: Request) {
3271
: // Prerender Request, we use the state of the root segment
3272
request.completedRootSegment === null ||
3273
request.completedRootSegment.status !== POSTPONED;
3262
- emitEarlyPreloads(request.renderState, request.resumableState, shellComplete);
3274
+ safelyEmitEarlyPreloads(request, shellComplete);
3275
const onAllReady = request.onAllReady;
3276
onAllReady();
3277
}
@@ -4124,7 +4136,7 @@ export function startWork(request: Request): void {
4136
4137
function enqueueEarlyPreloadsAfterInitialWork(request: Request) {
4138
const shellComplete = request.pendingRootTasks === 0;
4127
- emitEarlyPreloads(request.renderState, request.resumableState, shellComplete);
4139
+ safelyEmitEarlyPreloads(request, shellComplete);
4140
}
4141
4142
function enqueueFlush(request: Request): void {
@@ -4168,7 +4180,7 @@ export function prepareForStartFlowingIfBeforeAllReady(request: Request) {
4180
request.completedRootSegment === null
4181
? request.pendingRootTasks === 0
4182
: request.completedRootSegment.status !== POSTPONED;
4171
- emitEarlyPreloads(request.renderState, request.resumableState, shellComplete);
4183
+ safelyEmitEarlyPreloads(request, shellComplete);
4184
}
4185
4186
export function startFlowing(request: Request, destination: Destination): void {