diff --git a/packages/react-native/Libraries/LogBox/Data/LogBoxData.js b/packages/react-native/Libraries/LogBox/Data/LogBoxData.js index 367a78dd70db24..08c631c8fec2d8 100644 --- a/packages/react-native/Libraries/LogBox/Data/LogBoxData.js +++ b/packages/react-native/Libraries/LogBox/Data/LogBoxData.js @@ -82,9 +82,9 @@ let warningFilter: WarningFilter = function (format) { return { finalFormat: format, forceDialogImmediately: false, - suppressDialog_LEGACY: true, + suppressDialog_LEGACY: false, suppressCompletely: false, - monitorEvent: 'unknown', + monitorEvent: 'warning_unhandled', monitorListVersion: 0, monitorSampleRate: 1, }; diff --git a/packages/react-native/Libraries/LogBox/__tests__/LogBox-integration-test.js b/packages/react-native/Libraries/LogBox/__tests__/LogBox-integration-test.js index 07ea7507d77752..d0c836c4774d23 100644 --- a/packages/react-native/Libraries/LogBox/__tests__/LogBox-integration-test.js +++ b/packages/react-native/Libraries/LogBox/__tests__/LogBox-integration-test.js @@ -11,45 +11,29 @@ import { DoesNotUseKey, FragmentWithProp, + ManualConsoleError, + ManualConsoleErrorWithStack, } from './__fixtures__/ReactWarningFixtures'; import * as React from 'react'; const LogBoxData = require('../Data/LogBoxData'); const TestRenderer = require('react-test-renderer'); -const installLogBox = () => { - const LogBox = require('../LogBox'); +const ExceptionsManager = require('../../Core/ExceptionsManager.js'); +const installLogBox = () => { + const LogBox = require('../LogBox').default; LogBox.install(); }; const uninstallLogBox = () => { - const LogBox = require('../LogBox'); + const LogBox = require('../LogBox').default; LogBox.uninstall(); }; -const BEFORE_SLASH_RE = /(?:\/[a-zA-Z]+\/)(.+?)(?:\/.+)\//; - -const cleanPath = message => { - return message.replace(BEFORE_SLASH_RE, '/path/to/'); -}; - -const cleanLog = logs => { - return logs.map(log => { - return { - ...log, - componentStack: log.componentStack.map(stack => ({ - ...stack, - fileName: cleanPath(stack.fileName), - })), - }; - }); -}; - -// TODO(T71117418): Re-enable skipped LogBox integration tests once React component -// stack frames are the same internally and in open source. -// eslint-disable-next-line jest/no-disabled-tests -describe.skip('LogBox', () => { +// TODO: we can remove all the symetric matchers once OSS lands component stack frames. +// For now, the component stack parsing differs in ways we can't easily detect in this test. +describe('LogBox', () => { const {error, warn} = console; const mockError = jest.fn(); const mockWarn = jest.fn(); @@ -57,10 +41,14 @@ describe.skip('LogBox', () => { beforeEach(() => { jest.resetModules(); jest.restoreAllMocks(); + jest.spyOn(console, 'error').mockImplementation(() => {}); mockError.mockClear(); mockWarn.mockClear(); - + // Reset ExceptionManager patching. + if (console._errorOriginal) { + console._errorOriginal = null; + } (console: any).error = mockError; (console: any).warn = mockWarn; }); @@ -79,7 +67,10 @@ describe.skip('LogBox', () => { // so we can assert on what React logs. jest.spyOn(console, 'error'); - const output = TestRenderer.create(); + let output; + TestRenderer.act(() => { + output = TestRenderer.create(); + }); // The key error should always be the highest severity. // In LogBox, we expect these errors to: @@ -88,16 +79,37 @@ describe.skip('LogBox', () => { // - Pass to console.error, with a "Warning" prefix so it does not pop a RedBox. expect(output).toBeDefined(); expect(mockWarn).not.toBeCalled(); - expect(console.error.mock.calls[0].map(cleanPath)).toMatchSnapshot( - 'Log sent from React', - ); - expect(cleanLog(spy.mock.calls[0])).toMatchSnapshot('Log added to LogBox'); - expect(mockError.mock.calls[0].map(cleanPath)).toMatchSnapshot( - 'Log passed to console error', - ); + expect(console.error).toBeCalledTimes(1); + expect(console.error.mock.calls[0]).toEqual([ + 'Warning: Each child in a list should have a unique "key" prop.%s%s See https://reactjs.org/link/warning-keys for more information.%s', + '\n\nCheck the render method of `DoesNotUseKey`.', + '', + expect.stringMatching('at DoesNotUseKey'), + ]); + expect(spy).toHaveBeenCalledWith({ + level: 'error', + category: expect.stringContaining( + 'Warning: Each child in a list should have a unique', + ), + componentStack: expect.anything(), + componentStackType: 'stack', + message: { + content: + 'Warning: Each child in a list should have a unique "key" prop.\n\nCheck the render method of `DoesNotUseKey`. See https://reactjs.org/link/warning-keys for more information.', + substitutions: [ + {length: 45, offset: 62}, + {length: 0, offset: 107}, + ], + }, + }); // The Warning: prefix is added due to a hack in LogBox to prevent double logging. - expect(mockError.mock.calls[0][0].startsWith('Warning: ')).toBe(true); + // We also interpolate the string before passing to the underlying console method. + expect(mockError.mock.calls[0]).toEqual([ + expect.stringMatching( + 'Warning: Each child in a list should have a unique "key" prop.\n\nCheck the render method of `DoesNotUseKey`. See https://reactjs.org/link/warning-keys for more information.\n at ', + ), + ]); }); it('integrates with React and handles a fragment warning in LogBox', () => { @@ -108,7 +120,10 @@ describe.skip('LogBox', () => { // so we can assert on what React logs. jest.spyOn(console, 'error'); - const output = TestRenderer.create(); + let output; + TestRenderer.act(() => { + output = TestRenderer.create(); + }); // The fragment warning is not as severe. For this warning we don't want to // pop open a dialog, so we show a collapsed error UI. @@ -118,15 +133,125 @@ describe.skip('LogBox', () => { // - Pass to console.error, with a "Warning" prefix so it does not pop a RedBox. expect(output).toBeDefined(); expect(mockWarn).not.toBeCalled(); - expect(console.error.mock.calls[0].map(cleanPath)).toMatchSnapshot( - 'Log sent from React', - ); - expect(cleanLog(spy.mock.calls[0])).toMatchSnapshot('Log added to LogBox'); - expect(mockError.mock.calls[0].map(cleanPath)).toMatchSnapshot( - 'Log passed to console error', - ); + expect(console.error).toBeCalledTimes(1); + expect(console.error.mock.calls[0]).toEqual([ + 'Warning: Invalid prop `%s` supplied to `React.Fragment`. React.Fragment can only have `key` and `children` props.%s', + 'invalid', + expect.stringMatching('at FragmentWithProp'), + ]); + expect(spy).toHaveBeenCalledWith({ + level: 'error', + category: expect.stringContaining('Warning: Invalid prop'), + componentStack: expect.anything(), + componentStackType: expect.stringMatching(/(stack|legacy)/), + message: { + content: + 'Warning: Invalid prop `invalid` supplied to `React.Fragment`. React.Fragment can only have `key` and `children` props.', + substitutions: [{length: 7, offset: 23}], + }, + }); + + // The Warning: prefix is added due to a hack in LogBox to prevent double logging. + // We also interpolate the string before passing to the underlying console method. + expect(mockError.mock.calls[0]).toEqual([ + expect.stringMatching( + 'Warning: Invalid prop `invalid` supplied to `React.Fragment`. React.Fragment can only have `key` and `children` props.\n at FragmentWithProp', + ), + ]); + }); + + it('handles a manual console.error without a component stack in LogBox', () => { + const LogBox = require('../LogBox').default; + const spy = jest.spyOn(LogBox, 'addException'); + installLogBox(); + + // console.error handling depends on installing the ExceptionsManager error reporter. + ExceptionsManager.installConsoleErrorReporter(); + + // Spy console.error after LogBox is installed + // so we can assert on what React logs. + jest.spyOn(console, 'error'); + + let output; + TestRenderer.act(() => { + output = TestRenderer.create(); + }); + + // Manual console errors should show a collapsed error dialog. + // When there is no component stack, we expect these errors to: + // - Go to the LogBox patch and fall through to console.error. + // - Get picked up by the ExceptionsManager console.error override. + // - Get passed back to LogBox via addException (non-fatal). + expect(output).toBeDefined(); + expect(mockWarn).not.toBeCalled(); + expect(spy).toBeCalledTimes(1); + expect(console.error).toBeCalledTimes(1); + expect(console.error.mock.calls[0]).toEqual(['Manual console error']); + expect(spy).toHaveBeenCalledWith({ + id: 1, + isComponentError: false, + isFatal: false, + name: 'console.error', + originalMessage: 'Manual console error', + message: 'console.error: Manual console error', + extraData: expect.anything(), + componentStack: null, + stack: expect.anything(), + }); + + // No Warning: prefix is added due since this is falling through. + expect(mockError.mock.calls[0]).toEqual(['Manual console error']); + }); + + it('handles a manual console.error with a component stack in LogBox', () => { + const spy = jest.spyOn(LogBoxData, 'addLog'); + installLogBox(); + + // console.error handling depends on installing the ExceptionsManager error reporter. + ExceptionsManager.installConsoleErrorReporter(); + + // Spy console.error after LogBox is installed + // so we can assert on what React logs. + jest.spyOn(console, 'error'); + + let output; + TestRenderer.act(() => { + output = TestRenderer.create(); + }); + + // Manual console errors should show a collapsed error dialog. + // When there is a component stack, we expect these errors to: + // - Go to the LogBox patch and be detected as a React error. + // - Check the warning filter to see if there is a fiter setting. + // - Call console.error with the parsed error. + // - Get picked up by ExceptionsManager console.error override. + // - Log to console.error. + expect(output).toBeDefined(); + expect(mockWarn).not.toBeCalled(); + expect(console.error).toBeCalledTimes(1); + expect(spy).toBeCalledTimes(1); + expect(console.error.mock.calls[0]).toEqual([ + expect.stringContaining( + 'Manual console error\n at ManualConsoleErrorWithStack', + ), + ]); + expect(spy).toHaveBeenCalledWith({ + level: 'error', + category: expect.stringContaining('Warning: Manual console error'), + componentStack: expect.anything(), + componentStackType: 'stack', + message: { + content: 'Warning: Manual console error', + substitutions: [], + }, + }); // The Warning: prefix is added due to a hack in LogBox to prevent double logging. - expect(mockError.mock.calls[0][0].startsWith('Warning: ')).toBe(true); + // We also interpolate the string before passing to the underlying console method. + expect(mockError.mock.calls[0]).toEqual([ + expect.stringMatching( + 'Warning: Manual console error\n at ManualConsoleErrorWithStack', + ), + ]); }); }); diff --git a/packages/react-native/Libraries/LogBox/__tests__/LogBox-test.js b/packages/react-native/Libraries/LogBox/__tests__/LogBox-test.js index be5ea785bbb5e1..a8a40825354a1f 100644 --- a/packages/react-native/Libraries/LogBox/__tests__/LogBox-test.js +++ b/packages/react-native/Libraries/LogBox/__tests__/LogBox-test.js @@ -13,6 +13,7 @@ const LogBoxData = require('../Data/LogBoxData'); const LogBox = require('../LogBox').default; +const ExceptionsManager = require('../../Core/ExceptionsManager.js'); declare var console: any; @@ -34,15 +35,18 @@ describe('LogBox', () => { beforeEach(() => { jest.resetModules(); + jest.restoreAllMocks(); console.error = jest.fn(); - console.log = jest.fn(); console.warn = jest.fn(); }); afterEach(() => { LogBox.uninstall(); + // Reset ExceptionManager patching. + if (console._errorOriginal) { + console._errorOriginal = null; + } console.error = error; - console.log = log; console.warn = warn; }); @@ -95,7 +99,7 @@ describe('LogBox', () => { }); it('registers warnings', () => { - jest.mock('../Data/LogBoxData'); + jest.spyOn(LogBoxData, 'addLog'); LogBox.install(); @@ -105,13 +109,14 @@ describe('LogBox', () => { }); it('reports a LogBox exception if we fail to add warnings', () => { - jest.mock('../Data/LogBoxData'); - const mockError = new Error('Simulated error'); + jest.spyOn(LogBoxData, 'addLog'); + jest.spyOn(LogBoxData, 'reportLogBoxError'); // Picking a random implementation detail to simulate throwing. - (LogBoxData.isMessageIgnored: any).mockImplementation(() => { + jest.spyOn(LogBoxData, 'isMessageIgnored').mockImplementation(() => { throw mockError; }); + const mockError = new Error('Simulated error'); LogBox.install(); @@ -123,7 +128,8 @@ describe('LogBox', () => { }); it('only registers errors beginning with "Warning: "', () => { - jest.mock('../Data/LogBoxData'); + jest.spyOn(LogBoxData, 'addLog'); + jest.spyOn(LogBoxData, 'checkWarningFilter'); LogBox.install(); @@ -133,7 +139,8 @@ describe('LogBox', () => { }); it('registers react errors with the formatting from filter', () => { - jest.mock('../Data/LogBoxData'); + jest.spyOn(LogBoxData, 'addLog'); + jest.spyOn(LogBoxData, 'checkWarningFilter'); mockFilterResult({ finalFormat: 'Custom format', @@ -157,7 +164,8 @@ describe('LogBox', () => { }); it('registers errors with component stack as errors by default', () => { - jest.mock('../Data/LogBoxData'); + jest.spyOn(LogBoxData, 'addLog'); + jest.spyOn(LogBoxData, 'checkWarningFilter'); mockFilterResult({}); @@ -174,7 +182,8 @@ describe('LogBox', () => { }); it('registers errors with component stack as errors by default if not found in warning filter', () => { - jest.mock('../Data/LogBoxData'); + jest.spyOn(LogBoxData, 'addLog'); + jest.spyOn(LogBoxData, 'checkWarningFilter'); mockFilterResult({ monitorEvent: 'warning_unhandled', @@ -193,10 +202,12 @@ describe('LogBox', () => { }); it('registers errors with component stack with legacy suppression as warning', () => { - jest.mock('../Data/LogBoxData'); + jest.spyOn(LogBoxData, 'addLog'); + jest.spyOn(LogBoxData, 'checkWarningFilter'); mockFilterResult({ suppressDialog_LEGACY: true, + monitorEvent: 'warning', }); LogBox.install(); @@ -211,10 +222,12 @@ describe('LogBox', () => { }); it('registers errors with component stack and a forced dialog as fatals', () => { - jest.mock('../Data/LogBoxData'); + jest.spyOn(LogBoxData, 'addLog'); + jest.spyOn(LogBoxData, 'checkWarningFilter'); mockFilterResult({ forceDialogImmediately: true, + monitorEvent: 'warning', }); LogBox.install(); @@ -229,7 +242,8 @@ describe('LogBox', () => { }); it('registers warning module errors with the formatting from filter', () => { - jest.mock('../Data/LogBoxData'); + jest.spyOn(LogBoxData, 'addLog'); + jest.spyOn(LogBoxData, 'checkWarningFilter'); mockFilterResult({ finalFormat: 'Custom format', @@ -248,7 +262,8 @@ describe('LogBox', () => { }); it('registers warning module errors as errors by default', () => { - jest.mock('../Data/LogBoxData'); + jest.spyOn(LogBoxData, 'addLog'); + jest.spyOn(LogBoxData, 'checkWarningFilter'); mockFilterResult({}); @@ -262,10 +277,12 @@ describe('LogBox', () => { }); it('registers warning module errors with only legacy suppression as warning', () => { - jest.mock('../Data/LogBoxData'); + jest.spyOn(LogBoxData, 'addLog'); + jest.spyOn(LogBoxData, 'checkWarningFilter'); mockFilterResult({ suppressDialog_LEGACY: true, + monitorEvent: 'warning', }); LogBox.install(); @@ -277,10 +294,12 @@ describe('LogBox', () => { }); it('registers warning module errors with a forced dialog as fatals', () => { - jest.mock('../Data/LogBoxData'); + jest.spyOn(LogBoxData, 'addLog'); + jest.spyOn(LogBoxData, 'checkWarningFilter'); mockFilterResult({ forceDialogImmediately: true, + monitorEvent: 'warning', }); LogBox.install(); @@ -292,10 +311,12 @@ describe('LogBox', () => { }); it('ignores warning module errors that are suppressed completely', () => { - jest.mock('../Data/LogBoxData'); + jest.spyOn(LogBoxData, 'addLog'); + jest.spyOn(LogBoxData, 'checkWarningFilter'); mockFilterResult({ suppressCompletely: true, + monitorEvent: 'warning', }); LogBox.install(); @@ -305,10 +326,11 @@ describe('LogBox', () => { }); it('ignores warning module errors that are pattern ignored', () => { - jest.mock('../Data/LogBoxData'); + jest.spyOn(LogBoxData, 'checkWarningFilter'); + jest.spyOn(LogBoxData, 'isMessageIgnored').mockReturnValue(true); + jest.spyOn(LogBoxData, 'addLog'); mockFilterResult({}); - (LogBoxData.isMessageIgnored: any).mockReturnValue(true); LogBox.install(); @@ -317,10 +339,11 @@ describe('LogBox', () => { }); it('ignores warning module errors that are from LogBox itself', () => { - jest.mock('../Data/LogBoxData'); + jest.spyOn(LogBoxData, 'checkWarningFilter'); + jest.spyOn(LogBoxData, 'isLogBoxErrorMessage').mockReturnValue(true); + jest.spyOn(LogBoxData, 'addLog'); mockFilterResult({}); - (LogBoxData.isLogBoxErrorMessage: any).mockReturnValue(true); LogBox.install(); @@ -329,8 +352,9 @@ describe('LogBox', () => { }); it('ignores logs that are pattern ignored"', () => { - jest.mock('../Data/LogBoxData'); - (LogBoxData.isMessageIgnored: any).mockReturnValue(true); + jest.spyOn(LogBoxData, 'checkWarningFilter'); + jest.spyOn(LogBoxData, 'isMessageIgnored').mockReturnValue(true); + jest.spyOn(LogBoxData, 'addLog'); LogBox.install(); @@ -339,8 +363,8 @@ describe('LogBox', () => { }); it('does not add logs that are from LogBox itself"', () => { - jest.mock('../Data/LogBoxData'); - (LogBoxData.isLogBoxErrorMessage: any).mockReturnValue(true); + jest.spyOn(LogBoxData, 'isLogBoxErrorMessage').mockReturnValue(true); + jest.spyOn(LogBoxData, 'addLog'); LogBox.install(); @@ -349,7 +373,7 @@ describe('LogBox', () => { }); it('ignores logs starting with "(ADVICE)"', () => { - jest.mock('../Data/LogBoxData'); + jest.spyOn(LogBoxData, 'addLog'); LogBox.install(); @@ -358,7 +382,7 @@ describe('LogBox', () => { }); it('does not ignore logs formatted to start with "(ADVICE)"', () => { - jest.mock('../Data/LogBoxData'); + jest.spyOn(LogBoxData, 'addLog'); LogBox.install(); @@ -376,7 +400,7 @@ describe('LogBox', () => { }); it('ignores console methods after uninstalling', () => { - jest.mock('../Data/LogBoxData'); + jest.spyOn(LogBoxData, 'addLog'); LogBox.install(); LogBox.uninstall(); @@ -389,7 +413,7 @@ describe('LogBox', () => { }); it('does not add logs after uninstalling', () => { - jest.mock('../Data/LogBoxData'); + jest.spyOn(LogBoxData, 'addLog'); LogBox.install(); LogBox.uninstall(); @@ -406,7 +430,7 @@ describe('LogBox', () => { }); it('does not add exceptions after uninstalling', () => { - jest.mock('../Data/LogBoxData'); + jest.spyOn(LogBoxData, 'addException'); LogBox.install(); LogBox.uninstall(); @@ -482,4 +506,80 @@ describe('LogBox', () => { 'Custom: after installing for the second time', ); }); + + it('registers errors with component stack as errors by default, when ExceptionManager is registered first', () => { + jest.spyOn(LogBoxData, 'checkWarningFilter'); + jest.spyOn(LogBoxData, 'addLog'); + + ExceptionsManager.installConsoleErrorReporter(); + LogBox.install(); + + console.error( + 'HIT\n at Text (/path/to/Component:30:175)\n at DoesNotUseKey', + ); + + expect(LogBoxData.addLog).toBeCalledWith( + expect.objectContaining({level: 'error'}), + ); + expect(LogBoxData.checkWarningFilter).toBeCalledWith( + 'HIT\n at Text (/path/to/Component:30:175)\n at DoesNotUseKey', + ); + }); + + it('registers errors with component stack as errors by default, when ExceptionManager is registered second', () => { + jest.spyOn(LogBoxData, 'checkWarningFilter'); + jest.spyOn(LogBoxData, 'addLog'); + + LogBox.install(); + ExceptionsManager.installConsoleErrorReporter(); + + console.error( + 'HIT\n at Text (/path/to/Component:30:175)\n at DoesNotUseKey', + ); + + expect(LogBoxData.addLog).toBeCalledWith( + expect.objectContaining({level: 'error'}), + ); + expect(LogBoxData.checkWarningFilter).toBeCalledWith( + 'HIT\n at Text (/path/to/Component:30:175)\n at DoesNotUseKey', + ); + }); + + it('registers errors without component stack as errors by default, when ExceptionManager is registered first', () => { + jest.spyOn(LogBoxData, 'checkWarningFilter'); + jest.spyOn(LogBoxData, 'addException'); + + ExceptionsManager.installConsoleErrorReporter(); + LogBox.install(); + + console.error('HIT'); + + // Errors without a component stack skip the warning filter and + // fall through to the ExceptionManager, which are then reported + // back to LogBox as non-fatal exceptions, in a convuluted dance + // in the most legacy cruft way. + expect(LogBoxData.addException).toBeCalledWith( + expect.objectContaining({originalMessage: 'HIT'}), + ); + expect(LogBoxData.checkWarningFilter).not.toBeCalled(); + }); + + it('registers errors without component stack as errors by default, when ExceptionManager is registered second', () => { + jest.spyOn(LogBoxData, 'checkWarningFilter'); + jest.spyOn(LogBoxData, 'addException'); + + LogBox.install(); + ExceptionsManager.installConsoleErrorReporter(); + + console.error('HIT'); + + // Errors without a component stack skip the warning filter and + // fall through to the ExceptionManager, which are then reported + // back to LogBox as non-fatal exceptions, in a convuluted dance + // in the most legacy cruft way. + expect(LogBoxData.addException).toBeCalledWith( + expect.objectContaining({originalMessage: 'HIT'}), + ); + expect(LogBoxData.checkWarningFilter).not.toBeCalled(); + }); }); diff --git a/packages/react-native/Libraries/LogBox/__tests__/__fixtures__/ReactWarningFixtures.js b/packages/react-native/Libraries/LogBox/__tests__/__fixtures__/ReactWarningFixtures.js index 51b85b1fbe1aef..2d13e41dfe0683 100644 --- a/packages/react-native/Libraries/LogBox/__tests__/__fixtures__/ReactWarningFixtures.js +++ b/packages/react-native/Libraries/LogBox/__tests__/__fixtures__/ReactWarningFixtures.js @@ -30,3 +30,27 @@ export const FragmentWithProp = () => { ); }; + +export const ManualConsoleError = () => { + console.error('Manual console error'); + return ( + + {['foo', 'bar'].map(item => ( + {item} + ))} + + ); +}; + +export const ManualConsoleErrorWithStack = () => { + console.error( + 'Manual console error\n at ManualConsoleErrorWithStack (/path/to/ManualConsoleErrorWithStack:30:175)\n at TestApp', + ); + return ( + + {['foo', 'bar'].map(item => ( + {item} + ))} + + ); +};