Skip to content

Commit a5764c4

Browse files
committed
[Fizz] handle errors in onHeaders
`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.
1 parent aec521a commit a5764c4

File tree

4 files changed

+69
-9
lines changed

4 files changed

+69
-9
lines changed

packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6131,6 +6131,10 @@ export function emitEarlyPreloads(
61316131
if (onHeaders) {
61326132
const headers = renderState.headers;
61336133
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+
61346138
let linkHeader = headers.preconnects;
61356139
if (headers.fontPreloads) {
61366140
if (linkHeader) {
@@ -6205,7 +6209,6 @@ export function emitEarlyPreloads(
62056209
// it React will not provide any headers
62066210
onHeaders({});
62076211
}
6208-
renderState.headers = null;
62096212
return;
62106213
}
62116214
}

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3841,6 +3841,30 @@ describe('ReactDOMFizzServer', () => {
38413841
});
38423842
});
38433843

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+
38443868
describe('error escaping', () => {
38453869
it('escapes error hash, message, and component stack values in directly flushed errors (html escaping)', async () => {
38463870
window.__outlet = {};

packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1420,6 +1420,27 @@ describe('ReactDOMFizzStaticBrowser', () => {
14201420
);
14211421
});
14221422

1423+
it('logs an error if onHeaders throws but continues the prerender', async () => {
1424+
const errors = [];
1425+
function onError(error) {
1426+
errors.push(error.message);
1427+
}
1428+
1429+
function onHeaders(x) {
1430+
throw new Error('bad onHeaders');
1431+
}
1432+
1433+
const prerendered = await ReactDOMFizzStatic.prerender(<div>hello</div>, {
1434+
onHeaders,
1435+
onError,
1436+
});
1437+
expect(prerendered.postponed).toBe(null);
1438+
expect(errors).toEqual(['bad onHeaders']);
1439+
1440+
await readIntoContainer(prerendered.prelude);
1441+
expect(getVisibleChildren(container)).toEqual(<div>hello</div>);
1442+
});
1443+
14231444
// @gate enablePostpone
14241445
it('does not bootstrap again in a resume if it bootstraps', async () => {
14251446
let prerendering = true;

packages/react-server/src/ReactFizzServer.js

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3220,6 +3220,22 @@ function abortTask(task: Task, request: Request, error: mixed): void {
32203220
}
32213221
}
32223222

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+
32233239
// I extracted this function out because we want to ensure we consistently emit preloads before
32243240
// transitioning to the next request stage and this transition can happen in multiple places in this
32253241
// implementation.
@@ -3232,11 +3248,7 @@ function completeShell(request: Request) {
32323248
// we should only be calling completeShell when the shell is complete so we
32333249
// just use a literal here
32343250
const shellComplete = true;
3235-
emitEarlyPreloads(
3236-
request.renderState,
3237-
request.resumableState,
3238-
shellComplete,
3239-
);
3251+
safelyEmitEarlyPreloads(request, shellComplete);
32403252
}
32413253
// We have completed the shell so the shell can't error anymore.
32423254
request.onShellError = noop;
@@ -3259,7 +3271,7 @@ function completeAll(request: Request) {
32593271
: // Prerender Request, we use the state of the root segment
32603272
request.completedRootSegment === null ||
32613273
request.completedRootSegment.status !== POSTPONED;
3262-
emitEarlyPreloads(request.renderState, request.resumableState, shellComplete);
3274+
safelyEmitEarlyPreloads(request, shellComplete);
32633275
const onAllReady = request.onAllReady;
32643276
onAllReady();
32653277
}
@@ -4124,7 +4136,7 @@ export function startWork(request: Request): void {
41244136

41254137
function enqueueEarlyPreloadsAfterInitialWork(request: Request) {
41264138
const shellComplete = request.pendingRootTasks === 0;
4127-
emitEarlyPreloads(request.renderState, request.resumableState, shellComplete);
4139+
safelyEmitEarlyPreloads(request, shellComplete);
41284140
}
41294141

41304142
function enqueueFlush(request: Request): void {
@@ -4168,7 +4180,7 @@ export function prepareForStartFlowingIfBeforeAllReady(request: Request) {
41684180
request.completedRootSegment === null
41694181
? request.pendingRootTasks === 0
41704182
: request.completedRootSegment.status !== POSTPONED;
4171-
emitEarlyPreloads(request.renderState, request.resumableState, shellComplete);
4183+
safelyEmitEarlyPreloads(request, shellComplete);
41724184
}
41734185

41744186
export function startFlowing(request: Request, destination: Destination): void {

0 commit comments

Comments
 (0)