Skip to content

Commit

Permalink
[Fizz] handle errors in onHeaders (#27712)
Browse files Browse the repository at this point in the history
`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.

DiffTrain build for [ee68446](ee68446)
  • Loading branch information
gnoff committed Nov 15, 2023
1 parent 9e50f2a commit cecbf4f
Show file tree
Hide file tree
Showing 9 changed files with 298 additions and 284 deletions.
2 changes: 1 addition & 1 deletion compiled/facebook-www/REVISION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
593ecee66a609d4a4c2b36b39b1e5e23b2456dd1
ee68446ff198755bd38202ac9139275b657968b0
2 changes: 1 addition & 1 deletion compiled/facebook-www/ReactART-dev.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ if (__DEV__) {
return self;
}

var ReactVersion = "18.3.0-www-modern-7ac3e370";
var ReactVersion = "18.3.0-www-modern-b8021dcf";

var LegacyRoot = 0;
var ConcurrentRoot = 1;
Expand Down
4 changes: 2 additions & 2 deletions compiled/facebook-www/ReactART-prod.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -9905,7 +9905,7 @@ var slice = Array.prototype.slice,
return null;
},
bundleType: 0,
version: "18.3.0-www-modern-fc53dc79",
version: "18.3.0-www-modern-88df5dd9",
rendererPackageName: "react-art"
};
var internals$jscomp$inline_1302 = {
Expand Down Expand Up @@ -9936,7 +9936,7 @@ var internals$jscomp$inline_1302 = {
scheduleRoot: null,
setRefreshHandler: null,
getCurrentFiber: null,
reconcilerVersion: "18.3.0-www-modern-fc53dc79"
reconcilerVersion: "18.3.0-www-modern-88df5dd9"
};
if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
var hook$jscomp$inline_1303 = __REACT_DEVTOOLS_GLOBAL_HOOK__;
Expand Down
37 changes: 20 additions & 17 deletions compiled/facebook-www/ReactDOMServer-dev.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ if (__DEV__) {
var React = require("react");
var ReactDOM = require("react-dom");

var ReactVersion = "18.3.0-www-classic-deec0503";
var ReactVersion = "18.3.0-www-classic-e27c8048";

// This refers to a WWW module.
var warningWWW = require("warning");
Expand Down Expand Up @@ -7698,6 +7698,9 @@ if (__DEV__) {
var headers = renderState.headers;

if (headers) {
// Even if onHeaders throws we don't want to call this again so
// we drop the headers state from this point onwards.
renderState.headers = null;
var linkHeader = headers.preconnects;

if (headers.fontPreloads) {
Expand Down Expand Up @@ -7780,7 +7783,6 @@ if (__DEV__) {
onHeaders({});
}

renderState.headers = null;
return;
}
}
Expand Down Expand Up @@ -13219,6 +13221,19 @@ if (__DEV__) {
if (request.allPendingTasks === 0) {
completeAll(request);
}
}

function safelyEmitEarlyPreloads(request, shellComplete) {
try {
emitEarlyPreloads(
request.renderState,
request.resumableState,
shellComplete
);
} catch (error) {
// We assume preloads are optimistic and thus non-fatal if errored.
logRecoverableError(request, error);
}
} // I extracted this function out because we want to ensure we consistently emit preloads before
// transitioning to the next request stage and this transition can happen in multiple places in this
// implementation.
Expand All @@ -13231,11 +13246,7 @@ if (__DEV__) {
// we should only be calling completeShell when the shell is complete so we
// just use a literal here
var shellComplete = true;
emitEarlyPreloads(
request.renderState,
request.resumableState,
shellComplete
);
safelyEmitEarlyPreloads(request, shellComplete);
} // We have completed the shell so the shell can't error anymore.

request.onShellError = noop;
Expand All @@ -13255,11 +13266,7 @@ if (__DEV__) {
? true // Prerender Request, we use the state of the root segment
: request.completedRootSegment === null ||
request.completedRootSegment.status !== POSTPONED;
emitEarlyPreloads(
request.renderState,
request.resumableState,
shellComplete
);
safelyEmitEarlyPreloads(request, shellComplete);
var onAllReady = request.onAllReady;
onAllReady();
}
Expand Down Expand Up @@ -14102,11 +14109,7 @@ if (__DEV__) {

function enqueueEarlyPreloadsAfterInitialWork(request) {
var shellComplete = request.pendingRootTasks === 0;
emitEarlyPreloads(
request.renderState,
request.resumableState,
shellComplete
);
safelyEmitEarlyPreloads(request, shellComplete);
}

function enqueueFlush(request) {
Expand Down
37 changes: 20 additions & 17 deletions compiled/facebook-www/ReactDOMServer-dev.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ if (__DEV__) {
var React = require("react");
var ReactDOM = require("react-dom");

var ReactVersion = "18.3.0-www-modern-7ac3e370";
var ReactVersion = "18.3.0-www-modern-b8021dcf";

// This refers to a WWW module.
var warningWWW = require("warning");
Expand Down Expand Up @@ -7698,6 +7698,9 @@ if (__DEV__) {
var headers = renderState.headers;

if (headers) {
// Even if onHeaders throws we don't want to call this again so
// we drop the headers state from this point onwards.
renderState.headers = null;
var linkHeader = headers.preconnects;

if (headers.fontPreloads) {
Expand Down Expand Up @@ -7780,7 +7783,6 @@ if (__DEV__) {
onHeaders({});
}

renderState.headers = null;
return;
}
}
Expand Down Expand Up @@ -12947,6 +12949,19 @@ if (__DEV__) {
if (request.allPendingTasks === 0) {
completeAll(request);
}
}

function safelyEmitEarlyPreloads(request, shellComplete) {
try {
emitEarlyPreloads(
request.renderState,
request.resumableState,
shellComplete
);
} catch (error) {
// We assume preloads are optimistic and thus non-fatal if errored.
logRecoverableError(request, error);
}
} // I extracted this function out because we want to ensure we consistently emit preloads before
// transitioning to the next request stage and this transition can happen in multiple places in this
// implementation.
Expand All @@ -12959,11 +12974,7 @@ if (__DEV__) {
// we should only be calling completeShell when the shell is complete so we
// just use a literal here
var shellComplete = true;
emitEarlyPreloads(
request.renderState,
request.resumableState,
shellComplete
);
safelyEmitEarlyPreloads(request, shellComplete);
} // We have completed the shell so the shell can't error anymore.

request.onShellError = noop;
Expand All @@ -12983,11 +12994,7 @@ if (__DEV__) {
? true // Prerender Request, we use the state of the root segment
: request.completedRootSegment === null ||
request.completedRootSegment.status !== POSTPONED;
emitEarlyPreloads(
request.renderState,
request.resumableState,
shellComplete
);
safelyEmitEarlyPreloads(request, shellComplete);
var onAllReady = request.onAllReady;
onAllReady();
}
Expand Down Expand Up @@ -13830,11 +13837,7 @@ if (__DEV__) {

function enqueueEarlyPreloadsAfterInitialWork(request) {
var shellComplete = request.pendingRootTasks === 0;
emitEarlyPreloads(
request.renderState,
request.resumableState,
shellComplete
);
safelyEmitEarlyPreloads(request, shellComplete);
}

function enqueueFlush(request) {
Expand Down
Loading

0 comments on commit cecbf4f

Please sign in to comment.