diff --git a/fixtures/dom/public/index.html b/fixtures/dom/public/index.html index 813f416c0e..f6bb303c9a 100644 --- a/fixtures/dom/public/index.html +++ b/fixtures/dom/public/index.html @@ -15,6 +15,7 @@ --> React App +
diff --git a/fixtures/dom/src/components/fixtures/error-handling/index.js b/fixtures/dom/src/components/fixtures/error-handling/index.js index f5c68a8085..8a9eb61c24 100644 --- a/fixtures/dom/src/components/fixtures/error-handling/index.js +++ b/fixtures/dom/src/components/fixtures/error-handling/index.js @@ -2,11 +2,15 @@ import FixtureSet from '../../FixtureSet'; import TestCase from '../../TestCase'; const React = window.React; +const ReactDOM = window.ReactDOM; function BadRender(props) { - throw props.error; + props.doThrow(); } class ErrorBoundary extends React.Component { + static defaultProps = { + buttonText: 'Trigger error', + }; state = { shouldThrow: false, didThrow: false, @@ -23,15 +27,15 @@ class ErrorBoundary extends React.Component { render() { if (this.state.didThrow) { if (this.state.error) { - return `Captured an error: ${this.state.error.message}`; + return

Captured an error: {this.state.error.message}

; } else { - return `Captured an error: ${this.state.error}`; + return

Captured an error: {'' + this.state.error}

; } } if (this.state.shouldThrow) { - return ; + return ; } - return ; + return ; } } class Example extends React.Component { @@ -42,13 +46,44 @@ class Example extends React.Component { render() { return (
- +
); } } +class TriggerErrorAndCatch extends React.Component { + container = document.createElement('div'); + + triggerErrorAndCatch = () => { + try { + ReactDOM.flushSync(() => { + ReactDOM.render( + { + throw new Error('Caught error'); + }} + />, + this.container + ); + }); + } catch (e) {} + }; + + render() { + return ( + + ); + } +} + export default class ErrorHandlingTestCases extends React.Component { render() { return ( @@ -69,7 +104,11 @@ export default class ErrorHandlingTestCases extends React.Component { should be replaced with "Captured an error: Oops!" Clicking reset should reset the test case. - + { + throw new Error('Oops!'); + }} + /> @@ -80,7 +119,44 @@ export default class ErrorHandlingTestCases extends React.Component { The "Trigger error" button should be replaced with "Captured an error: null". Clicking reset should reset the test case. - + { + throw null; // eslint-disable-line no-throw-literal + }} + /> + + + +
  • Click the "Trigger cross-origin error" button
  • +
  • Click the reset button
  • +
    + + The "Trigger error" button should be replaced with "Captured an + error: A cross-origin error was thrown [...]". The actual error message should + be logged to the console: "Uncaught Error: Expected true to + be false". + + { + // The `expect` module is loaded via unpkg, so that this assertion + // triggers a cross-origin error + window.expect(true).toBe(false); + }} + /> +
    + + +
  • Click the "Trigger render error and catch" button
  • +
    + + Open the console. "Uncaught Error: Caught error" should have been logged by the browser. + +
    ); diff --git a/src/renderers/shared/fiber/ReactFiberErrorLogger.js b/src/renderers/shared/fiber/ReactFiberErrorLogger.js index fef28fe232..74ec53e946 100644 --- a/src/renderers/shared/fiber/ReactFiberErrorLogger.js +++ b/src/renderers/shared/fiber/ReactFiberErrorLogger.js @@ -30,28 +30,6 @@ function logCapturedError(capturedError: CapturedError): void { } const error = (capturedError.error: any); - - // Duck-typing - let message; - let name; - let stack; - - if ( - error !== null && - typeof error.message === 'string' && - typeof error.name === 'string' && - typeof error.stack === 'string' - ) { - message = error.message; - name = error.name; - stack = error.stack; - } else { - // A non-error was thrown. - message = '' + error; - name = 'Error'; - stack = ''; - } - if (__DEV__) { const { componentName, @@ -61,25 +39,9 @@ function logCapturedError(capturedError: CapturedError): void { willRetry, } = capturedError; - const errorSummary = message ? `${name}: ${message}` : name; - const componentNameMessage = componentName - ? `React caught an error thrown by ${componentName}.` - : 'React caught an error thrown by one of your components.'; - - // Error stack varies by browser, eg: - // Chrome prepends the Error name and type. - // Firefox, Safari, and IE don't indent the stack lines. - // Format it in a consistent way for error logging. - let formattedCallStack = stack.slice(0, errorSummary.length) === - errorSummary - ? stack.slice(errorSummary.length) - : stack; - formattedCallStack = formattedCallStack - .trim() - .split('\n') - .map(line => `\n ${line.trim()}`) - .join(); + ? `The above error occurred in the <${componentName}> component:` + : 'The above error occurred in one of your React components:'; let errorBoundaryMessage; // errorBoundaryFound check is sufficient; errorBoundaryName check is to satisfy Flow. @@ -90,25 +52,28 @@ function logCapturedError(capturedError: CapturedError): void { `using the error boundary you provided, ${errorBoundaryName}.`; } else { errorBoundaryMessage = - `This error was initially handled by the error boundary ${errorBoundaryName}. ` + + `This error was initially handled by the error boundary ${errorBoundaryName}.\n` + `Recreating the tree from scratch failed so React will unmount the tree.`; } } else { errorBoundaryMessage = - 'Consider adding an error boundary to your tree to customize error handling behavior. ' + - 'See https://fb.me/react-error-boundaries for more information.'; + 'Consider adding an error boundary to your tree to customize error handling behavior.\n' + + 'You can learn more about error boundaries at https://fb.me/react-error-boundaries.'; } + const combinedMessage = + `${componentNameMessage}${componentStack}\n\n` + + `${errorBoundaryMessage}`; - console.error( - `${componentNameMessage} You should fix this error in your code. ${errorBoundaryMessage}\n\n` + - `${errorSummary}\n\n` + - `The error is located at: ${componentStack}\n\n` + - `The error was thrown at: ${formattedCallStack}`, - ); + // In development, we provide our own message with just the component stack. + // We don't include the original error message and JS stack because the browser + // has already printed it. Even if the application swallows the error, it is still + // displayed by the browser thanks to the DEV-only fake event trick in ReactErrorUtils. + console.error(combinedMessage); } else { - console.error( - `React caught an error thrown by one of your components.\n\n${error.stack}`, - ); + // In production, we print the error directly. + // This will include the message, the JS stack, and anything the browser wants to show. + // We pass the error object instead of custom message so that the browser displays the error natively. + console.error(error); } } diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index fc1f56a4a8..9a5d4abe23 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -1188,6 +1188,7 @@ module.exports = function( if (capturedErrors === null) { capturedErrors = new Map(); } + capturedErrors.set(boundary, { componentName, componentStack, diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js index f440ea5746..83b1ad07b1 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js @@ -902,18 +902,15 @@ describe('ReactIncrementalErrorHandling', () => { expect(console.error.calls.count()).toBe(1); const errorMessage = console.error.calls.argsFor(0)[0]; - expect(errorMessage).toContain( - 'React caught an error thrown by ErrorThrowingComponent. ' + - 'You should fix this error in your code. ' + - 'Consider adding an error boundary to your tree to customize error handling behavior.', - ); - expect(errorMessage).toContain('Error: componentWillMount error'); expect(normalizeCodeLocInfo(errorMessage)).toContain( - 'The error is located at: \n' + + 'The above error occurred in the component:\n' + ' in ErrorThrowingComponent (at **)\n' + ' in span (at **)\n' + ' in div (at **)', ); + expect(errorMessage).toContain( + 'Consider adding an error boundary to your tree to customize error handling behavior.', + ); }); it('should log errors that occur during the commit phase', () => { @@ -936,18 +933,15 @@ describe('ReactIncrementalErrorHandling', () => { expect(console.error.calls.count()).toBe(1); const errorMessage = console.error.calls.argsFor(0)[0]; - expect(errorMessage).toContain( - 'React caught an error thrown by ErrorThrowingComponent. ' + - 'You should fix this error in your code. ' + - 'Consider adding an error boundary to your tree to customize error handling behavior.', - ); - expect(errorMessage).toContain('Error: componentDidMount error'); expect(normalizeCodeLocInfo(errorMessage)).toContain( - 'The error is located at: \n' + + 'The above error occurred in the component:\n' + ' in ErrorThrowingComponent (at **)\n' + ' in span (at **)\n' + ' in div (at **)', ); + expect(errorMessage).toContain( + 'Consider adding an error boundary to your tree to customize error handling behavior.', + ); }); it('should ignore errors thrown in log method to prevent cycle', () => { @@ -1026,15 +1020,17 @@ describe('ReactIncrementalErrorHandling', () => { expect(handleErrorCalls.length).toBe(1); expect(console.error.calls.count()).toBe(2); expect(console.error.calls.argsFor(0)[0]).toContain( - 'React caught an error thrown by ErrorThrowingComponent. ' + - 'You should fix this error in your code. ' + - 'React will try to recreate this component tree from scratch ' + + 'The above error occurred in the component:', + ); + expect(console.error.calls.argsFor(0)[0]).toContain( + 'React will try to recreate this component tree from scratch ' + 'using the error boundary you provided, ErrorBoundaryComponent.', ); expect(console.error.calls.argsFor(1)[0]).toContain( - 'React caught an error thrown by ErrorThrowingComponent. ' + - 'You should fix this error in your code. ' + - 'This error was initially handled by the error boundary ErrorBoundaryComponent. ' + + 'The above error occurred in the component:', + ); + expect(console.error.calls.argsFor(1)[0]).toContain( + 'This error was initially handled by the error boundary ErrorBoundaryComponent.\n' + 'Recreating the tree from scratch failed so React will unmount the tree.', ); }); diff --git a/src/renderers/shared/utils/ReactErrorUtils.js b/src/renderers/shared/utils/ReactErrorUtils.js index 8373cf1f5f..eef7aeb6a6 100644 --- a/src/renderers/shared/utils/ReactErrorUtils.js +++ b/src/renderers/shared/utils/ReactErrorUtils.js @@ -208,10 +208,14 @@ if (__DEV__) { let error; // Use this to track whether the error event is ever called. let didSetError = false; + let isCrossOriginError = false; function onError(event) { error = event.error; didSetError = true; + if (error === null && event.colno === 0 && event.lineno === 0) { + isCrossOriginError = true; + } } // Create a fake event type. @@ -240,6 +244,18 @@ if (__DEV__) { 'or switching to a modern browser. If you suspect that this is ' + 'actually an issue with React, please file an issue.', ); + } else if (isCrossOriginError) { + error = new Error( + "A cross-origin error was thrown. React doesn't have access to " + + 'the actual error because it catches errors using a global ' + + 'error handler, in order to preserve the "Pause on exceptions" ' + + 'behavior of the DevTools. This is only an issue in DEV-mode; ' + + 'in production, React uses a normal try-catch statement.\n\n' + + 'If you are using React from a CDN, ensure that the