diff --git a/Libraries/Core/ExceptionsManager.js b/Libraries/Core/ExceptionsManager.js index d97a4d468ac..45b2e174f3f 100644 --- a/Libraries/Core/ExceptionsManager.js +++ b/Libraries/Core/ExceptionsManager.js @@ -11,6 +11,7 @@ 'use strict'; import type {ExtendedError} from './Devtools/parseErrorStack'; +import * as LogBoxData from '../LogBox/Data/LogBoxData'; import type {ExceptionData} from './NativeExceptionsManager'; class SyntheticError extends Error { @@ -81,6 +82,14 @@ function reportException(e: ExtendedError, isFatal: boolean) { message = e.jsEngine == null ? message : `${message}, js engine: ${e.jsEngine}`; + // TransformErrors need to be popped to the user and can happen both in JS + // through Fast Resfresh and through native when reloading a broken bundle. + // We want a consistent experience here, so we're opting to always pass + // these errors to the native redbox handling. + const isHandledByLogBox = + !/^TransformError SyntaxError: /.test(originalMessage) && + global.__reactExperimentalLogBox; + const data = preprocessException({ message, originalMessage: message === originalMessage ? null : originalMessage, @@ -93,9 +102,17 @@ function reportException(e: ExtendedError, isFatal: boolean) { 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 (isHandledByLogBox) { + LogBoxData.addException(data); + } + NativeExceptionsManager.reportException(data); if (__DEV__) { @@ -156,8 +173,13 @@ function reactConsoleErrorHandler() { } else { console._errorOriginal.apply(console, arguments); const stringifySafe = require('../Utilities/stringifySafe'); - const str = Array.prototype.map.call(arguments, stringifySafe).join(', '); - if (str.slice(0, 10) === '"Warning: ') { + const str = Array.prototype.map + .call(arguments, value => + typeof value === 'string' ? value : stringifySafe(value), + ) + .join(' '); + + if (str.slice(0, 9) === 'Warning: ') { // React warnings use console.error so that a stack trace is shown, but // we don't (currently) want these to show a redbox // (Note: Logic duplicated in polyfills/console.js.) diff --git a/Libraries/Core/__tests__/ExceptionsManager-test.js b/Libraries/Core/__tests__/ExceptionsManager-test.js index 3cf7f49cf13..2c7c8fbd277 100644 --- a/Libraries/Core/__tests__/ExceptionsManager-test.js +++ b/Libraries/Core/__tests__/ExceptionsManager-test.js @@ -301,10 +301,8 @@ describe('ExceptionsManager', () => { expect(nativeReportException.mock.calls.length).toBe(1); const exceptionData = nativeReportException.mock.calls[0][0]; - expect(exceptionData.message).toBe( - 'console.error: "Some error happened"', - ); - expect(exceptionData.originalMessage).toBe('"Some error happened"'); + expect(exceptionData.message).toBe('console.error: Some error happened'); + expect(exceptionData.originalMessage).toBe('Some error happened'); expect(exceptionData.name).toBe('console.error'); expect( getLineFromFrame(getFirstFrameInThisFile(exceptionData.stack)), @@ -321,10 +319,10 @@ describe('ExceptionsManager', () => { expect(nativeReportException.mock.calls.length).toBe(1); const exceptionData = nativeReportException.mock.calls[0][0]; expect(exceptionData.message).toBe( - 'console.error: 42, true, ["symbol" failed to stringify], {"y":null}', + 'console.error: 42 true ["symbol" failed to stringify] {"y":null}', ); expect(exceptionData.originalMessage).toBe( - '42, true, ["symbol" failed to stringify], {"y":null}', + '42 true ["symbol" failed to stringify] {"y":null}', ); expect(exceptionData.name).toBe('console.error'); expect( diff --git a/Libraries/LogBox/Data/LogBoxData.js b/Libraries/LogBox/Data/LogBoxData.js index 8fdf9b29203..151d782f9f5 100644 --- a/Libraries/LogBox/Data/LogBoxData.js +++ b/Libraries/LogBox/Data/LogBoxData.js @@ -11,9 +11,14 @@ ('use strict'); import LogBoxLog from './LogBoxLog'; -import parseLogBoxLog from './parseLogBoxLog'; +import { + parseLogBoxLog, + parseCategory, + parseComponentStack, +} from './parseLogBoxLog'; import type {LogLevel} from './LogBoxLog'; import parseErrorStack from '../../Core/Devtools/parseErrorStack'; +import type {ExceptionData} from '../../Core/NativeExceptionsManager'; export type LogBoxLogs = Set; @@ -54,7 +59,7 @@ function handleUpdate(): void { } } -export function add(level: LogLevel, args: $ReadOnlyArray): void { +export function addLog(level: LogLevel, args: $ReadOnlyArray): void { const errorForStackTrace = new Error(); // Parsing logs are expensive so we schedule this @@ -107,6 +112,43 @@ export function symbolicateLogLazy(log: LogBoxLog) { log.symbolicate(); } +export function addException(error: ExceptionData): void { + // Parsing logs are expensive so we schedule this + // otherwise spammy logs would pause rendering. + setImmediate(() => { + const {category, message} = parseCategory([ + error.originalMessage != null ? error.originalMessage : 'Unknown', + ]); + + // We don't want to store these logs because they trigger a + // state update whenever we add them to the store, which is + // expensive to noisy logs. If we later want to display these + // we will store them in a different state object. + if (isMessageIgnored(message.content)) { + return; + } + + const lastLog = Array.from(logs).pop(); + if (lastLog && lastLog.category === category) { + lastLog.incrementCount(); + } else { + logs.add( + new LogBoxLog( + 'error', + message, + error.stack, + category, + error.componentStack != null + ? parseComponentStack(error.componentStack) + : [], + ), + ); + } + + handleUpdate(); + }); +} + export function clear(): void { if (logs.size > 0) { logs.clear(); diff --git a/Libraries/LogBox/Data/__tests__/LogBoxData-test.js b/Libraries/LogBox/Data/__tests__/LogBoxData-test.js index 0db334fdd10..126f05c35e7 100644 --- a/Libraries/LogBox/Data/__tests__/LogBoxData-test.js +++ b/Libraries/LogBox/Data/__tests__/LogBoxData-test.js @@ -36,55 +36,71 @@ const observe = () => { }; }; -const logAndFlush = logs => { +const addLogs = logs => { logs.forEach(log => { - LogBoxData.add(log.level, log.args); + LogBoxData.addLog('warn', typeof log === 'string' ? [log] : log); }); - - jest.runAllImmediates(); }; -const logAndFlushAndUpdate = logs => { - logAndFlush(logs); - - // We run immediates again to flush the updates. - jest.runAllImmediates(); +const addExceptions = errors => { + errors.forEach(error => { + LogBoxData.addException( + Object.assign( + {}, + { + message: '', + originalMessage: '', + name: 'console.error', + componentStack: '', + stack: [], + id: 0, + isFatal: false, + }, + typeof error === 'string' + ? {message: error, originalMessage: error} + : error, + ), + ); + }); }; + +beforeEach(() => { + jest.resetModules(); +}); + describe('LogBoxData', () => { - beforeEach(() => { - jest.resetModules(); - }); - it('adds and dismisses logs', () => { - logAndFlush([{level: 'warn', args: ['A']}]); + addLogs(['A']); + addExceptions(['B']); + jest.runAllImmediates(); - expect(registry().length).toBe(1); + expect(registry().length).toBe(2); expect(registry()[0]).toBeDefined(); + expect(registry()[1]).toBeDefined(); + LogBoxData.dismiss(registry()[0]); + expect(registry().length).toBe(1); LogBoxData.dismiss(registry()[0]); expect(registry().length).toBe(0); expect(registry()[0]).toBeUndefined(); }); it('clears all logs', () => { - logAndFlush([ - {level: 'warn', args: ['A']}, - {level: 'warn', args: ['B']}, - {level: 'warn', args: ['C']}, - ]); + addLogs(['A', 'B', 'C']); + addExceptions(['D']); + jest.runAllImmediates(); - expect(registry().length).toBe(3); + expect(registry().length).toBe(4); LogBoxData.clear(); expect(registry().length).toBe(0); }); it('keeps logs in chronological order', () => { - logAndFlush([ - {level: 'warn', args: ['A']}, - {level: 'warn', args: ['B']}, - {level: 'warn', args: ['C']}, - ]); + addLogs(['A']); + addExceptions(['B']); + addLogs(['C']); + jest.runAllImmediates(); let logs = registry(); expect(logs.length).toBe(3); @@ -92,7 +108,8 @@ describe('LogBoxData', () => { expect(logs[1].category).toEqual('B'); expect(logs[2].category).toEqual('C'); - logAndFlush([{level: 'warn', args: ['A']}]); + addLogs(['A']); + jest.runAllImmediates(); // Expect `A` to be added to the end of the registry. logs = registry(); @@ -103,8 +120,9 @@ describe('LogBoxData', () => { expect(logs[3].category).toEqual('A'); }); - it('increments the count of previous log with matching category', () => { - logAndFlush([{level: 'warn', args: ['A']}, {level: 'warn', args: ['B']}]); + it('increments the count of previous log with matching category (logs)', () => { + addLogs(['A', 'B']); + jest.runAllImmediates(); let logs = registry(); expect(logs.length).toBe(2); @@ -113,7 +131,8 @@ describe('LogBoxData', () => { expect(logs[1].category).toEqual('B'); expect(logs[1].count).toBe(1); - logAndFlush([{level: 'warn', args: ['B']}]); + addLogs(['B']); + jest.runAllImmediates(); // Expect `B` to be rolled into the last log. logs = registry(); @@ -124,12 +143,33 @@ describe('LogBoxData', () => { expect(logs[1].count).toBe(2); }); - it('ignores logs matching patterns', () => { - logAndFlush([ - {level: 'warn', args: ['A!']}, - {level: 'warn', args: ['B?']}, - {level: 'warn', args: ['C!']}, - ]); + it('increments the count of previous log with matching category (exceptions)', () => { + addExceptions(['A', 'B']); + jest.runAllImmediates(); + + let logs = registry(); + expect(logs.length).toBe(2); + expect(logs[0].category).toEqual('A'); + expect(logs[0].count).toBe(1); + expect(logs[1].category).toEqual('B'); + expect(logs[1].count).toBe(1); + + addExceptions(['B']); + jest.runAllImmediates(); + + // Expect `B` to be rolled into the last log. + logs = registry(); + expect(logs.length).toBe(2); + expect(logs[0].category).toEqual('A'); + expect(logs[0].count).toBe(1); + expect(logs[1].category).toEqual('B'); + expect(logs[1].count).toBe(2); + }); + + it('ignores logs matching patterns (logs)', () => { + addLogs(['A!', 'B?', 'C!']); + jest.runAllImmediates(); + expect(filteredRegistry().length).toBe(3); LogBoxData.addIgnorePatterns(['!']); @@ -139,12 +179,38 @@ describe('LogBoxData', () => { expect(filteredRegistry().length).toBe(0); }); - it('ignores logs matching regexs or pattern', () => { - logAndFlush([ - {level: 'warn', args: ['There are 4 dogs']}, - {level: 'warn', args: ['There are 3 cats']}, - {level: 'warn', args: ['There are H cats']}, - ]); + it('ignores logs matching patterns (exceptions)', () => { + addExceptions(['A!', 'B?', 'C!']); + jest.runAllImmediates(); + + expect(filteredRegistry().length).toBe(3); + + LogBoxData.addIgnorePatterns(['!']); + expect(filteredRegistry().length).toBe(1); + + LogBoxData.addIgnorePatterns(['?']); + expect(filteredRegistry().length).toBe(0); + }); + + it('ignores matching regexs or pattern (logs)', () => { + addLogs(['There are 4 dogs', 'There are 3 cats', 'There are H cats']); + jest.runAllImmediates(); + + expect(filteredRegistry().length).toBe(3); + + LogBoxData.addIgnorePatterns(['dogs']); + expect(filteredRegistry().length).toBe(2); + + LogBoxData.addIgnorePatterns([/There are \d+ cats/]); + expect(filteredRegistry().length).toBe(1); + + LogBoxData.addIgnorePatterns(['cats']); + expect(filteredRegistry().length).toBe(0); + }); + + it('ignores matching regexs or pattern (exceptions)', () => { + addExceptions(['There are 4 dogs', 'There are 3 cats', 'There are H cats']); + jest.runAllImmediates(); expect(filteredRegistry().length).toBe(3); @@ -159,11 +225,10 @@ describe('LogBoxData', () => { }); it('ignores all logs when disabled', () => { - logAndFlush([ - {level: 'warn', args: ['A!']}, - {level: 'warn', args: ['B?']}, - {level: 'warn', args: ['C!']}, - ]); + addLogs(['A!', 'B?']); + addExceptions(['C!']); + jest.runAllImmediates(); + expect(registry().length).toBe(3); LogBoxData.setDisabled(true); @@ -174,57 +239,68 @@ describe('LogBoxData', () => { }); it('groups consecutive logs by format string categories', () => { - logAndFlush([{level: 'warn', args: ['%s', 'A']}]); - + addLogs([['%s', 'A']]); + jest.runAllImmediates(); expect(registry().length).toBe(1); expect(registry()[0].count).toBe(1); - logAndFlush([{level: 'warn', args: ['%s', 'B']}]); + addLogs([['%s', 'B']]); + jest.runAllImmediates(); expect(registry().length).toBe(1); expect(registry()[0].count).toBe(2); - logAndFlush([{level: 'warn', args: ['A']}]); + addLogs(['A']); + jest.runAllImmediates(); expect(registry().length).toBe(2); expect(registry()[1].count).toBe(1); - logAndFlush([{level: 'warn', args: ['B']}]); + addLogs(['B']); + jest.runAllImmediates(); expect(registry().length).toBe(3); expect(registry()[2].count).toBe(1); }); it('groups warnings with consideration for arguments', () => { - logAndFlush([{level: 'warn', args: ['A', 'B']}]); + addLogs([['A', 'B']]); + jest.runAllImmediates(); expect(registry().length).toBe(1); expect(registry()[0].count).toBe(1); - logAndFlush([{level: 'warn', args: ['A', 'B']}]); + addLogs([['A', 'B']]); + jest.runAllImmediates(); expect(registry().length).toBe(1); expect(registry()[0].count).toBe(2); - logAndFlush([{level: 'warn', args: ['A', 'C']}]); + addLogs([['A', 'C']]); + jest.runAllImmediates(); expect(registry().length).toBe(2); expect(registry()[1].count).toBe(1); - logAndFlush([{level: 'warn', args: ['%s', 'A', 'A']}]); + addLogs([['%s', 'A', 'A']]); + jest.runAllImmediates(); expect(registry().length).toBe(3); expect(registry()[2].count).toBe(1); - logAndFlush([{level: 'warn', args: ['%s', 'B', 'A']}]); + addLogs([['%s', 'B', 'A']]); + jest.runAllImmediates(); expect(registry().length).toBe(3); expect(registry()[2].count).toBe(2); - logAndFlush([{level: 'warn', args: ['%s', 'B', 'B']}]); + addLogs([['%s', 'B', 'B']]); + jest.runAllImmediates(); expect(registry().length).toBe(4); expect(registry()[3].count).toBe(1); }); it('ignores logs starting with "(ADVICE)"', () => { - logAndFlush([{level: 'warn', args: ['(ADVICE) ...']}]); + addLogs(['(ADVICE) ...']); + jest.runAllImmediates(); expect(registry().length).toBe(0); }); it('does not ignore logs formatted to start with "(ADVICE)"', () => { - logAndFlush([{level: 'warn', args: ['%s ...', '(ADVICE)']}]); + addLogs([['%s ...', '(ADVICE)']]); + jest.runAllImmediates(); expect(registry().length).toBe(1); }); @@ -243,10 +319,8 @@ describe('LogBoxData', () => { const {observer} = observe(); expect(observer.mock.calls.length).toBe(1); - logAndFlushAndUpdate([ - {level: 'warn', args: ['A']}, - {level: 'warn', args: ['B']}, - ]); + addLogs(['A']); + jest.runAllImmediates(); expect(observer.mock.calls.length).toBe(2); // We expect observers to recieve the same Set object in sequential updates @@ -270,7 +344,8 @@ describe('LogBoxData', () => { const {observer} = observe(); expect(observer.mock.calls.length).toBe(1); - logAndFlushAndUpdate([{level: 'warn', args: ['A']}]); + addLogs(['A']); + jest.runAllImmediates(); expect(observer.mock.calls.length).toBe(2); const lastLog = Array.from(observer.mock.calls[1][0])[0]; @@ -288,7 +363,8 @@ describe('LogBoxData', () => { const {observer} = observe(); expect(observer.mock.calls.length).toBe(1); - logAndFlushAndUpdate([{level: 'warn', args: ['A']}]); + addLogs(['A']); + jest.runAllImmediates(); expect(observer.mock.calls.length).toBe(2); LogBoxData.clear(); diff --git a/Libraries/LogBox/Data/__tests__/LogBoxLogParser-test.js b/Libraries/LogBox/Data/__tests__/LogBoxLogParser-test.js index fc0471847e5..94315fe4a0c 100644 --- a/Libraries/LogBox/Data/__tests__/LogBoxLogParser-test.js +++ b/Libraries/LogBox/Data/__tests__/LogBoxLogParser-test.js @@ -15,7 +15,7 @@ jest.mock('../../../Core/Devtools/parseErrorStack', () => { return {__esModule: true, default: jest.fn(() => [])}; }); -const parseLogBoxLog = require('../parseLogBoxLog').default; +const {parseLogBoxLog} = require('../parseLogBoxLog'); describe('parseLogBoxLog', () => { it('parses strings', () => { diff --git a/Libraries/LogBox/Data/parseLogBoxLog.js b/Libraries/LogBox/Data/parseLogBoxLog.js index 33dc5ba0902..b6e7e129b02 100644 --- a/Libraries/LogBox/Data/parseLogBoxLog.js +++ b/Libraries/LogBox/Data/parseLogBoxLog.js @@ -34,7 +34,7 @@ export type ComponentStack = $ReadOnlyArray< const SUBSTITUTION = UTFSequence.BOM + '%s'; -function parseCategory( +export function parseCategory( args: $ReadOnlyArray, ): $ReadOnly<{| category: Category, @@ -110,8 +110,23 @@ function parseCategory( }, }; } +export function parseComponentStack(message: string): ComponentStack { + return message + .split(/\n {4}in /g) + .map(s => { + if (!s) { + return null; + } + let [component, location] = s.split(/ \(at /); + if (!location) { + [component, location] = s.split(/ \(/); + } + return {component, location: location && location.replace(')', '')}; + }) + .filter(Boolean); +} -function parseLog( +export function parseLogBoxLog( args: $ReadOnlyArray, ): {| componentStack: ComponentStack, @@ -129,19 +144,7 @@ function parseLog( let argsWithoutComponentStack = []; for (const arg of args) { if (typeof arg === 'string' && /^\n {4}in/.exec(arg)) { - componentStack = arg - .split(/\n {4}in /g) - .map(s => { - if (!s) { - return null; - } - let [component, location] = s.split(/ \(at /); - if (!location) { - [component, location] = s.split(/ \(/); - } - return {component, location: location && location.replace(')', '')}; - }) - .filter(Boolean); + componentStack = parseComponentStack(arg); } else { argsWithoutComponentStack.push(arg); } @@ -152,5 +155,3 @@ function parseLog( componentStack, }; } - -export default parseLog; diff --git a/Libraries/LogBox/LogBox.js b/Libraries/LogBox/LogBox.js index 66f4498fab1..191be891443 100644 --- a/Libraries/LogBox/LogBox.js +++ b/Libraries/LogBox/LogBox.js @@ -60,8 +60,6 @@ if (__DEV__) { // Show LogBox for the `warning` module. if (typeof args[0] === 'string' && args[0].startsWith('Warning: ')) { registerWarning(...args); - } else { - registerError(...args); } }; @@ -133,11 +131,7 @@ if (__DEV__) { }; const registerWarning = (...args): void => { - LogBoxData.add('warn', args); - }; - - const registerError = (...args): void => { - LogBoxData.add('error', args); + LogBoxData.addLog('warn', args); }; } else { LogBoxComponent = class extends React.Component { diff --git a/Libraries/LogBox/UI/LogBoxMessage.js b/Libraries/LogBox/UI/LogBoxMessage.js index 1e84ce7667e..0651b62f789 100644 --- a/Libraries/LogBox/UI/LogBoxMessage.js +++ b/Libraries/LogBox/UI/LogBoxMessage.js @@ -21,7 +21,7 @@ type Props = { style: TextStyleProp, }; -const cleanContent = content => content.replace(/Warning: /g, ''); +const cleanContent = content => content.replace(/(Warning|Error): /g, ''); function LogBoxMessage(props: Props): React.Node { const {content, substitutions}: Message = props.message; diff --git a/Libraries/LogBox/__tests__/LogBox-test.js b/Libraries/LogBox/__tests__/LogBox-test.js index 6f4db5d3b23..1212d8da7ad 100644 --- a/Libraries/LogBox/__tests__/LogBox-test.js +++ b/Libraries/LogBox/__tests__/LogBox-test.js @@ -60,9 +60,9 @@ describe('LogBox', () => { LogBox.install(); - expect(LogBoxData.add).not.toBeCalled(); + expect(LogBoxData.addLog).not.toBeCalled(); console.warn('...'); - expect(LogBoxData.add).toBeCalled(); + expect(LogBoxData.addLog).toBeCalled(); }); it('registers errors beginning with "Warning: " as warnings', () => { @@ -71,9 +71,9 @@ describe('LogBox', () => { LogBox.install(); console.error('...'); - expect(LogBoxData.add).toBeCalledWith('error', ['...']); + expect(LogBoxData.addLog).not.toBeCalled(); console.error('Warning: ...'); - expect(LogBoxData.add).toBeCalledWith('warn', ['Warning: ...']); + expect(LogBoxData.addLog).toBeCalledWith('warn', ['Warning: ...']); }); });