Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fizz] Reset error component stack and fix error messages #27456

Merged
merged 3 commits into from
Oct 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
145 changes: 145 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -981,4 +981,149 @@ describe('ReactDOMFizzStaticBrowser', () => {

expect(getVisibleChildren(container)).toEqual(<div>Hello</div>);
});

// @gate enablePostpone
it('errors if the replay does not line up', async () => {
let prerendering = true;
function Postpone() {
if (prerendering) {
React.unstable_postpone();
}
return 'Hello';
}

function Wrapper({children}) {
return children;
}

const lazySpan = React.lazy(async () => {
await 0;
return {default: <span />};
});

function App() {
const children = (
<Suspense fallback="Loading...">
<Postpone />
</Suspense>
);
return (
<>
<div>{prerendering ? <Wrapper>{children}</Wrapper> : children}</div>
<div>
{prerendering ? (
<Suspense fallback="Loading...">
<div>
<Postpone />
</div>
</Suspense>
) : (
lazySpan
)}
</div>
</>
);
}

const prerendered = await ReactDOMFizzStatic.prerender(<App />);
expect(prerendered.postponed).not.toBe(null);

await readIntoContainer(prerendered.prelude);

expect(getVisibleChildren(container)).toEqual([
<div>Loading...</div>,
<div>Loading...</div>,
]);

prerendering = false;

const errors = [];
const resumed = await ReactDOMFizzServer.resume(
<App />,
JSON.parse(JSON.stringify(prerendered.postponed)),
{
onError(x) {
errors.push(x.message);
},
},
);

expect(errors).toEqual([
'Expected the resume to render <Wrapper> in this slot but instead it rendered <Suspense>. ' +
"The tree doesn't match so React will fallback to client rendering.",
'Expected the resume to render <Suspense> in this slot but instead it rendered <span>. ' +
"The tree doesn't match so React will fallback to client rendering.",
]);

// TODO: Test the component stack but we don't expose it to the server yet.

await readIntoContainer(resumed);

// Client rendered
expect(getVisibleChildren(container)).toEqual([
<div>Loading...</div>,
<div>Loading...</div>,
]);
});

// @gate enablePostpone
it('can abort the resume', async () => {
let prerendering = true;
const infinitePromise = new Promise(() => {});
function Postpone() {
if (prerendering) {
React.unstable_postpone();
}
return 'Hello';
}

function App() {
if (!prerendering) {
React.use(infinitePromise);
}
return (
<div>
<Suspense fallback="Loading...">
<Postpone />
</Suspense>
</div>
);
}

const prerendered = await ReactDOMFizzStatic.prerender(<App />);
expect(prerendered.postponed).not.toBe(null);

await readIntoContainer(prerendered.prelude);

expect(getVisibleChildren(container)).toEqual(<div>Loading...</div>);

prerendering = false;

const controller = new AbortController();

const errors = [];

const resumedPromise = ReactDOMFizzServer.resume(
<App />,
JSON.parse(JSON.stringify(prerendered.postponed)),
{
signal: controller.signal,
onError(x) {
errors.push(x);
},
},
);

controller.abort('abort');

const resumed = await resumedPromise;
await resumed.allReady;

expect(errors).toEqual(['abort']);

await readIntoContainer(resumed);

// Client rendered
expect(getVisibleChildren(container)).toEqual(<div>Loading...</div>);
});
});
58 changes: 41 additions & 17 deletions packages/react-server/src/ReactFizzServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -1110,7 +1110,6 @@ function replaySuspenseBoundary(
// on preparing fallbacks if we don't have any more main content to task on.
request.pingedTasks.push(suspendedFallbackTask);

// TODO: Should this be in the finally?
popComponentStackInDEV(task);
}

