From 01e247089593c64130867e4fbfeeb06746fbfd4b Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Tue, 1 Aug 2017 11:09:43 +0530 Subject: [PATCH 01/15] Add DOM fixture for cross-origin errors --- fixtures/dom/public/index.html | 1 + .../fixtures/error-handling/index.js | 49 ++++++++++++++++--- 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/fixtures/dom/public/index.html b/fixtures/dom/public/index.html index ed1922517a1b9..edbd5fd23f987 100644 --- a/fixtures/dom/public/index.html +++ b/fixtures/dom/public/index.html @@ -16,6 +16,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 4f8208566faf0..c918ba0cb2e5a 100644 --- a/fixtures/dom/src/components/fixtures/error-handling/index.js +++ b/fixtures/dom/src/components/fixtures/error-handling/index.js @@ -4,9 +4,12 @@ import FixtureSet from '../../FixtureSet'; import TestCase from '../../TestCase'; function BadRender(props) { - throw props.error; + throw props.throws(); } class ErrorBoundary extends React.Component { + static defaultProps = { + buttonText: 'Trigger error', + }; state = { shouldThrow: false, didThrow: false, @@ -29,9 +32,9 @@ class ErrorBoundary extends React.Component { } } if (this.state.shouldThrow) { - return ; + return ; } - return ; + return ; } } class Example extends React.Component { @@ -42,7 +45,11 @@ class Example extends React.Component { render() { return (
- +
); @@ -69,7 +76,11 @@ export default class ErrorHandlingTestCases extends React.Component { should be replaced with "Captured an error: Oops!" Clicking reset should reset the test case. - + { + new Error('Oops!'); + }} + /> @@ -80,7 +91,33 @@ 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; + }} + /> + + + +
  • Click the "Trigger cross-origin error" button
    • +
  • Click the reset button
    • +     + + The "Trigger error" button should be replaced with "Captured an + error: (TODO: custom error text)". 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); + }} + />     ); From cb6fd8c583e211dcfab34b045cc833d3c70c316e Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Wed, 2 Aug 2017 19:50:39 +0530 Subject: [PATCH 02/15] Use a custom error object in place of cross-origin errors Cross-origin errors aren't accessible by React in DEV mode because we catch errors using a global error handler, in order to preserve the "Pause on exceptions" behavior of the DevTools. When this happens, we should use a custom error object that explains what happened. For uncaught errors, the actual error message is logged to the console by the browser, so React should skip logging the message again. --- .../fixtures/error-handling/index.js | 10 +++--- .../shared/fiber/ReactFiberErrorLogger.js | 16 +++++---- .../shared/fiber/ReactFiberScheduler.js | 36 ++++++++++++++----- src/renderers/shared/utils/ReactErrorUtils.js | 15 ++++++++ 4 files changed, 57 insertions(+), 20 deletions(-) diff --git a/fixtures/dom/src/components/fixtures/error-handling/index.js b/fixtures/dom/src/components/fixtures/error-handling/index.js index c918ba0cb2e5a..b73be12910018 100644 --- a/fixtures/dom/src/components/fixtures/error-handling/index.js +++ b/fixtures/dom/src/components/fixtures/error-handling/index.js @@ -26,9 +26,9 @@ 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) { @@ -45,12 +45,12 @@ class Example extends React.Component { render() { return (
    + -
    ); } @@ -93,7 +93,7 @@ export default class ErrorHandlingTestCases extends React.Component { { - throw null; + throw null; // eslint-disable-line no-throw-literal }} /> @@ -106,7 +106,7 @@ export default class ErrorHandlingTestCases extends React.Component { The "Trigger error" button should be replaced with "Captured an - error: (TODO: custom error text)". The actual error message should + error: A cross-origin error was thrown [...]". The actual error message should be logged to the console: "Uncaught Error: Expected true to be false". diff --git a/src/renderers/shared/fiber/ReactFiberErrorLogger.js b/src/renderers/shared/fiber/ReactFiberErrorLogger.js index fef28fe2326b9..cc105af2e2ff8 100644 --- a/src/renderers/shared/fiber/ReactFiberErrorLogger.js +++ b/src/renderers/shared/fiber/ReactFiberErrorLogger.js @@ -59,6 +59,7 @@ function logCapturedError(capturedError: CapturedError): void { errorBoundaryName, errorBoundaryFound, willRetry, + shouldIgnoreErrorMessage, } = capturedError; const errorSummary = message ? `${name}: ${message}` : name; @@ -99,12 +100,15 @@ function logCapturedError(capturedError: CapturedError): void { 'See https://fb.me/react-error-boundaries for more information.'; } - 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}`, - ); + let combinedMessage = `${componentNameMessage} You should fix this error in your code. ${errorBoundaryMessage}\n\n`; + if (!shouldIgnoreErrorMessage) { + combinedMessage += `${errorSummary}\n\n`; + } + combinedMessage += + `The error is located at: ${componentStack}\n\n` + + `The error was thrown at: ${formattedCallStack}`; + + console.error(combinedMessage); } else { console.error( `React caught an error thrown by one of your components.\n\n${error.stack}`, diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index fc1f56a4a8a18..8876ebcb83324 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -26,6 +26,7 @@ export type CapturedError = { errorBoundaryFound: boolean, errorBoundaryName: string | null, willRetry: boolean, + shouldIgnoreErrorMessage: boolean, }; export type HandleErrorInfo = { @@ -1188,15 +1189,32 @@ module.exports = function( if (capturedErrors === null) { capturedErrors = new Map(); } - capturedErrors.set(boundary, { - componentName, - componentStack, - error, - errorBoundary: errorBoundaryFound ? boundary.stateNode : null, - errorBoundaryFound, - errorBoundaryName, - willRetry, - }); + + if (__DEV__) { + capturedErrors.set(boundary, { + componentName, + componentStack, + error, + errorBoundary: errorBoundaryFound ? boundary.stateNode : null, + errorBoundaryFound, + errorBoundaryName, + willRetry, + shouldIgnoreErrorMessage: error != null && + typeof error.__reactShouldIgnoreErrorMessage === 'boolean' && + error.__reactShouldIgnoreErrorMessage, + }); + } else { + capturedErrors.set(boundary, { + componentName, + componentStack, + error, + errorBoundary: errorBoundaryFound ? boundary.stateNode : null, + errorBoundaryFound, + errorBoundaryName, + willRetry, + shouldIgnoreErrorMessage: false, + }); + } // If we're in the commit phase, defer scheduling an update on the // boundary until after the commit is complete diff --git a/src/renderers/shared/utils/ReactErrorUtils.js b/src/renderers/shared/utils/ReactErrorUtils.js index 8373cf1f5fb0b..093d33100d468 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,17 @@ 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' + + "It's recommended to serve JavaScript files from the same " + + 'origin as your application.', + ); + (error: any).__reactShouldIgnoreErrorMessage = true; } ReactErrorUtils._hasCaughtError = true; ReactErrorUtils._caughtError = error; From 6f6425ba2fb380938fbe15b7d52d6a38cda04330 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Wed, 2 Aug 2017 21:29:54 +0530 Subject: [PATCH 03/15] Add test case that demonstrates errors are logged even if they're caught --- .../fixtures/error-handling/index.js | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/fixtures/dom/src/components/fixtures/error-handling/index.js b/fixtures/dom/src/components/fixtures/error-handling/index.js index b73be12910018..1433deb066fda 100644 --- a/fixtures/dom/src/components/fixtures/error-handling/index.js +++ b/fixtures/dom/src/components/fixtures/error-handling/index.js @@ -1,4 +1,5 @@ const React = window.React; +const ReactDOM = window.ReactDOM; import FixtureSet from '../../FixtureSet'; import TestCase from '../../TestCase'; @@ -56,6 +57,33 @@ class Example extends React.Component { } } +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 ( @@ -119,6 +147,17 @@ export default class ErrorHandlingTestCases extends React.Component { }} /> + + +
  • Click the "Trigger render error and catch" button
    • +     + + Open the console. "Uncaught Error: Caught error" should have been logged by the browser. + + +     ); } From b145a0845544c12cea9e0b080d40870274c4609b Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Wed, 2 Aug 2017 21:52:56 +0530 Subject: [PATCH 04/15] Don't double log error messages in DEV In DEV, the browser always logs errors thrown inside React components, even if the originating update is wrapped in a try-catch, because of the dispatchEvent trick used by invokeGuardedCallback. So the error logger should not log the message again. --- .../shared/fiber/ReactFiberErrorLogger.js | 7 ++-- .../shared/fiber/ReactFiberScheduler.js | 35 +++++-------------- src/renderers/shared/utils/ReactErrorUtils.js | 1 - 3 files changed, 12 insertions(+), 31 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberErrorLogger.js b/src/renderers/shared/fiber/ReactFiberErrorLogger.js index cc105af2e2ff8..505e4e24384ee 100644 --- a/src/renderers/shared/fiber/ReactFiberErrorLogger.js +++ b/src/renderers/shared/fiber/ReactFiberErrorLogger.js @@ -59,14 +59,13 @@ function logCapturedError(capturedError: CapturedError): void { errorBoundaryName, errorBoundaryFound, willRetry, - shouldIgnoreErrorMessage, } = 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.'; + ? `An error was thrown by ${componentName}.` + : 'An error was thrown by one of your components.'; // Error stack varies by browser, eg: // Chrome prepends the Error name and type. @@ -101,7 +100,7 @@ function logCapturedError(capturedError: CapturedError): void { } let combinedMessage = `${componentNameMessage} You should fix this error in your code. ${errorBoundaryMessage}\n\n`; - if (!shouldIgnoreErrorMessage) { + if (!__DEV__) { combinedMessage += `${errorSummary}\n\n`; } combinedMessage += diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 8876ebcb83324..9a5d4abe2362b 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -26,7 +26,6 @@ export type CapturedError = { errorBoundaryFound: boolean, errorBoundaryName: string | null, willRetry: boolean, - shouldIgnoreErrorMessage: boolean, }; export type HandleErrorInfo = { @@ -1190,31 +1189,15 @@ module.exports = function( capturedErrors = new Map(); } - if (__DEV__) { - capturedErrors.set(boundary, { - componentName, - componentStack, - error, - errorBoundary: errorBoundaryFound ? boundary.stateNode : null, - errorBoundaryFound, - errorBoundaryName, - willRetry, - shouldIgnoreErrorMessage: error != null && - typeof error.__reactShouldIgnoreErrorMessage === 'boolean' && - error.__reactShouldIgnoreErrorMessage, - }); - } else { - capturedErrors.set(boundary, { - componentName, - componentStack, - error, - errorBoundary: errorBoundaryFound ? boundary.stateNode : null, - errorBoundaryFound, - errorBoundaryName, - willRetry, - shouldIgnoreErrorMessage: false, - }); - } + capturedErrors.set(boundary, { + componentName, + componentStack, + error, + errorBoundary: errorBoundaryFound ? boundary.stateNode : null, + errorBoundaryFound, + errorBoundaryName, + willRetry, + }); // If we're in the commit phase, defer scheduling an update on the // boundary until after the commit is complete diff --git a/src/renderers/shared/utils/ReactErrorUtils.js b/src/renderers/shared/utils/ReactErrorUtils.js index 093d33100d468..5032a789446a2 100644 --- a/src/renderers/shared/utils/ReactErrorUtils.js +++ b/src/renderers/shared/utils/ReactErrorUtils.js @@ -254,7 +254,6 @@ if (__DEV__) { "It's recommended to serve JavaScript files from the same " + 'origin as your application.', ); - (error: any).__reactShouldIgnoreErrorMessage = true; } ReactErrorUtils._hasCaughtError = true; ReactErrorUtils._caughtError = error; From 26770b2fc0013bc2745b92b6c7ccfe18a85f6a6c Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Thu, 3 Aug 2017 15:47:01 +0100 Subject: [PATCH 05/15] Fix tests --- .../shared/fiber/ReactFiberErrorLogger.js | 2 +- .../ReactIncrementalErrorHandling-test.js | 18 ++++++++++++------ 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberErrorLogger.js b/src/renderers/shared/fiber/ReactFiberErrorLogger.js index 505e4e24384ee..bbb5588a1f1a2 100644 --- a/src/renderers/shared/fiber/ReactFiberErrorLogger.js +++ b/src/renderers/shared/fiber/ReactFiberErrorLogger.js @@ -110,7 +110,7 @@ function logCapturedError(capturedError: CapturedError): void { console.error(combinedMessage); } else { console.error( - `React caught an error thrown by one of your components.\n\n${error.stack}`, + `An error was thrown by one of your components.\n\n${error.stack}`, ); } } diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js index f440ea57464b4..d57026428ac20 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js @@ -903,11 +903,14 @@ 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. ' + + 'An error was 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 was thrown at:\n' + + ' at ErrorThrowingComponent.componentWillMount (**)' + ); expect(normalizeCodeLocInfo(errorMessage)).toContain( 'The error is located at: \n' + ' in ErrorThrowingComponent (at **)\n' + @@ -937,11 +940,14 @@ 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. ' + + 'An error was 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 was thrown at:\n' + + ' at ErrorThrowingComponent.componentDidMount (**)' + ); expect(normalizeCodeLocInfo(errorMessage)).toContain( 'The error is located at: \n' + ' in ErrorThrowingComponent (at **)\n' + @@ -1026,13 +1032,13 @@ 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. ' + + 'An error was thrown by ErrorThrowingComponent. ' + 'You should fix this error in your code. ' + '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. ' + + 'An error was thrown by ErrorThrowingComponent. ' + 'You should fix this error in your code. ' + 'This error was initially handled by the error boundary ErrorBoundaryComponent. ' + 'Recreating the tree from scratch failed so React will unmount the tree.', From fde39dc08de88fafd454318dd30b59a508b6a303 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Thu, 3 Aug 2017 16:22:33 +0100 Subject: [PATCH 06/15] Change how error is printed in DEV and PROD In DEV, we don't want to print the stack trace because the browser already always prints it. We'll just print the component stack now. In PROD, we used to omit the JS error message. However we *do* want to show it because if the application swallows the error, the browser will *not* print it. In DEV it works only because of the fake event trick. So in PROD we will always print the underlying error by logging the error object directly. This will show both the message and the JS stack. --- .../fixtures/error-handling/index.js | 2 +- .../shared/fiber/ReactFiberErrorLogger.js | 39 ++++++------------- .../ReactIncrementalErrorHandling-test.js | 8 ---- src/renderers/shared/utils/ReactErrorUtils.js | 6 ++- 4 files changed, 17 insertions(+), 38 deletions(-) diff --git a/fixtures/dom/src/components/fixtures/error-handling/index.js b/fixtures/dom/src/components/fixtures/error-handling/index.js index cc42ae8514aab..4e735b16809dc 100644 --- a/fixtures/dom/src/components/fixtures/error-handling/index.js +++ b/fixtures/dom/src/components/fixtures/error-handling/index.js @@ -69,7 +69,7 @@ class TriggerErrorAndCatch extends React.Component { throw new Error('Caught error'); }} />, - this.container, + this.container ); }); } catch (e) {} diff --git a/src/renderers/shared/fiber/ReactFiberErrorLogger.js b/src/renderers/shared/fiber/ReactFiberErrorLogger.js index bbb5588a1f1a2..5b8045facc466 100644 --- a/src/renderers/shared/fiber/ReactFiberErrorLogger.js +++ b/src/renderers/shared/fiber/ReactFiberErrorLogger.js @@ -62,25 +62,10 @@ function logCapturedError(capturedError: CapturedError): void { } = capturedError; const errorSummary = message ? `${name}: ${message}` : name; - const componentNameMessage = componentName ? `An error was thrown by ${componentName}.` : 'An error was 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(); - let errorBoundaryMessage; // errorBoundaryFound check is sufficient; errorBoundaryName check is to satisfy Flow. if (errorBoundaryFound && errorBoundaryName) { @@ -98,20 +83,20 @@ function logCapturedError(capturedError: CapturedError): void { 'Consider adding an error boundary to your tree to customize error handling behavior. ' + 'See https://fb.me/react-error-boundaries for more information.'; } - - let combinedMessage = `${componentNameMessage} You should fix this error in your code. ${errorBoundaryMessage}\n\n`; - if (!__DEV__) { - combinedMessage += `${errorSummary}\n\n`; - } - combinedMessage += - `The error is located at: ${componentStack}\n\n` + - `The error was thrown at: ${formattedCallStack}`; - + const combinedMessage = + `${componentNameMessage} You should fix this error in your code. ${errorBoundaryMessage}\n\n` + + `The error is located at: ${componentStack}\n\n`; + + // 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( - `An error was 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/__tests__/ReactIncrementalErrorHandling-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js index d57026428ac20..611ebb71f7364 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js @@ -907,10 +907,6 @@ describe('ReactIncrementalErrorHandling', () => { 'You should fix this error in your code. ' + 'Consider adding an error boundary to your tree to customize error handling behavior.', ); - expect(normalizeCodeLocInfo(errorMessage)).toContain( - 'The error was thrown at:\n' + - ' at ErrorThrowingComponent.componentWillMount (**)' - ); expect(normalizeCodeLocInfo(errorMessage)).toContain( 'The error is located at: \n' + ' in ErrorThrowingComponent (at **)\n' + @@ -944,10 +940,6 @@ describe('ReactIncrementalErrorHandling', () => { 'You should fix this error in your code. ' + 'Consider adding an error boundary to your tree to customize error handling behavior.', ); - expect(normalizeCodeLocInfo(errorMessage)).toContain( - 'The error was thrown at:\n' + - ' at ErrorThrowingComponent.componentDidMount (**)' - ); expect(normalizeCodeLocInfo(errorMessage)).toContain( 'The error is located at: \n' + ' in ErrorThrowingComponent (at **)\n' + diff --git a/src/renderers/shared/utils/ReactErrorUtils.js b/src/renderers/shared/utils/ReactErrorUtils.js index 5032a789446a2..eef7aeb6a641c 100644 --- a/src/renderers/shared/utils/ReactErrorUtils.js +++ b/src/renderers/shared/utils/ReactErrorUtils.js @@ -251,8 +251,10 @@ if (__DEV__) { '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' + - "It's recommended to serve JavaScript files from the same " + - 'origin as your application.', + 'If you are using React from a CDN, ensure that the - From 39df724090a5383abd8277eb38a0d4852d30ca90 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Thu, 3 Aug 2017 18:16:53 +0100 Subject: [PATCH 14/15] Concise wording --- .../shared/fiber/ReactFiberErrorLogger.js | 7 +++---- .../ReactIncrementalErrorHandling-test.js | 18 ++++-------------- 2 files changed, 7 insertions(+), 18 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberErrorLogger.js b/src/renderers/shared/fiber/ReactFiberErrorLogger.js index 97383bd082619..ebe0d06fc4e1d 100644 --- a/src/renderers/shared/fiber/ReactFiberErrorLogger.js +++ b/src/renderers/shared/fiber/ReactFiberErrorLogger.js @@ -59,8 +59,8 @@ function logCapturedError(capturedError: CapturedError): void { } = capturedError; const componentNameMessage = componentName - ? `There was an error in the <${componentName}> component.` - : 'There was an error in one of your React components.'; + ? `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. @@ -80,8 +80,7 @@ function logCapturedError(capturedError: CapturedError): void { 'You can learn more about error boundaries at https://fb.me/react-error-boundaries.'; } const combinedMessage = - `${componentNameMessage}\nYou can find its details in an earlier log.\n\n` + - `The component hierarchy was: ${componentStack}\n\n` + + `${componentNameMessage}${componentStack}\n\n` + `${errorBoundaryMessage}`; // In development, we provide our own message with just the component stack. diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js index bac289b834d2c..83b1ad07b168e 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js @@ -902,12 +902,8 @@ describe('ReactIncrementalErrorHandling', () => { expect(console.error.calls.count()).toBe(1); const errorMessage = console.error.calls.argsFor(0)[0]; - expect(errorMessage).toContain( - 'There was an error in the component.\n' + - 'You can find its details in an earlier log.\n', - ); expect(normalizeCodeLocInfo(errorMessage)).toContain( - 'The component hierarchy was: \n' + + 'The above error occurred in the component:\n' + ' in ErrorThrowingComponent (at **)\n' + ' in span (at **)\n' + ' in div (at **)', @@ -937,12 +933,8 @@ describe('ReactIncrementalErrorHandling', () => { expect(console.error.calls.count()).toBe(1); const errorMessage = console.error.calls.argsFor(0)[0]; - expect(errorMessage).toContain( - 'There was an error in the component.\n' + - 'You can find its details in an earlier log.\n', - ); expect(normalizeCodeLocInfo(errorMessage)).toContain( - 'The component hierarchy was: \n' + + 'The above error occurred in the component:\n' + ' in ErrorThrowingComponent (at **)\n' + ' in span (at **)\n' + ' in div (at **)', @@ -1028,16 +1020,14 @@ describe('ReactIncrementalErrorHandling', () => { expect(handleErrorCalls.length).toBe(1); expect(console.error.calls.count()).toBe(2); expect(console.error.calls.argsFor(0)[0]).toContain( - 'There was an error in the component.\n' + - 'You can find its details in an earlier log.\n', + '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( - 'There was an error in the component.\n' + - 'You can find its details in an earlier log.\n', + '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' + From 2ea6063f308cf1ac79fcbb45112a1fa4cf26f711 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Thu, 3 Aug 2017 18:17:47 +0100 Subject: [PATCH 15/15] Unused variables --- .../shared/fiber/ReactFiberErrorLogger.js | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberErrorLogger.js b/src/renderers/shared/fiber/ReactFiberErrorLogger.js index ebe0d06fc4e1d..74ec53e946ac9 100644 --- a/src/renderers/shared/fiber/ReactFiberErrorLogger.js +++ b/src/renderers/shared/fiber/ReactFiberErrorLogger.js @@ -30,25 +30,6 @@ function logCapturedError(capturedError: CapturedError): void { } const error = (capturedError.error: any); - - // Duck-typing - let message; - let name; - - if ( - error !== null && - typeof error.message === 'string' && - typeof error.name === 'string' && - typeof error.stack === 'string' - ) { - message = error.message; - name = error.name; - } else { - // A non-error was thrown. - message = '' + error; - name = 'Error'; - } - if (__DEV__) { const { componentName,