Skip to content

Commit

Permalink
Warn if boundary with only componentDidCatch swallows error
Browse files Browse the repository at this point in the history
  • Loading branch information
Brian Vaughn committed Sep 27, 2018
1 parent f6fb111 commit 3df17ba
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -2084,4 +2084,70 @@ describe('ReactErrorBoundaries', () => {
// Error should be the first thrown
expect(caughtError.message).toBe('child sad');
});

it('should warn if an error boundary with only componentDidCatch does not update state', () => {
class InvalidErrorBoundary extends React.Component {
componentDidCatch(error, info) {
// This component does not define getDerivedStateFromError().
// It also doesn't call setState().
// So it would swallow errors (which is probably unintentional).
}
render() {
return this.props.children;
}
}

const Throws = () => {
throw new Error('expected');
};

const container = document.createElement('div');
expect(() => {
ReactDOM.render(
<InvalidErrorBoundary>
<Throws />
</InvalidErrorBoundary>,
container,
);
}).toWarnDev(
'InvalidErrorBoundary: Error boundaries should implement getDerivedStateFromError(). ' +
'In that method, return a state update to display an error message or fallback UI, ' +
'or rethrow the error to let parent components handle it',
{withoutStack: true},
);
expect(container.textContent).toBe('');
});

it('should call both componentDidCatch and getDerivedStateFromError if both exist on a component', () => {
let componentDidCatchError, getDerivedStateFromErrorError;
class ErrorBoundaryWithBothMethods extends React.Component {
state = {error: null};
static getDerivedStateFromError(error) {
getDerivedStateFromErrorError = error;
return {error};
}
componentDidCatch(error, info) {
componentDidCatchError = error;
}
render() {
return this.state.error ? 'ErrorBoundary' : this.props.children;
}
}

const thrownError = new Error('expected');
const Throws = () => {
throw thrownError;
};

const container = document.createElement('div');
ReactDOM.render(
<ErrorBoundaryWithBothMethods>
<Throws />
</ErrorBoundaryWithBothMethods>,
container,
);
expect(container.textContent).toBe('ErrorBoundary');
expect(componentDidCatchError).toBe(thrownError);
expect(getDerivedStateFromErrorError).toBe(thrownError);
});
});
19 changes: 18 additions & 1 deletion packages/react-reconciler/src/ReactFiberUnwindWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import type {CapturedValue} from './ReactCapturedValue';
import type {Update} from './ReactUpdateQueue';
import type {Thenable} from './ReactFiberScheduler';

import getComponentName from 'shared/getComponentName';
import warningWithoutStack from 'shared/warningWithoutStack';
import {
IndeterminateComponent,
FunctionalComponent,
Expand Down Expand Up @@ -111,7 +113,7 @@ function createClassErrorUpdate(
const inst = fiber.stateNode;
if (inst !== null && typeof inst.componentDidCatch === 'function') {
update.callback = function callback() {
if (getDerivedStateFromError !== 'function') {
if (typeof getDerivedStateFromError === 'function') {
// To preserve the preexisting retry behavior of error boundaries,
// we keep track of which ones already failed during this batch.
// This gets reset before we yield back to the browser.
Expand All @@ -125,6 +127,21 @@ function createClassErrorUpdate(
this.componentDidCatch(error, {
componentStack: stack !== null ? stack : '',
});
if (__DEV__) {
if (typeof getDerivedStateFromError !== 'function') {
// If componentDidCatch is the only error boundary method defined,
// then it needs to call setState to recover from errors.
// If no state update is scheduled then the boundary will swallow the error.
const updateQueue = fiber.updateQueue;
warningWithoutStack(
updateQueue !== null && updateQueue.firstUpdate !== null,
'%s: Error boundaries should implement getDerivedStateFromError(). ' +
'In that method, return a state update to display an error message or fallback UI, ' +
'or rethrow the error to let parent components handle it.',
getComponentName(fiber.type) || 'Unknown',
);
}
}
};
}
return update;
Expand Down

0 comments on commit 3df17ba

Please sign in to comment.