Expand Down Expand Up @@ -1953,17 +1952,19 @@ function replayElement(
if (keyOrIndex !== node[1]) {
continue;
}
// Let's double check that the component name matches as a precaution.
if (name !== null && name !== node[0]) {
throw new Error(
'Expected to see a component of type "' +
name +
'" in this slot. ' +
"The tree doesn't match so React will fallback to client rendering.",
);
}
if (node.length === 4) {
// Matched a replayable path.
// Let's double check that the component name matches as a precaution.
if (name !== null && name !== node[0]) {
throw new Error(
'Expected the resume to render <' +
(node[0]: any) +
'> in this slot but instead it rendered <' +
name +
'>. ' +
"The tree doesn't match so React will fallback to client rendering.",
);
}
const childNodes = node[2];
const childSlots = node[3];
task.replay = {nodes: childNodes, slots: childSlots, pendingTasks: 1};
Expand Down Expand Up @@ -2009,8 +2010,13 @@ function replayElement(
} else {
// Let's double check that the component type matches.
if (type !== REACT_SUSPENSE_TYPE) {
const expectedType = 'Suspense';
throw new Error(
'Expected to see a Suspense boundary in this slot. ' +
'Expected the resume to render <' +
expectedType +
'> in this slot but instead it rendered <' +
(getComponentNameFromType(type) || 'Unknown') +
'>. ' +
"The tree doesn't match so React will fallback to client rendering.",
);
}
Expand Down Expand Up @@ -2378,6 +2384,7 @@ function replayFragment(
// in the original prerender. What's unable to complete is the child
// replay nodes which might be Suspense boundaries which are able to
// absorb the error and we can still continue with siblings.
// This is an error, stash the component stack if it is null.
erroredReplay(request, task.blockedBoundary, x, childNodes, childSlots);
} finally {
task.replay.pendingTasks--;
Expand Down Expand Up @@ -2849,6 +2856,7 @@ function renderNode(
if (__DEV__) {
task.componentStack = previousComponentStack;
}
lastBoundaryErrorComponentStackDev = null;
return;
}
}
Expand Down Expand Up @@ -2930,6 +2938,7 @@ function erroredTask(
errorDigest = logRecoverableError(request, error);
}
if (boundary === null) {
lastBoundaryErrorComponentStackDev = null;
fatalError(request, error);
} else {
boundary.pendingTasks--;
Expand All @@ -2949,6 +2958,8 @@ function erroredTask(
// We reuse the same queue for errors.
request.clientRenderedBoundaries.push(boundary);
}
} else {
lastBoundaryErrorComponentStackDev = null;
}
}

Expand Down Expand Up @@ -3077,14 +3088,14 @@ function abortTask(task: Task, request: Request, error: mixed): void {
}

if (boundary === null) {
request.allPendingTasks--;
if (request.status !== CLOSING && request.status !== CLOSED) {
const replay: null | ReplaySet = task.replay;
if (replay === null) {
// We didn't complete the root so we have nothing to show. We can close
// the request;
logRecoverableError(request, error);
fatalError(request, error);
return;
} else {
// If the shell aborts during a replay, that's not a fatal error. Instead
// we should be able to recover by client rendering all the root boundaries in
Expand All @@ -3101,6 +3112,12 @@ function abortTask(task: Task, request: Request, error: mixed): void {
errorDigest,
);
}
request.pendingRootTasks--;
if (request.pendingRootTasks === 0) {
request.onShellError = noop;
const onShellReady = request.onShellReady;
onShellReady();
}
}
}
} else {
Expand Down Expand Up @@ -3137,12 +3154,12 @@ function abortTask(task: Task, request: Request, error: mixed): void {
abortTask(fallbackTask, request, error),
);
boundary.fallbackAbortableTasks.clear();
}

request.allPendingTasks--;
if (request.allPendingTasks === 0) {
const onAllReady = request.onAllReady;
onAllReady();
}
request.allPendingTasks--;
if (request.allPendingTasks === 0) {
const onAllReady = request.onAllReady;
onAllReady();
}
}

Expand Down Expand Up @@ -3365,6 +3382,7 @@ function retryRenderTask(
logPostpone(request, postponeInstance.message);
trackPostpone(request, trackedPostpones, task, segment);
finishedTask(request, task.blockedBoundary, segment);
lastBoundaryErrorComponentStackDev = null;
return;
}
}
Expand Down Expand Up @@ -3452,6 +3470,12 @@ function retryReplayTask(request: Request, task: ReplayTask): void {
task.replay.nodes,
task.replay.slots,
);
request.pendingRootTasks--;
if (request.pendingRootTasks === 0) {
request.onShellError = noop;
const onShellReady = request.onShellReady;
onShellReady();
}
request.allPendingTasks--;
if (request.allPendingTasks === 0) {
const onAllReady = request.onAllReady;
Expand Down
4 changes: 2 additions & 2 deletions scripts/error-codes/codes.json
Original file line number Diff line number Diff line change
Expand Up @@ -474,8 +474,8 @@
"486": "It should not be possible to postpone at the root. This is a bug in React.",
"487": "We should not have any resumable nodes in the shell. This is a bug in React.",
"488": "Couldn't find all resumable slots by key/index during replaying. The tree doesn't match so React will fallback to client rendering.",
"489": "Expected to see a component of type \"%s\" in this slot. The tree doesn't match so React will fallback to client rendering.",
"490": "Expected to see a Suspense boundary in this slot. The tree doesn't match so React will fallback to client rendering.",
"489": "Expected the resume to render <%s> in this slot but instead it rendered <%s>. The tree doesn't match so React will fallback to client rendering.",
"490": "Expected the resume to render <%s> in this slot but instead it rendered <%s>. The tree doesn't match so React will fallback to client rendering.",
"491": "It should not be possible to postpone both at the root of an element as well as a slot below. This is a bug in React.",
"492": "The \"react\" package in this environment is not configured correctly. The \"react-server\" condition must be enabled in any environment that runs React Server Components.",
"493": "To taint a value, a lifetime must be defined by passing an object that holds the value.",
Expand Down