Skip to content

Commit 4ccf58a

Browse files
authored
Fix context stack misalignment caused by error replay (#12508)
* Add regression tests for error boundary replay bugs * Ensure the context stack is aligned if renderer throws * Always throw when replaying a failed unit of work Replaying a failed unit of work should always throw, because the render phase is meant to be idempotent, If it doesn't throw, rethrow the original error, so React's internal stack is not misaligned. * Reset originalReplayError after replaying * Typo fix
1 parent 7a27ebd commit 4ccf58a

File tree

3 files changed

+94
-9
lines changed

3 files changed

+94
-9
lines changed

packages/react-reconciler/src/ReactFiberHostContext.js

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,12 +60,19 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
6060
// Push current root instance onto the stack;
6161
// This allows us to reset root when portals are popped.
6262
push(rootInstanceStackCursor, nextRootInstance, fiber);
63-
64-
const nextRootContext = getRootHostContext(nextRootInstance);
65-
6663
// Track the context and the Fiber that provided it.
6764
// This enables us to pop only Fibers that provide unique contexts.
6865
push(contextFiberStackCursor, fiber, fiber);
66+
67+
// Finally, we need to push the host context to the stack.
68+
// However, we can't just call getRootHostContext() and push it because
69+
// we'd have a different number of entries on the stack depending on
70+
// whether getRootHostContext() throws somewhere in renderer code or not.
71+
// So we push an empty value first. This lets us safely unwind on errors.
72+
push(contextStackCursor, NO_CONTEXT, fiber);
73+
const nextRootContext = getRootHostContext(nextRootInstance);
74+
// Now that we know this function doesn't throw, replace it.
75+
pop(contextStackCursor, fiber);
6976
push(contextStackCursor, nextRootContext, fiber);
7077
}
7178

packages/react-reconciler/src/ReactFiberScheduler.js

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -263,10 +263,18 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
263263

264264
let stashedWorkInProgressProperties;
265265
let replayUnitOfWork;
266+
let isReplayingFailedUnitOfWork;
267+
let originalReplayError;
266268
if (__DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback) {
267269
stashedWorkInProgressProperties = null;
268-
replayUnitOfWork = (failedUnitOfWork: Fiber, isAsync: boolean) => {
269-
// Retore the original state of the work-in-progress
270+
isReplayingFailedUnitOfWork = false;
271+
originalReplayError = null;
272+
replayUnitOfWork = (
273+
failedUnitOfWork: Fiber,
274+
error: mixed,
275+
isAsync: boolean,
276+
) => {
277+
// Restore the original state of the work-in-progress
270278
assignFiberPropertiesInDEV(
271279
failedUnitOfWork,
272280
stashedWorkInProgressProperties,
@@ -290,12 +298,17 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
290298
break;
291299
}
292300
// Replay the begin phase.
301+
isReplayingFailedUnitOfWork = true;
302+
originalReplayError = error;
293303
invokeGuardedCallback(null, workLoop, null, isAsync);
304+
isReplayingFailedUnitOfWork = false;
305+
originalReplayError = null;
294306
if (hasCaughtError()) {
295307
clearCaughtError();
296308
} else {
297-
// This should be unreachable because the render phase is
298-
// idempotent
309+
// If the begin phase did not fail the second time, set this pointer
310+
// back to the original value.
311+
nextUnitOfWork = failedUnitOfWork;
299312
}
300313
};
301314
}
@@ -855,9 +868,15 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
855868
);
856869
}
857870
let next = beginWork(current, workInProgress, nextRenderExpirationTime);
858-
859871
if (__DEV__) {
860872
ReactDebugCurrentFiber.resetCurrentFiber();
873+
if (isReplayingFailedUnitOfWork) {
874+
// Currently replaying a failed unit of work. This should be unreachable,
875+
// because the render phase is meant to be idempotent, and it should
876+
// have thrown again. Since it didn't, rethrow the original error, so
877+
// React's internal stack is not misaligned.
878+
throw originalReplayError;
879+
}
861880
}
862881
if (__DEV__ && ReactFiberInstrumentation.debugTool) {
863882
ReactFiberInstrumentation.debugTool.onBeginWork(workInProgress);
@@ -935,7 +954,7 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
935954

936955
if (__DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback) {
937956
const failedUnitOfWork = nextUnitOfWork;
938-
replayUnitOfWork(failedUnitOfWork, isAsync);
957+
replayUnitOfWork(failedUnitOfWork, thrownValue, isAsync);
939958
}
940959

941960
const sourceFiber: Fiber = nextUnitOfWork;
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
/**
2+
* Copyright (c) 2013-present, Facebook, Inc.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*
7+
* @emails react-core
8+
* @jest-environment node
9+
*/
10+
11+
'use strict';
12+
13+
let React;
14+
let ReactNoop;
15+
16+
describe('ReactIncrementalErrorReplay', () => {
17+
beforeEach(() => {
18+
jest.resetModules();
19+
React = require('react');
20+
ReactNoop = require('react-noop-renderer');
21+
});
22+
23+
function div(...children) {
24+
children = children.map(c => (typeof c === 'string' ? {text: c} : c));
25+
return {type: 'div', children, prop: undefined};
26+
}
27+
28+
function span(prop) {
29+
return {type: 'span', children: [], prop};
30+
}
31+
32+
it('should fail gracefully on error in the host environment', () => {
33+
ReactNoop.simulateErrorInHostConfig(() => {
34+
ReactNoop.render(<span />);
35+
expect(() => ReactNoop.flush()).toThrow('Error in host config.');
36+
});
37+
});
38+
39+
it('should fail gracefully on error that does not reproduce on replay', () => {
40+
let didInit = false;
41+
42+
function badLazyInit() {
43+
const needsInit = !didInit;
44+
didInit = true;
45+
if (needsInit) {
46+
throw new Error('Hi');
47+
}
48+
}
49+
50+
class App extends React.Component {
51+
render() {
52+
badLazyInit();
53+
return <div />;
54+
}
55+
}
56+
ReactNoop.render(<App />);
57+
expect(() => ReactNoop.flush()).toThrow('Hi');
58+
});
59+
});

0 commit comments

Comments
 (0)