Skip to content

Commit

Permalink
stash the component stack on the thrown value and reuse (#25790)
Browse files Browse the repository at this point in the history
ErrorBoundaries are currently not fully composable. The reason is if you
decide your boundary cannot handle a particular error and rethrow it to
higher boundary the React runtime does not understand that this throw is
a forward and it recreates the component stack from the Boundary
position. This loses fidelity and is especially bad if the boundary is
limited it what it handles and high up in the component tree.

This implementation uses a WeakMap to store component stacks for values
that are objects. If an error is rethrown from an ErrorBoundary the
stack will be pulled from the map if it exists. This doesn't work for
thrown primitives but this is uncommon and stashing the stack on the
primitive also wouldn't work
  • Loading branch information
gnoff authored Feb 16, 2024
1 parent 2ba1b78 commit a9cc325
Show file tree
Hide file tree
Showing 3 changed files with 129 additions and 8 deletions.
28 changes: 23 additions & 5 deletions packages/react-reconciler/src/ReactCapturedValue.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@ import type {Fiber} from './ReactInternalTypes';

import {getStackByFiberInDevAndProd} from './ReactFiberComponentStack';

const CapturedStacks: WeakMap<any, string> = new WeakMap();

This comment has been minimized.

Copy link
@sebmarkbage

sebmarkbage Feb 16, 2024

Collaborator

const PossiblyWeakMap = typeof WeakMap === 'function' ? WeakMap : Map;

We use this defensive pattern elsewhere since WeakMap took a while to be widely supported and it would error early. We can probably start removing that defensive pattern though.


export type CapturedValue<T> = {
value: T,
+value: T,
source: Fiber | null,
stack: string | null,
digest: string | null,
Expand All @@ -24,19 +26,35 @@ export function createCapturedValueAtFiber<T>(
): CapturedValue<T> {
// If the value is an error, call this function immediately after it is thrown
// so the stack is accurate.
let stack;
if (typeof value === 'object' && value !== null) {
const capturedStack = CapturedStacks.get(value);
if (typeof capturedStack === 'string') {
stack = capturedStack;
} else {
stack = getStackByFiberInDevAndProd(source);
CapturedStacks.set(value, stack);
}
} else {
stack = getStackByFiberInDevAndProd(source);
}

return {
value,
source,
stack: getStackByFiberInDevAndProd(source),
stack,
digest: null,
};
}

export function createCapturedValue<T>(
value: T,
export function createCapturedValueFromError(
value: Error,
digest: ?string,
stack: ?string,
): CapturedValue<T> {
): CapturedValue<Error> {
if (typeof stack === 'string') {
CapturedStacks.set(value, stack);
}
return {
value,
source: null,
Expand Down
6 changes: 3 additions & 3 deletions packages/react-reconciler/src/ReactFiberBeginWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ import {
import {enqueueConcurrentRenderForLane} from './ReactFiberConcurrentUpdates';
import {pushCacheProvider, CacheContext} from './ReactFiberCacheComponent';
import {
createCapturedValue,
createCapturedValueFromError,
createCapturedValueAtFiber,
type CapturedValue,
} from './ReactCapturedValue';
Expand Down Expand Up @@ -2804,7 +2804,7 @@ function updateDehydratedSuspenseComponent(
);
}
(error: any).digest = digest;
capturedValue = createCapturedValue<mixed>(error, digest, stack);
capturedValue = createCapturedValueFromError(error, digest, stack);
}
return retrySuspenseComponentWithoutHydrating(
current,
Expand Down Expand Up @@ -2941,7 +2941,7 @@ function updateDehydratedSuspenseComponent(
pushPrimaryTreeSuspenseHandler(workInProgress);

workInProgress.flags &= ~ForceClientRender;
const capturedValue = createCapturedValue<mixed>(
const capturedValue = createCapturedValueFromError(
new Error(
'There was an error while hydrating this Suspense boundary. ' +
'Switched to client rendering.',
Expand Down
103 changes: 103 additions & 0 deletions packages/react-reconciler/src/__tests__/ReactErrorStacks-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
/**
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @emails react-core
* @jest-environment node
*/
'use strict';

let React;
let ReactNoop;
let waitForAll;

describe('ReactFragment', () => {
beforeEach(function () {
jest.resetModules();

React = require('react');
ReactNoop = require('react-noop-renderer');
const InternalTestUtils = require('internal-test-utils');
waitForAll = InternalTestUtils.waitForAll;
});

function componentStack(components) {
return components
.map(component => `\n in ${component} (at **)`)
.join('');
}

function normalizeCodeLocInfo(str) {
return (
str &&
str.replace(/\n +(?:at|in) ([\S]+)[^\n]*/g, function (m, name) {
return '\n in ' + name + ' (at **)';
})
);
}

it('retains component stacks when rethrowing an error', async () => {
function Foo() {
return (
<RethrowingBoundary>
<Bar />
</RethrowingBoundary>
);
}
function Bar() {
return <SomethingThatErrors />;
}
function SomethingThatErrors() {
throw new Error('uh oh');
}

class RethrowingBoundary extends React.Component {
static getDerivedStateFromError(error) {
throw error;
}

render() {
return this.props.children;
}
}

const errors = [];
class CatchingBoundary extends React.Component {
constructor() {
super();
this.state = {};
}
static getDerivedStateFromError(error) {
return {errored: true};
}
componentDidCatch(err, errInfo) {
errors.push(err.message, normalizeCodeLocInfo(errInfo.componentStack));
}
render() {
if (this.state.errored) {
return null;
}
return this.props.children;
}
}

ReactNoop.render(
<CatchingBoundary>
<Foo />
</CatchingBoundary>,
);
await waitForAll([]);
expect(errors).toEqual([
'uh oh',
componentStack([
'SomethingThatErrors',
'Bar',
'RethrowingBoundary',
'Foo',
'CatchingBoundary',
]),
]);
});
});

0 comments on commit a9cc325

Please sign in to comment.