From 5022fdfd5f9b3639c9c8bb2df31100586d1cfd01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Fri, 3 Apr 2020 19:01:40 -0700 Subject: [PATCH] Refactor Error Dialog Logging (#18487) * Remove unnecessary CapturedError fields. componentName is not necessary and is misleading when the error is caused elsewhere in the stack. The stack is sufficient. The many error boundary fields are unnecessary because they can be inferred by the boundary itself. * Don't attempt to build a stack twice If it was possible, it would've been done in createCapturedValue. * Push the work needed by the works into the forks This avoids needing this in the npm published case. --- .../src/ReactCapturedValue.js | 10 -- .../src/ReactFiberCommitWork.js | 39 ------ .../src/ReactFiberErrorDialog.js | 9 +- .../src/ReactFiberErrorLogger.js | 128 +++++++++--------- .../react-reconciler/src/ReactFiberThrow.js | 8 +- .../src/forks/ReactFiberErrorDialog.native.js | 19 ++- .../src/forks/ReactFiberErrorDialog.www.js | 18 ++- 7 files changed, 111 insertions(+), 120 deletions(-) diff --git a/packages/react-reconciler/src/ReactCapturedValue.js b/packages/react-reconciler/src/ReactCapturedValue.js index 726a22c976d5..a3ec7597e611 100644 --- a/packages/react-reconciler/src/ReactCapturedValue.js +++ b/packages/react-reconciler/src/ReactCapturedValue.js @@ -17,16 +17,6 @@ export type CapturedValue = {| stack: string | null, |}; -export type CapturedError = {| - componentName: ?string, - componentStack: string, - error: mixed, - errorBoundary: ?Object, - errorBoundaryFound: boolean, - errorBoundaryName: string | null, - willRetry: boolean, -|}; - export function createCapturedValue( value: T, source: Fiber, diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index e9cf9d96d22b..16925f04defe 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -18,7 +18,6 @@ import type { import type {Fiber} from './ReactFiber'; import type {FiberRoot} from './ReactFiberRoot'; import type {ExpirationTime} from './ReactFiberExpirationTime'; -import type {CapturedValue, CapturedError} from './ReactCapturedValue'; import type {SuspenseState} from './ReactFiberSuspenseComponent'; import type {FunctionComponentUpdateQueue} from './ReactFiberHooks'; import type {Wakeable} from 'shared/ReactTypes'; @@ -75,7 +74,6 @@ import invariant from 'shared/invariant'; import {onCommitUnmount} from './ReactFiberDevToolsHook'; import {getStackByFiberInDevAndProd} from './ReactCurrentFiber'; -import {logCapturedError} from './ReactFiberErrorLogger'; import {resolveDefaultProps} from './ReactFiberLazyComponent'; import { getCommitTime, @@ -143,43 +141,6 @@ if (__DEV__) { const PossiblyWeakSet = typeof WeakSet === 'function' ? WeakSet : Set; -export function logError(boundary: Fiber, errorInfo: CapturedValue) { - const source = errorInfo.source; - let stack = errorInfo.stack; - if (stack === null && source !== null) { - stack = getStackByFiberInDevAndProd(source); - } - - const capturedError: CapturedError = { - componentName: source !== null ? getComponentName(source.type) : null, - componentStack: stack !== null ? stack : '', - error: errorInfo.value, - errorBoundary: null, - errorBoundaryName: null, - errorBoundaryFound: false, - willRetry: false, - }; - - if (boundary !== null && boundary.tag === ClassComponent) { - capturedError.errorBoundary = boundary.stateNode; - capturedError.errorBoundaryName = getComponentName(boundary.type); - capturedError.errorBoundaryFound = true; - capturedError.willRetry = true; - } - - try { - logCapturedError(capturedError); - } catch (e) { - // This method must not throw, or React internal state will get messed up. - // If console.error is overridden, or logCapturedError() shows a dialog that throws, - // we want to report this error outside of the normal stack as a last resort. - // https://github.com/facebook/react/issues/13188 - setTimeout(() => { - throw e; - }); - } -} - const callComponentWillUnmountWithTimer = function(current, instance) { instance.props = current.memoizedProps; instance.state = current.memoizedState; diff --git a/packages/react-reconciler/src/ReactFiberErrorDialog.js b/packages/react-reconciler/src/ReactFiberErrorDialog.js index a10cb9abdcf7..f7e914cda747 100644 --- a/packages/react-reconciler/src/ReactFiberErrorDialog.js +++ b/packages/react-reconciler/src/ReactFiberErrorDialog.js @@ -7,11 +7,16 @@ * @flow */ -import type {CapturedError} from './ReactCapturedValue'; +import type {Fiber} from './ReactFiber'; +import type {CapturedValue} from './ReactCapturedValue'; // This module is forked in different environments. // By default, return `true` to log errors to the console. // Forks can return `false` if this isn't desirable. -export function showErrorDialog(capturedError: CapturedError): boolean { + +export function showErrorDialog( + boundary: Fiber, + errorInfo: CapturedValue, +): boolean { return true; } diff --git a/packages/react-reconciler/src/ReactFiberErrorLogger.js b/packages/react-reconciler/src/ReactFiberErrorLogger.js index 724907d561e0..28d9e2572fc9 100644 --- a/packages/react-reconciler/src/ReactFiberErrorLogger.js +++ b/packages/react-reconciler/src/ReactFiberErrorLogger.js @@ -7,82 +7,88 @@ * @flow */ -import type {CapturedError} from './ReactCapturedValue'; +import type {Fiber} from './ReactFiber'; +import type {CapturedValue} from './ReactCapturedValue'; import {showErrorDialog} from './ReactFiberErrorDialog'; +import {ClassComponent} from './ReactWorkTags'; +import getComponentName from 'shared/getComponentName'; -export function logCapturedError(capturedError: CapturedError): void { - const logError = showErrorDialog(capturedError); +export function logCapturedError( + boundary: Fiber, + errorInfo: CapturedValue, +): void { + try { + const logError = showErrorDialog(boundary, errorInfo); - // Allow injected showErrorDialog() to prevent default console.error logging. - // This enables renderers like ReactNative to better manage redbox behavior. - if (logError === false) { - return; - } - - const error = (capturedError.error: any); - if (__DEV__) { - const { - componentName, - componentStack, - errorBoundaryName, - errorBoundaryFound, - willRetry, - } = capturedError; + // Allow injected showErrorDialog() to prevent default console.error logging. + // This enables renderers like ReactNative to better manage redbox behavior. + if (logError === false) { + return; + } - // Browsers support silencing uncaught errors by calling - // `preventDefault()` in window `error` handler. - // We record this information as an expando on the error. - if (error != null && error._suppressLogging) { - if (errorBoundaryFound && willRetry) { - // The error is recoverable and was silenced. - // Ignore it and don't print the stack addendum. - // This is handy for testing error boundaries without noise. - return; + const error = (errorInfo.value: any); + if (__DEV__) { + const source = errorInfo.source; + const stack = errorInfo.stack; + const componentStack = stack !== null ? stack : ''; + // Browsers support silencing uncaught errors by calling + // `preventDefault()` in window `error` handler. + // We record this information as an expando on the error. + if (error != null && error._suppressLogging) { + if (boundary.tag === ClassComponent) { + // The error is recoverable and was silenced. + // Ignore it and don't print the stack addendum. + // This is handy for testing error boundaries without noise. + return; + } + // The error is fatal. Since the silencing might have + // been accidental, we'll surface it anyway. + // However, the browser would have silenced the original error + // so we'll print it first, and then print the stack addendum. + console['error'](error); // Don't transform to our wrapper + // For a more detailed description of this block, see: + // https://github.com/facebook/react/pull/13384 } - // The error is fatal. Since the silencing might have - // been accidental, we'll surface it anyway. - // However, the browser would have silenced the original error - // so we'll print it first, and then print the stack addendum. - console['error'](error); // Don't transform to our wrapper - // For a more detailed description of this block, see: - // https://github.com/facebook/react/pull/13384 - } - const componentNameMessage = componentName - ? `The above error occurred in the <${componentName}> component:` - : 'The above error occurred in one of your React components:'; + const componentName = source ? getComponentName(source.type) : null; + const componentNameMessage = componentName + ? `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. - if (errorBoundaryFound && errorBoundaryName) { - if (willRetry) { + let errorBoundaryMessage; + const errorBoundaryName = getComponentName(boundary.type); + if (errorBoundaryName) { errorBoundaryMessage = `React will try to recreate this component tree from scratch ` + `using the error boundary you provided, ${errorBoundaryName}.`; } else { errorBoundaryMessage = - `This error was initially handled by the error boundary ${errorBoundaryName}.\n` + - `Recreating the tree from scratch failed so React will unmount the tree.`; + 'Consider adding an error boundary to your tree to customize error handling behavior.\n' + + 'Visit https://fb.me/react-error-boundaries to learn more about error boundaries.'; } + const combinedMessage = + `${componentNameMessage}\n${componentStack}\n\n` + + `${errorBoundaryMessage}`; + + // 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); // Don't transform to our wrapper } else { - errorBoundaryMessage = - 'Consider adding an error boundary to your tree to customize error handling behavior.\n' + - 'Visit https://fb.me/react-error-boundaries to learn more about error boundaries.'; + // 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); // Don't transform to our wrapper } - const combinedMessage = - `${componentNameMessage}${componentStack}\n\n` + - `${errorBoundaryMessage}`; - - // 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); // Don't transform to our wrapper - } else { - // 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); // Don't transform to our wrapper + } catch (e) { + // This method must not throw, or React internal state will get messed up. + // If console.error is overridden, or logCapturedError() shows a dialog that throws, + // we want to report this error outside of the normal stack as a last resort. + // https://github.com/facebook/react/issues/13188 + setTimeout(() => { + throw e; + }); } } diff --git a/packages/react-reconciler/src/ReactFiberThrow.js b/packages/react-reconciler/src/ReactFiberThrow.js index 797ea802cccd..649b6675e81d 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.js +++ b/packages/react-reconciler/src/ReactFiberThrow.js @@ -40,7 +40,6 @@ import { ForceUpdate, enqueueUpdate, } from './ReactUpdateQueue'; -import {logError} from './ReactFiberCommitWork'; import {getStackByFiberInDevAndProd} from './ReactCurrentFiber'; import {markFailedErrorBoundaryForHotReloading} from './ReactFiberHotReloading'; import { @@ -55,6 +54,7 @@ import { isAlreadyFailedLegacyErrorBoundary, pingSuspendedRoot, } from './ReactFiberWorkLoop'; +import {logCapturedError} from './ReactFiberErrorLogger'; import {Sync} from './ReactFiberExpirationTime'; @@ -74,7 +74,7 @@ function createRootErrorUpdate( const error = errorInfo.value; update.callback = () => { onUncaughtError(error); - logError(fiber, errorInfo); + logCapturedError(fiber, errorInfo); }; return update; } @@ -90,7 +90,7 @@ function createClassErrorUpdate( if (typeof getDerivedStateFromError === 'function') { const error = errorInfo.value; update.payload = () => { - logError(fiber, errorInfo); + logCapturedError(fiber, errorInfo); return getDerivedStateFromError(error); }; } @@ -110,7 +110,7 @@ function createClassErrorUpdate( markLegacyErrorBoundaryAsFailed(this); // Only log here if componentDidCatch is the only error boundary method defined - logError(fiber, errorInfo); + logCapturedError(fiber, errorInfo); } const error = errorInfo.value; const stack = errorInfo.stack; diff --git a/packages/react-reconciler/src/forks/ReactFiberErrorDialog.native.js b/packages/react-reconciler/src/forks/ReactFiberErrorDialog.native.js index d31a45fc8a7c..728e73bad19c 100644 --- a/packages/react-reconciler/src/forks/ReactFiberErrorDialog.native.js +++ b/packages/react-reconciler/src/forks/ReactFiberErrorDialog.native.js @@ -7,10 +7,14 @@ * @flow */ -import type {CapturedError} from '../ReactCapturedValue'; +import type {Fiber} from '../ReactFiber'; +import type {CapturedValue} from '../ReactCapturedValue'; + +import {ClassComponent} from '../ReactWorkTags'; // Module provided by RN: import {ReactFiberErrorDialog as RNImpl} from 'react-native/Libraries/ReactPrivate/ReactNativePrivateInterface'; + import invariant from 'shared/invariant'; invariant( @@ -18,6 +22,17 @@ invariant( 'Expected ReactFiberErrorDialog.showErrorDialog to be a function.', ); -export function showErrorDialog(capturedError: CapturedError): boolean { +export function showErrorDialog( + boundary: Fiber, + errorInfo: CapturedValue, +): boolean { + const capturedError = { + componentStack: errorInfo.stack !== null ? errorInfo.stack : '', + error: errorInfo.value, + errorBoundary: + boundary !== null && boundary.tag === ClassComponent + ? boundary.stateNode + : null, + }; return RNImpl.showErrorDialog(capturedError); } diff --git a/packages/react-reconciler/src/forks/ReactFiberErrorDialog.www.js b/packages/react-reconciler/src/forks/ReactFiberErrorDialog.www.js index eb676a8afbce..32bd10b45021 100644 --- a/packages/react-reconciler/src/forks/ReactFiberErrorDialog.www.js +++ b/packages/react-reconciler/src/forks/ReactFiberErrorDialog.www.js @@ -7,7 +7,10 @@ * @flow */ -import type {CapturedError} from '../ReactCapturedValue'; +import type {Fiber} from '../ReactFiber'; +import type {CapturedValue} from '../ReactCapturedValue'; + +import {ClassComponent} from '../ReactWorkTags'; import invariant from 'shared/invariant'; @@ -18,6 +21,17 @@ invariant( 'Expected ReactFiberErrorDialog.showErrorDialog to be a function.', ); -export function showErrorDialog(capturedError: CapturedError): boolean { +export function showErrorDialog( + boundary: Fiber, + errorInfo: CapturedValue, +): boolean { + const capturedError = { + componentStack: errorInfo.stack !== null ? errorInfo.stack : '', + error: errorInfo.value, + errorBoundary: + boundary !== null && boundary.tag === ClassComponent + ? boundary.stateNode + : null, + }; return ReactFiberErrorDialogWWW.showErrorDialog(capturedError); }