Fix error logging in getDerivedStateFromProps (#15797)

* Fix error logging in getDerivedStateFromProps

* Update tests, don't log for both error boundary methods

* Re-add change lost in rebase
This commit is contained in:
Ricky 2019-06-25 18:02:27 +01:00 committed by Dan Abramov
parent 6088a201e1
commit 20da1dae4b
4 changed files with 130 additions and 1 deletions

View File

@ -644,6 +644,39 @@ describe('ReactErrorBoundaries', () => {
expect(container3.firstChild).toBe(null);
});
it('logs a single error when using error boundary', () => {
const container = document.createElement('div');
expect(() =>
ReactDOM.render(
<ErrorBoundary>
<BrokenRender />
</ErrorBoundary>,
container,
),
).toWarnDev('The above error occurred in the <BrokenRender> component:', {
logAllErrors: true,
});
expect(container.firstChild.textContent).toBe('Caught an error: Hello.');
expect(log).toEqual([
'ErrorBoundary constructor',
'ErrorBoundary componentWillMount',
'ErrorBoundary render success',
'BrokenRender constructor',
'BrokenRender componentWillMount',
'BrokenRender render [!]',
// Catch and render an error message
'ErrorBoundary static getDerivedStateFromError',
'ErrorBoundary componentWillMount',
'ErrorBoundary render error',
'ErrorBoundary componentDidMount',
]);
log.length = 0;
ReactDOM.unmountComponentAtNode(container);
expect(log).toEqual(['ErrorBoundary componentWillUnmount']);
});
it('renders an error state if child throws in render', () => {
const container = document.createElement('div');
ReactDOM.render(

View File

@ -30,6 +30,7 @@ describe('ReactLegacyErrorBoundaries', () => {
let BrokenComponentDidMountErrorBoundary;
let BrokenRender;
let ErrorBoundary;
let BothErrorBoundaries;
let ErrorMessage;
let NoopErrorBoundary;
let RetryErrorBoundary;
@ -486,6 +487,57 @@ describe('ReactLegacyErrorBoundaries', () => {
},
};
BothErrorBoundaries = class extends React.Component {
constructor(props) {
super(props);
this.state = {error: null};
log.push('BothErrorBoundaries constructor');
}
static getDerivedStateFromError(error) {
log.push('BothErrorBoundaries static getDerivedStateFromError');
return {error};
}
render() {
if (this.state.error) {
log.push('BothErrorBoundaries render error');
return <div>Caught an error: {this.state.error.message}.</div>;
}
log.push('BothErrorBoundaries render success');
return <div>{this.props.children}</div>;
}
componentDidCatch(error) {
log.push('BothErrorBoundaries componentDidCatch');
this.setState({error});
}
UNSAFE_componentWillMount() {
log.push('BothErrorBoundaries componentWillMount');
}
componentDidMount() {
log.push('BothErrorBoundaries componentDidMount');
}
UNSAFE_componentWillReceiveProps() {
log.push('BothErrorBoundaries componentWillReceiveProps');
}
UNSAFE_componentWillUpdate() {
log.push('BothErrorBoundaries componentWillUpdate');
}
componentDidUpdate() {
log.push('BothErrorBoundaries componentDidUpdate');
}
componentWillUnmount() {
log.push('BothErrorBoundaries componentWillUnmount');
}
};
RetryErrorBoundary = class extends React.Component {
constructor(props) {
super(props);
@ -614,6 +666,45 @@ describe('ReactLegacyErrorBoundaries', () => {
expect(container3.firstChild).toBe(null);
});
it('logs a single error using both error boundaries', () => {
const container = document.createElement('div');
expect(() =>
ReactDOM.render(
<BothErrorBoundaries>
<BrokenRender />
</BothErrorBoundaries>,
container,
),
).toWarnDev('The above error occurred in the <BrokenRender> component', {
logAllErrors: true,
});
expect(container.firstChild.textContent).toBe('Caught an error: Hello.');
expect(log).toEqual([
'BothErrorBoundaries constructor',
'BothErrorBoundaries componentWillMount',
'BothErrorBoundaries render success',
'BrokenRender constructor',
'BrokenRender componentWillMount',
'BrokenRender render [!]',
// Both getDerivedStateFromError and componentDidCatch should be called
'BothErrorBoundaries static getDerivedStateFromError',
'BothErrorBoundaries componentWillMount',
'BothErrorBoundaries render error',
// Fiber mounts with null children before capturing error
'BothErrorBoundaries componentDidMount',
// Catch and render an error message
'BothErrorBoundaries componentDidCatch',
'BothErrorBoundaries componentWillUpdate',
'BothErrorBoundaries render error',
'BothErrorBoundaries componentDidUpdate',
]);
log.length = 0;
ReactDOM.unmountComponentAtNode(container);
expect(log).toEqual(['BothErrorBoundaries componentWillUnmount']);
});
it('renders an error state if child throws in render', () => {
const container = document.createElement('div');
ReactDOM.render(

View File

@ -102,6 +102,7 @@ function createClassErrorUpdate(
if (typeof getDerivedStateFromError === 'function') {
const error = errorInfo.value;
update.payload = () => {
logError(fiber, errorInfo);
return getDerivedStateFromError(error);
};
}
@ -119,10 +120,12 @@ function createClassErrorUpdate(
// TODO: Warn in strict mode if getDerivedStateFromError is
// not defined.
markLegacyErrorBoundaryAsFailed(this);
// Only log here if componentDidCatch is the only error boundary method defined
logError(fiber, errorInfo);
}
const error = errorInfo.value;
const stack = errorInfo.stack;
logError(fiber, errorInfo);
this.componentDidCatch(error, {
componentStack: stack !== null ? stack : '',
});

View File

@ -38,6 +38,7 @@ const createMatcherFor = consoleMethod =>
}
const withoutStack = options.withoutStack;
const logAllErrors = options.logAllErrors;
const warningsWithoutComponentStack = [];
const warningsWithComponentStack = [];
const unexpectedWarnings = [];
@ -58,6 +59,7 @@ const createMatcherFor = consoleMethod =>
// Ignore uncaught errors reported by jsdom
// and React addendums because they're too noisy.
if (
!logAllErrors &&
consoleMethod === 'error' &&
shouldIgnoreConsoleError(format, args)
) {