From c901c43d11a243e7e4a9389f7e8f1440581ef7d9 Mon Sep 17 00:00:00 2001 From: Lulu Wu Date: Fri, 8 Oct 2021 11:06:46 -0700 Subject: [PATCH] Remove shared responsibility between LogBox and ExceptionsManager native module Summary: ## Context Right now we are using both LogBox and ExceptionsManager native module to report JS errors in ExceptionsManager.js, from below code we can tell they have some overlapping - when ```__DEV__ === true``` both could report the error. https://www.internalfb.com/code/fbsource/[5fb44bc926de87e62e6e538082496f22017698eb]/xplat/js/react-native-github/Libraries/Core/ExceptionsManager.js?lines=109-141 ## Changes In this diff overlapping is removed: in ```ExceptionsManager.js``` LogBox will be responsible for showing the error with dialog when ```__DEV__ === true```, when it's prod we'll use ExceptionsManager native module to report the error. As a result LogBox and ExceptionsManager native module don't share responsibilities any more. Changelog: [General][Changed] - Remove shared responsibility between LogBox and ExceptionsManager native module Reviewed By: philIip Differential Revision: D30942433 fbshipit-source-id: 8fceaaa431e5a460c0ccd151fe9831dcccbcf237 --- Libraries/Core/ExceptionsManager.js | 125 +++----- Libraries/Core/ExtendedError.js | 1 - .../Core/__tests__/ExceptionsManager-test.js | 286 ++++++++++++++---- Libraries/LogBox/Data/LogBoxData.js | 1 - .../LogBox/Data/__tests__/LogBoxData-test.js | 2 - React/CoreModules/RCTExceptionsManager.mm | 35 +-- .../modules/core/ExceptionsManagerModule.java | 26 +- 7 files changed, 290 insertions(+), 186 deletions(-) diff --git a/Libraries/Core/ExceptionsManager.js b/Libraries/Core/ExceptionsManager.js index d4826d4b54a..00caaf92ab9 100644 --- a/Libraries/Core/ExceptionsManager.js +++ b/Libraries/Core/ExceptionsManager.js @@ -56,94 +56,57 @@ function reportException( isFatal: boolean, reportToConsole: boolean, // only true when coming from handleException; the error has not yet been logged ) { - const NativeExceptionsManager = require('./NativeExceptionsManager').default; - if (NativeExceptionsManager) { - const parseErrorStack = require('./Devtools/parseErrorStack'); - const stack = parseErrorStack(e?.stack); - const currentExceptionID = ++exceptionID; - const originalMessage = e.message || ''; - let message = originalMessage; - if (e.componentStack != null) { - message += `\n\nThis error is located at:${e.componentStack}`; - } - const namePrefix = e.name == null || e.name === '' ? '' : `${e.name}: `; + const parseErrorStack = require('./Devtools/parseErrorStack'); + const stack = parseErrorStack(e?.stack); + const currentExceptionID = ++exceptionID; + const originalMessage = e.message || ''; + let message = originalMessage; + if (e.componentStack != null) { + message += `\n\nThis error is located at:${e.componentStack}`; + } + const namePrefix = e.name == null || e.name === '' ? '' : `${e.name}: `; - if (!message.startsWith(namePrefix)) { - message = namePrefix + message; - } + if (!message.startsWith(namePrefix)) { + message = namePrefix + message; + } - message = - e.jsEngine == null ? message : `${message}, js engine: ${e.jsEngine}`; + message = + e.jsEngine == null ? message : `${message}, js engine: ${e.jsEngine}`; - const isHandledByLogBox = - e.forceRedbox !== true && - global.RN$Bridgeless !== true && - !global.RN$Express; + const data = preprocessException({ + message, + originalMessage: message === originalMessage ? null : originalMessage, + name: e.name == null || e.name === '' ? null : e.name, + componentStack: + typeof e.componentStack === 'string' ? e.componentStack : null, + stack, + id: currentExceptionID, + isFatal, + extraData: { + jsEngine: e.jsEngine, + rawStack: e.stack, + }, + }); - const data = preprocessException({ - message, - originalMessage: message === originalMessage ? null : originalMessage, - name: e.name == null || e.name === '' ? null : e.name, - componentStack: - typeof e.componentStack === 'string' ? e.componentStack : null, - stack, - id: currentExceptionID, - isFatal, - extraData: { - jsEngine: e.jsEngine, - rawStack: e.stack, - - // Hack to hide native redboxes when in the LogBox experiment. - // This is intentionally untyped and stuffed here, because it is temporary. - suppressRedBox: isHandledByLogBox, - }, - }); - - if (reportToConsole) { - // we feed back into console.error, to make sure any methods that are - // monkey patched on top of console.error are called when coming from - // handleException - console.error(data.message); - } - - if (__DEV__ && isHandledByLogBox) { - const LogBox = require('../LogBox/LogBox'); - LogBox.addException({ - ...data, - isComponentError: !!e.isComponentError, - }); - } - - if (isFatal || e.type !== 'warn') { - NativeExceptionsManager.reportException(data); - - if (__DEV__ && !global.RN$Express) { - if (e.preventSymbolication === true) { - return; - } - const symbolicateStackTrace = require('./Devtools/symbolicateStackTrace'); - symbolicateStackTrace(stack) - .then(({stack: prettyStack}) => { - if (prettyStack) { - NativeExceptionsManager.updateExceptionMessage( - data.message, - prettyStack, - currentExceptionID, - ); - } else { - throw new Error('The stack is null'); - } - }) - .catch(error => { - console.log('Unable to symbolicate stack trace: ' + error.message); - }); - } - } - } else if (reportToConsole) { + if (reportToConsole) { // we feed back into console.error, to make sure any methods that are // monkey patched on top of console.error are called when coming from // handleException - console.error(e); + console.error(data.message); + } + + if (__DEV__) { + const LogBox = require('../LogBox/LogBox'); + LogBox.addException({ + ...data, + isComponentError: !!e.isComponentError, + }); + } else if (isFatal || e.type !== 'warn') { + const NativeExceptionsManager = require('./NativeExceptionsManager') + .default; + if (NativeExceptionsManager) { + NativeExceptionsManager.reportException(data); + } } } diff --git a/Libraries/Core/ExtendedError.js b/Libraries/Core/ExtendedError.js index 88e6331ced0..e6b9a651031 100644 --- a/Libraries/Core/ExtendedError.js +++ b/Libraries/Core/ExtendedError.js @@ -12,7 +12,6 @@ export type ExtendedError = Error & { jsEngine?: string, preventSymbolication?: boolean, componentStack?: string, - forceRedbox?: boolean, isComponentError?: boolean, type?: string, ... diff --git a/Libraries/Core/__tests__/ExceptionsManager-test.js b/Libraries/Core/__tests__/ExceptionsManager-test.js index ffd3be593ee..d85927d1231 100644 --- a/Libraries/Core/__tests__/ExceptionsManager-test.js +++ b/Libraries/Core/__tests__/ExceptionsManager-test.js @@ -12,6 +12,7 @@ const ExceptionsManager = require('../ExceptionsManager'); const NativeExceptionsManager = require('../NativeExceptionsManager').default; +const LogBox = require('../../LogBox/LogBox'); const ReactFiberErrorDialog = require('../ReactFiberErrorDialog').default; const fs = require('fs'); const path = require('path'); @@ -27,9 +28,13 @@ const capturedErrorDefaults = { describe('ExceptionsManager', () => { let nativeReportException; + let logBoxAddException; beforeEach(() => { jest.resetModules(); + jest.mock('../../LogBox/LogBox', () => ({ + addException: jest.fn(), + })); jest.mock('../NativeExceptionsManager', () => { return { default: { @@ -49,6 +54,7 @@ describe('ExceptionsManager', () => { ); jest.spyOn(console, 'error').mockReturnValue(undefined); nativeReportException = NativeExceptionsManager.reportException; + logBoxAddException = LogBox.addException; }); afterEach(() => { @@ -66,6 +72,11 @@ describe('ExceptionsManager', () => { error, }); + if (__DEV__) { + expect(nativeReportException.mock.calls.length).toBe(0); + expect(logBoxAddException.mock.calls.length).toBe(1); + return; + } expect(nativeReportException.mock.calls.length).toBe(1); const exceptionData = nativeReportException.mock.calls[0][0]; const formattedMessage = @@ -101,8 +112,15 @@ describe('ExceptionsManager', () => { error, }); - expect(nativeReportException.mock.calls.length).toBe(1); - const exceptionData = nativeReportException.mock.calls[0][0]; + let exceptionData; + + if (__DEV__) { + expect(logBoxAddException.mock.calls.length).toBe(1); + exceptionData = logBoxAddException.mock.calls[0][0]; + } else { + expect(nativeReportException.mock.calls.length).toBe(1); + exceptionData = nativeReportException.mock.calls[0][0]; + } expect(getLineFromFrame(exceptionData.stack[0])).toBe( "const error = new Error('Some error happened');", ); @@ -119,8 +137,15 @@ describe('ExceptionsManager', () => { error, }); - expect(nativeReportException.mock.calls.length).toBe(1); - const exceptionData = nativeReportException.mock.calls[0][0]; + let exceptionData; + + if (__DEV__) { + expect(logBoxAddException.mock.calls.length).toBe(1); + exceptionData = logBoxAddException.mock.calls[0][0]; + } else { + expect(nativeReportException.mock.calls.length).toBe(1); + exceptionData = nativeReportException.mock.calls[0][0]; + } expect(exceptionData.message).toBe( 'Error: ' + message + @@ -149,8 +174,15 @@ describe('ExceptionsManager', () => { error: message, }); - expect(nativeReportException.mock.calls.length).toBe(1); - const exceptionData = nativeReportException.mock.calls[0][0]; + let exceptionData; + + if (__DEV__) { + expect(logBoxAddException.mock.calls.length).toBe(1); + exceptionData = logBoxAddException.mock.calls[0][0]; + } else { + expect(nativeReportException.mock.calls.length).toBe(1); + exceptionData = nativeReportException.mock.calls[0][0]; + } const formattedMessage = message + '\n\n' + @@ -173,8 +205,15 @@ describe('ExceptionsManager', () => { error: null, }); - expect(nativeReportException.mock.calls.length).toBe(1); - const exceptionData = nativeReportException.mock.calls[0][0]; + let exceptionData; + + if (__DEV__) { + expect(logBoxAddException.mock.calls.length).toBe(1); + exceptionData = logBoxAddException.mock.calls[0][0]; + } else { + expect(nativeReportException.mock.calls.length).toBe(1); + exceptionData = nativeReportException.mock.calls[0][0]; + } const formattedMessage = 'Unspecified error' + '\n\n' + @@ -200,8 +239,15 @@ describe('ExceptionsManager', () => { error, }); - expect(nativeReportException.mock.calls.length).toBe(1); - const exceptionData = nativeReportException.mock.calls[0][0]; + let exceptionData; + + if (__DEV__) { + expect(logBoxAddException.mock.calls.length).toBe(1); + exceptionData = logBoxAddException.mock.calls[0][0]; + } else { + expect(nativeReportException.mock.calls.length).toBe(1); + exceptionData = nativeReportException.mock.calls[0][0]; + } expect(getLineFromFrame(exceptionData.stack[0])).toBe( "const error = Object.freeze(new Error('Some error happened'));", ); @@ -216,8 +262,12 @@ describe('ExceptionsManager', () => { error, }); - expect(nativeReportException).toHaveBeenCalled(); - expect(error.message).toBe(message); + if (__DEV__) { + expect(logBoxAddException).toHaveBeenCalled(); + } else { + expect(nativeReportException).toHaveBeenCalled(); + expect(error.message).toBe(message); + } }); test('can safely process the same error multiple times', () => { @@ -230,6 +280,7 @@ describe('ExceptionsManager', () => { ]; for (const componentStack of componentStacks) { nativeReportException.mockClear(); + logBoxAddException.mockClear(); const formattedMessage = 'ReferenceError: ' + message + @@ -242,8 +293,15 @@ describe('ExceptionsManager', () => { error, }); - expect(nativeReportException.mock.calls.length).toBe(1); - const exceptionData = nativeReportException.mock.calls[0][0]; + let exceptionData; + + if (__DEV__) { + expect(logBoxAddException.mock.calls.length).toBe(1); + exceptionData = logBoxAddException.mock.calls[0][0]; + } else { + expect(nativeReportException.mock.calls.length).toBe(1); + exceptionData = nativeReportException.mock.calls[0][0]; + } expect(exceptionData.message).toBe(formattedMessage); expect(exceptionData.originalMessage).toBe(message); expect(exceptionData.componentStack).toBe(componentStack); @@ -281,8 +339,15 @@ describe('ExceptionsManager', () => { console.error(error); - expect(nativeReportException.mock.calls.length).toBe(1); - const exceptionData = nativeReportException.mock.calls[0][0]; + let exceptionData; + + if (__DEV__) { + expect(logBoxAddException.mock.calls.length).toBe(1); + exceptionData = logBoxAddException.mock.calls[0][0]; + } else { + expect(nativeReportException.mock.calls.length).toBe(1); + exceptionData = nativeReportException.mock.calls[0][0]; + } const formattedMessage = 'Error: ' + message; expect(exceptionData.message).toBe(formattedMessage); expect(exceptionData.originalMessage).toBe(message); @@ -301,8 +366,15 @@ describe('ExceptionsManager', () => { console.error(message); - expect(nativeReportException.mock.calls.length).toBe(1); - const exceptionData = nativeReportException.mock.calls[0][0]; + let exceptionData; + + if (__DEV__) { + expect(logBoxAddException.mock.calls.length).toBe(1); + exceptionData = logBoxAddException.mock.calls[0][0]; + } else { + expect(nativeReportException.mock.calls.length).toBe(1); + exceptionData = nativeReportException.mock.calls[0][0]; + } expect(exceptionData.message).toBe('console.error: Some error happened'); expect(exceptionData.originalMessage).toBe('Some error happened'); expect(exceptionData.name).toBe('console.error'); @@ -318,8 +390,15 @@ describe('ExceptionsManager', () => { console.error(...args); - expect(nativeReportException.mock.calls.length).toBe(1); - const exceptionData = nativeReportException.mock.calls[0][0]; + let exceptionData; + + if (__DEV__) { + expect(logBoxAddException.mock.calls.length).toBe(1); + exceptionData = logBoxAddException.mock.calls[0][0]; + } else { + expect(nativeReportException.mock.calls.length).toBe(1); + exceptionData = nativeReportException.mock.calls[0][0]; + } expect(exceptionData.message).toBe( 'console.error: 42 true ["symbol" failed to stringify] {"y":null}', ); @@ -367,7 +446,11 @@ describe('ExceptionsManager', () => { console.error(...args); - expect(nativeReportException).toHaveBeenCalled(); + if (__DEV__) { + expect(logBoxAddException).toHaveBeenCalled(); + } else { + expect(nativeReportException).toHaveBeenCalled(); + } }); test('does not log "warn"-type errors', () => { @@ -399,8 +482,15 @@ describe('ExceptionsManager', () => { console.error(error); - expect(nativeReportException.mock.calls.length).toBe(1); - const exceptionData = nativeReportException.mock.calls[0][0]; + let exceptionData; + + if (__DEV__) { + expect(logBoxAddException.mock.calls.length).toBe(1); + exceptionData = logBoxAddException.mock.calls[0][0]; + } else { + expect(nativeReportException.mock.calls.length).toBe(1); + exceptionData = nativeReportException.mock.calls[0][0]; + } expect(getLineFromFrame(exceptionData.stack[0])).toBe( "const error = new Error('Some error happened');", ); @@ -414,8 +504,15 @@ describe('ExceptionsManager', () => { ExceptionsManager.handleException(error, true); - expect(nativeReportException.mock.calls.length).toBe(1); - const exceptionData = nativeReportException.mock.calls[0][0]; + let exceptionData; + + if (__DEV__) { + expect(logBoxAddException.mock.calls.length).toBe(1); + exceptionData = logBoxAddException.mock.calls[0][0]; + } else { + expect(nativeReportException.mock.calls.length).toBe(1); + exceptionData = nativeReportException.mock.calls[0][0]; + } const formattedMessage = 'Error: ' + message; expect(exceptionData.message).toBe(formattedMessage); expect(exceptionData.originalMessage).toBe(message); @@ -434,8 +531,15 @@ describe('ExceptionsManager', () => { ExceptionsManager.handleException(error, false); - expect(nativeReportException.mock.calls.length).toBe(1); - const exceptionData = nativeReportException.mock.calls[0][0]; + let exceptionData; + + if (__DEV__) { + expect(logBoxAddException.mock.calls.length).toBe(1); + exceptionData = logBoxAddException.mock.calls[0][0]; + } else { + expect(nativeReportException.mock.calls.length).toBe(1); + exceptionData = nativeReportException.mock.calls[0][0]; + } const formattedMessage = 'Error: ' + message; expect(exceptionData.message).toBe(formattedMessage); expect(exceptionData.originalMessage).toBe(message); @@ -453,8 +557,15 @@ describe('ExceptionsManager', () => { ExceptionsManager.handleException(message, true); - expect(nativeReportException.mock.calls.length).toBe(1); - const exceptionData = nativeReportException.mock.calls[0][0]; + let exceptionData; + + if (__DEV__) { + expect(logBoxAddException.mock.calls.length).toBe(1); + exceptionData = logBoxAddException.mock.calls[0][0]; + } else { + expect(nativeReportException.mock.calls.length).toBe(1); + exceptionData = nativeReportException.mock.calls[0][0]; + } expect(exceptionData.message).toBe(message); expect(exceptionData.originalMessage).toBe(null); expect(exceptionData.name).toBe(null); @@ -473,8 +584,15 @@ describe('ExceptionsManager', () => { ExceptionsManager.handleException(error, true); - expect(nativeReportException.mock.calls.length).toBe(1); - const exceptionData = nativeReportException.mock.calls[0][0]; + let exceptionData; + + if (__DEV__) { + expect(logBoxAddException.mock.calls.length).toBe(1); + exceptionData = logBoxAddException.mock.calls[0][0]; + } else { + expect(nativeReportException.mock.calls.length).toBe(1); + exceptionData = nativeReportException.mock.calls[0][0]; + } expect(getLineFromFrame(exceptionData.stack[0])).toBe( "const error = new Error('Some error happened');", ); @@ -486,7 +604,11 @@ describe('ExceptionsManager', () => { ExceptionsManager.handleException(error, true); - expect(nativeReportException).toHaveBeenCalled(); + if (__DEV__) { + expect(logBoxAddException).toHaveBeenCalled(); + } else { + expect(nativeReportException).toHaveBeenCalled(); + } }); }); @@ -520,19 +642,30 @@ describe('ExceptionsManager', () => { ExceptionsManager.unstable_setExceptionDecorator(decorator); ExceptionsManager.handleException(error, true); - expect(nativeReportException.mock.calls.length).toBe(2); expect(decorator.mock.calls.length).toBe(1); - - const withoutDecoratorInstalled = nativeReportException.mock.calls[0][0]; - const afterDecorator = nativeReportException.mock.calls[1][0]; const beforeDecorator = decorator.mock.calls[0][0]; + let withoutDecoratorInstalled; + let afterDecorator; + + if (__DEV__) { + expect(logBoxAddException.mock.calls.length).toBe(2); + withoutDecoratorInstalled = logBoxAddException.mock.calls[0][0]; + afterDecorator = logBoxAddException.mock.calls[1][0]; + } else { + expect(nativeReportException.mock.calls.length).toBe(2); + withoutDecoratorInstalled = nativeReportException.mock.calls[0][0]; + afterDecorator = nativeReportException.mock.calls[1][0]; + } + expect(afterDecorator.id).toEqual(beforeDecorator.id); // id will change between successive exceptions delete withoutDecoratorInstalled.id; delete beforeDecorator.id; delete afterDecorator.id; + delete withoutDecoratorInstalled.isComponentError; + delete afterDecorator.isComponentError; expect(withoutDecoratorInstalled).toEqual(beforeDecorator); expect(afterDecorator).toEqual({ @@ -553,7 +686,11 @@ describe('ExceptionsManager', () => { ExceptionsManager.handleException(error, true); expect(decorator).not.toHaveBeenCalled(); - expect(nativeReportException).toHaveBeenCalled(); + if (__DEV__) { + expect(logBoxAddException).toHaveBeenCalled(); + } else { + expect(nativeReportException).toHaveBeenCalled(); + } }); test('prevents decorator recursion from error handler', () => { @@ -569,10 +706,17 @@ describe('ExceptionsManager', () => { ExceptionsManager.unstable_setExceptionDecorator(decorator); ExceptionsManager.handleException(error, true); - expect(nativeReportException).toHaveBeenCalledTimes(1); - expect(nativeReportException.mock.calls[0][0].message).toMatch( - /decorated: .*Some error happened/, - ); + if (__DEV__) { + expect(logBoxAddException).toHaveBeenCalledTimes(1); + expect(logBoxAddException.mock.calls[0][0].message).toMatch( + /decorated: .*Some error happened/, + ); + } else { + expect(nativeReportException).toHaveBeenCalledTimes(1); + expect(nativeReportException.mock.calls[0][0].message).toMatch( + /decorated: .*Some error happened/, + ); + } expect(mockError).toHaveBeenCalledTimes(2); expect(mockError.mock.calls[0][0]).toMatch( /Logging an error within the decorator/, @@ -595,13 +739,23 @@ describe('ExceptionsManager', () => { ExceptionsManager.unstable_setExceptionDecorator(decorator); console.error(error); - expect(nativeReportException).toHaveBeenCalledTimes(2); - expect(nativeReportException.mock.calls[0][0].message).toMatch( - /Logging an error within the decorator/, - ); - expect(nativeReportException.mock.calls[1][0].message).toMatch( - /decorated: .*Some error happened/, - ); + if (__DEV__) { + expect(logBoxAddException).toHaveBeenCalledTimes(2); + expect(logBoxAddException.mock.calls[0][0].message).toMatch( + /Logging an error within the decorator/, + ); + expect(logBoxAddException.mock.calls[1][0].message).toMatch( + /decorated: .*Some error happened/, + ); + } else { + expect(nativeReportException).toHaveBeenCalledTimes(2); + expect(nativeReportException.mock.calls[0][0].message).toMatch( + /Logging an error within the decorator/, + ); + expect(nativeReportException.mock.calls[1][0].message).toMatch( + /decorated: .*Some error happened/, + ); + } expect(mockError).toHaveBeenCalledTimes(2); // console.error calls are chained without exception pre-processing, so decorator doesn't apply expect(mockError.mock.calls[0][0].toString()).toMatch( @@ -621,11 +775,19 @@ describe('ExceptionsManager', () => { ExceptionsManager.unstable_setExceptionDecorator(decorator); ExceptionsManager.handleException(error, true); - expect(nativeReportException).toHaveBeenCalledTimes(1); - // Exceptions in decorators are ignored and the decorator is not applied - expect(nativeReportException.mock.calls[0][0].message).toMatch( - /Error: Some error happened/, - ); + if (__DEV__) { + expect(logBoxAddException).toHaveBeenCalledTimes(1); + // Exceptions in decorators are ignored and the decorator is not applied + expect(logBoxAddException.mock.calls[0][0].message).toMatch( + /Error: Some error happened/, + ); + } else { + expect(nativeReportException).toHaveBeenCalledTimes(1); + // Exceptions in decorators are ignored and the decorator is not applied + expect(nativeReportException.mock.calls[0][0].message).toMatch( + /Error: Some error happened/, + ); + } expect(mockError).toHaveBeenCalledTimes(1); expect(mockError.mock.calls[0][0]).toMatch(/Error: Some error happened/); }); @@ -639,11 +801,19 @@ describe('ExceptionsManager', () => { ExceptionsManager.unstable_setExceptionDecorator(decorator); console.error(error); - expect(nativeReportException).toHaveBeenCalledTimes(1); - // Exceptions in decorators are ignored and the decorator is not applied - expect(nativeReportException.mock.calls[0][0].message).toMatch( - /Error: Some error happened/, - ); + if (__DEV__) { + expect(logBoxAddException).toHaveBeenCalledTimes(1); + // Exceptions in decorators are ignored and the decorator is not applied + expect(logBoxAddException.mock.calls[0][0].message).toMatch( + /Error: Some error happened/, + ); + } else { + expect(nativeReportException).toHaveBeenCalledTimes(1); + // Exceptions in decorators are ignored and the decorator is not applied + expect(nativeReportException.mock.calls[0][0].message).toMatch( + /Error: Some error happened/, + ); + } expect(mockError).toHaveBeenCalledTimes(1); expect(mockError.mock.calls[0][0].toString()).toMatch( /Error: Some error happened/, diff --git a/Libraries/LogBox/Data/LogBoxData.js b/Libraries/LogBox/Data/LogBoxData.js index 55d8dd4165a..1c2f8426024 100644 --- a/Libraries/LogBox/Data/LogBoxData.js +++ b/Libraries/LogBox/Data/LogBoxData.js @@ -100,7 +100,6 @@ export function reportLogBoxError( ): void { const ExceptionsManager = require('../../Core/ExceptionsManager'); - error.forceRedbox = true; error.message = `${LOGBOX_ERROR_MESSAGE}\n\n${error.message}`; if (componentStack != null) { error.componentStack = componentStack; diff --git a/Libraries/LogBox/Data/__tests__/LogBoxData-test.js b/Libraries/LogBox/Data/__tests__/LogBoxData-test.js index 5e4225e9465..556aef41c77 100644 --- a/Libraries/LogBox/Data/__tests__/LogBoxData-test.js +++ b/Libraries/LogBox/Data/__tests__/LogBoxData-test.js @@ -676,7 +676,6 @@ describe('LogBoxData', () => { const receivedError = ExceptionsManager.handleException.mock.calls[0][0]; expect(receivedError.componentStack).toBe(' in Component (file.js:1)'); - expect(receivedError.forceRedbox).toBe(true); expect(receivedError.message).toBe( 'An error was thrown when attempting to render log messages via LogBox.\n\nSimulated Error', ); @@ -689,7 +688,6 @@ describe('LogBoxData', () => { const receivedError = ExceptionsManager.handleException.mock.calls[0][0]; expect(receivedError.componentStack).toBeUndefined(); - expect(receivedError.forceRedbox).toBe(true); expect(receivedError.message).toBe( 'An error was thrown when attempting to render log messages via LogBox.\n\nSimulated Error', ); diff --git a/React/CoreModules/RCTExceptionsManager.mm b/React/CoreModules/RCTExceptionsManager.mm index 175c5612cff..749e3914eb6 100644 --- a/React/CoreModules/RCTExceptionsManager.mm +++ b/React/CoreModules/RCTExceptionsManager.mm @@ -35,15 +35,10 @@ RCT_EXPORT_MODULE() return self; } -- (void)reportSoft:(NSString *)message - stack:(NSArray *)stack - exceptionId:(double)exceptionId - suppressRedBox:(BOOL)suppressRedBox +- (void)reportSoft:(NSString *)message stack:(NSArray *)stack exceptionId:(double)exceptionId { - if (!suppressRedBox) { - RCTRedBox *redbox = [_moduleRegistry moduleForName:"RedBox"]; - [redbox showErrorMessage:message withStack:stack errorCookie:(int)exceptionId]; - } + RCTRedBox *redbox = [_moduleRegistry moduleForName:"RedBox"]; + [redbox showErrorMessage:message withStack:stack errorCookie:(int)exceptionId]; if (_delegate) { [_delegate handleSoftJSExceptionWithMessage:message @@ -52,15 +47,10 @@ RCT_EXPORT_MODULE() } } -- (void)reportFatal:(NSString *)message - stack:(NSArray *)stack - exceptionId:(double)exceptionId - suppressRedBox:(BOOL)suppressRedBox +- (void)reportFatal:(NSString *)message stack:(NSArray *)stack exceptionId:(double)exceptionId { - if (!suppressRedBox) { - RCTRedBox *redbox = [_moduleRegistry moduleForName:"RedBox"]; - [redbox showErrorMessage:message withStack:stack errorCookie:(int)exceptionId]; - } + RCTRedBox *redbox = [_moduleRegistry moduleForName:"RedBox"]; + [redbox showErrorMessage:message withStack:stack errorCookie:(int)exceptionId]; if (_delegate) { [_delegate handleFatalJSExceptionWithMessage:message @@ -72,7 +62,7 @@ RCT_EXPORT_MODULE() if (!RCT_DEBUG && reloadRetries < _maxReloadAttempts) { reloadRetries++; RCTTriggerReloadCommandListeners(@"JS Crash Reload"); - } else if (!RCT_DEV || !suppressRedBox) { + } else if (!RCT_DEV) { NSString *description = [@"Unhandled JS Exception: " stringByAppendingString:message]; NSDictionary *errorInfo = @{NSLocalizedDescriptionKey : description, RCTJSStackTraceKey : stack}; RCTFatal([NSError errorWithDomain:RCTErrorDomain code:0 userInfo:errorInfo]); @@ -84,7 +74,7 @@ RCT_EXPORT_METHOD(reportSoftException : (NSArray *)stack exceptionId : (double)exceptionId) { - [self reportSoft:message stack:stack exceptionId:exceptionId suppressRedBox:NO]; + [self reportSoft:message stack:stack exceptionId:exceptionId]; } RCT_EXPORT_METHOD(reportFatalException @@ -92,7 +82,7 @@ RCT_EXPORT_METHOD(reportFatalException : (NSArray *)stack exceptionId : (double)exceptionId) { - [self reportFatal:message stack:stack exceptionId:exceptionId suppressRedBox:NO]; + [self reportFatal:message stack:stack exceptionId:exceptionId]; } RCT_EXPORT_METHOD(updateExceptionMessage @@ -120,7 +110,6 @@ RCT_EXPORT_METHOD(reportException : (JS::NativeExceptionsManager::ExceptionData { NSString *message = data.message(); double exceptionId = data.id_(); - id extraData = data.extraData(); // Reserialize data.stack() into an array of untyped dictionaries. // TODO: (moti) T53588496 Replace `(NSArray *)stack` in @@ -141,13 +130,11 @@ RCT_EXPORT_METHOD(reportException : (JS::NativeExceptionsManager::ExceptionData } [stackArray addObject:frameDict]; } - NSDictionary *dict = (NSDictionary *)extraData; - BOOL suppressRedBox = [[dict objectForKey:@"suppressRedBox"] boolValue]; if (data.isFatal()) { - [self reportFatal:message stack:stackArray exceptionId:exceptionId suppressRedBox:suppressRedBox]; + [self reportFatal:message stack:stackArray exceptionId:exceptionId]; } else { - [self reportSoft:message stack:stackArray exceptionId:exceptionId suppressRedBox:suppressRedBox]; + [self reportSoft:message stack:stackArray exceptionId:exceptionId]; } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/modules/core/ExceptionsManagerModule.java b/ReactAndroid/src/main/java/com/facebook/react/modules/core/ExceptionsManagerModule.java index 265ecf88be0..bb5e4be1b1a 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/modules/core/ExceptionsManagerModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/modules/core/ExceptionsManagerModule.java @@ -65,28 +65,16 @@ public class ExceptionsManagerModule extends NativeExceptionsManagerSpec { public void reportException(ReadableMap data) { String message = data.hasKey("message") ? data.getString("message") : ""; ReadableArray stack = data.hasKey("stack") ? data.getArray("stack") : Arguments.createArray(); - int id = data.hasKey("id") ? data.getInt("id") : -1; boolean isFatal = data.hasKey("isFatal") ? data.getBoolean("isFatal") : false; - if (mDevSupportManager.getDevSupportEnabled()) { - boolean suppressRedBox = false; - if (data.getMap("extraData") != null && data.getMap("extraData").hasKey("suppressRedBox")) { - suppressRedBox = data.getMap("extraData").getBoolean("suppressRedBox"); - } - - if (!suppressRedBox) { - mDevSupportManager.showNewJSError(message, stack, id); - } + String extraDataAsJson = ExceptionDataHelper.getExtraDataAsJson(data); + if (isFatal) { + throw new JavascriptException(JSStackTrace.format(message, stack)) + .setExtraDataAsJson(extraDataAsJson); } else { - String extraDataAsJson = ExceptionDataHelper.getExtraDataAsJson(data); - if (isFatal) { - throw new JavascriptException(JSStackTrace.format(message, stack)) - .setExtraDataAsJson(extraDataAsJson); - } else { - FLog.e(ReactConstants.TAG, JSStackTrace.format(message, stack)); - if (extraDataAsJson != null) { - FLog.d(ReactConstants.TAG, "extraData: %s", extraDataAsJson); - } + FLog.e(ReactConstants.TAG, JSStackTrace.format(message, stack)); + if (extraDataAsJson != null) { + FLog.d(ReactConstants.TAG, "extraData: %s", extraDataAsJson); } } }