From 75deeb32fe1eac8da44913a36179b143dd24cfb2 Mon Sep 17 00:00:00 2001 From: Rick Hanlon Date: Wed, 13 Nov 2019 11:30:43 -0800 Subject: [PATCH] LogBox - Errors thrown in render are fatals Summary: The React team wants exceptions thrown during render to pop over the screen as fatals. Changelog: [Internal] Reviewed By: motiz88 Differential Revision: D18439258 fbshipit-source-id: dded7b9d93271c1a4eff682be521c7567dfe7d7e --- Libraries/Core/Devtools/parseErrorStack.js | 1 + Libraries/Core/ExceptionsManager.js | 5 ++++- Libraries/Core/ReactFiberErrorDialog.js | 1 + Libraries/LogBox/Data/LogBoxData.js | 10 +++++++--- Libraries/LogBox/Data/__tests__/LogBoxData-test.js | 2 ++ .../LogBox/Data/__tests__/parseLogBoxLog-test.js | 13 ++++++++----- Libraries/LogBox/Data/parseLogBoxLog.js | 3 ++- 7 files changed, 25 insertions(+), 10 deletions(-) diff --git a/Libraries/Core/Devtools/parseErrorStack.js b/Libraries/Core/Devtools/parseErrorStack.js index 613f4b1e922..8d54d46b5c3 100644 --- a/Libraries/Core/Devtools/parseErrorStack.js +++ b/Libraries/Core/Devtools/parseErrorStack.js @@ -17,6 +17,7 @@ export type ExtendedError = Error & { preventSymbolication?: boolean, componentStack?: string, forceRedbox?: boolean, + isComponentError?: boolean, }; function parseErrorStack(e: ExtendedError): Array { diff --git a/Libraries/Core/ExceptionsManager.js b/Libraries/Core/ExceptionsManager.js index 935a45d78ee..a3cfd810101 100644 --- a/Libraries/Core/ExceptionsManager.js +++ b/Libraries/Core/ExceptionsManager.js @@ -105,7 +105,10 @@ function reportException(e: ExtendedError, isFatal: boolean) { }); if (isHandledByLogBox) { - LogBoxData.addException(data); + LogBoxData.addException({ + ...data, + isComponentError: !!e.isComponentError, + }); } NativeExceptionsManager.reportException(data); diff --git a/Libraries/Core/ReactFiberErrorDialog.js b/Libraries/Core/ReactFiberErrorDialog.js index 936f716e6a5..3ea3b623462 100644 --- a/Libraries/Core/ReactFiberErrorDialog.js +++ b/Libraries/Core/ReactFiberErrorDialog.js @@ -41,6 +41,7 @@ function showErrorDialog(capturedError: CapturedError): boolean { } try { errorToHandle.componentStack = componentStack; + errorToHandle.isComponentError = true; } catch (e) {} handleException(errorToHandle, false); diff --git a/Libraries/LogBox/Data/LogBoxData.js b/Libraries/LogBox/Data/LogBoxData.js index 6925532b3ae..11ddce922a8 100644 --- a/Libraries/LogBox/Data/LogBoxData.js +++ b/Libraries/LogBox/Data/LogBoxData.js @@ -13,9 +13,13 @@ import LogBoxLog from './LogBoxLog'; import {parseLogBoxException} from './parseLogBoxLog'; import type {LogLevel} from './LogBoxLog'; -import type {Message, Category, ComponentStack} from './parseLogBoxLog'; +import type { + Message, + Category, + ComponentStack, + ExtendedExceptionData, +} from './parseLogBoxLog'; import parseErrorStack from '../../Core/Devtools/parseErrorStack'; -import type {ExceptionData} from '../../Core/NativeExceptionsManager'; import type {ExtendedError} from '../../Core/Devtools/parseErrorStack'; export type LogBoxLogs = Set; @@ -213,7 +217,7 @@ export function addLog(log: LogData): void { }); } -export function addException(error: ExceptionData): void { +export function addException(error: ExtendedExceptionData): void { // Parsing logs are expensive so we schedule this // otherwise spammy logs would pause rendering. setImmediate(() => { diff --git a/Libraries/LogBox/Data/__tests__/LogBoxData-test.js b/Libraries/LogBox/Data/__tests__/LogBoxData-test.js index 623b8936442..0315be0ff22 100644 --- a/Libraries/LogBox/Data/__tests__/LogBoxData-test.js +++ b/Libraries/LogBox/Data/__tests__/LogBoxData-test.js @@ -72,6 +72,7 @@ const addSoftErrors = errors => { {}, { message: '', + isComponentError: false, originalMessage: '', name: 'console.error', componentStack: '', @@ -94,6 +95,7 @@ const addFatalErrors = errors => { {}, { message: '', + isComponentError: false, originalMessage: '', name: 'console.error', componentStack: '', diff --git a/Libraries/LogBox/Data/__tests__/parseLogBoxLog-test.js b/Libraries/LogBox/Data/__tests__/parseLogBoxLog-test.js index 8de7037b571..8fed42107cf 100644 --- a/Libraries/LogBox/Data/__tests__/parseLogBoxLog-test.js +++ b/Libraries/LogBox/Data/__tests__/parseLogBoxLog-test.js @@ -16,7 +16,6 @@ jest.mock('../../../Core/Devtools/parseErrorStack', () => { }); const {parseLogBoxLog, parseLogBoxException} = require('../parseLogBoxLog'); -import type {ExceptionData} from '../../../Core/NativeExceptionsManager'; describe('parseLogBoxLog', () => { it('parses strings', () => { @@ -152,7 +151,7 @@ describe('parseLogBoxLog', () => { }); it('parses a syntax error', () => { - const error: ExceptionData = { + const error = { message: ` 197 | }); @@ -172,6 +171,7 @@ describe('parseLogBoxLog', () => { stack: [], id: 0, isFatal: true, + isComponentError: false, }; expect(parseLogBoxException(error)).toEqual({ @@ -195,9 +195,10 @@ describe('parseLogBoxLog', () => { }); it('parses a error log', () => { - const error: ExceptionData = { + const error = { id: 0, isFatal: false, + isComponentError: false, message: '### Error', originalMessage: '### Error', name: '', @@ -244,8 +245,9 @@ describe('parseLogBoxLog', () => { }); it('parses a fatal exception', () => { - const error: ExceptionData = { + const error = { id: 0, + isComponentError: false, isFatal: true, message: '### Fatal', originalMessage: '### Fatal', @@ -283,9 +285,10 @@ describe('parseLogBoxLog', () => { }); it('a malformed syntax error falls back to a fatal', () => { - const error: ExceptionData = { + const error = { id: 0, isFatal: true, + isComponentError: false, // Note no code frame. message: "TransformError SyntaxError: /path/to/RKJSModules/Apps/CrashReact/CrashReactApp.js: 'import' and 'export' may only appear at the top level (199:0)", diff --git a/Libraries/LogBox/Data/parseLogBoxLog.js b/Libraries/LogBox/Data/parseLogBoxLog.js index d417e05b64d..2481b9efe1e 100644 --- a/Libraries/LogBox/Data/parseLogBoxLog.js +++ b/Libraries/LogBox/Data/parseLogBoxLog.js @@ -16,6 +16,7 @@ import type {LogLevel} from './LogBoxLog'; import type {ExceptionData} from '../../Core/NativeExceptionsManager'; import type {Stack} from './LogBoxSymbolication'; +export type ExtendedExceptionData = ExceptionData & {isComponentError: boolean}; export type Category = string; export type CodeFrame = $ReadOnly<{| content: string, @@ -137,7 +138,7 @@ export function parseComponentStack(message: string): ComponentStack { } export function parseLogBoxException( - error: ExceptionData, + error: ExtendedExceptionData, ): {| level: LogLevel, category: Category,