From 8335ebaeab53fa421745fceeaf3cab3f7b3ce9bc Mon Sep 17 00:00:00 2001 From: Rick Hanlon Date: Wed, 13 Nov 2019 11:30:43 -0800 Subject: [PATCH] Add support for isRenderError Summary: This diff adds `isRenderError` to the Log data, and refactors the LogBoxLog object to accept an object in the constructor instead of adding the 7th argument. No visual updates as those are in the next diff. Changelog: [Internal] Reviewed By: cpojer Differential Revision: D18466192 fbshipit-source-id: e38ef9032b8074abbc7b40cbe7a84d45285944c4 --- Libraries/LogBox/Data/LogBoxData.js | 33 +---- Libraries/LogBox/Data/LogBoxLog.js | 33 +++-- .../LogBox/Data/__tests__/LogBoxLog-test.js | 17 ++- .../Data/__tests__/parseLogBoxLog-test.js | 48 +++++- Libraries/LogBox/Data/parseLogBoxLog.js | 17 +-- .../UI/__tests__/LogBoxContainer-test.js | 137 ++++++++++-------- .../UI/__tests__/LogBoxInspector-test.js | 45 +++--- .../LogBoxInspectorReactFrames-test.js | 30 ++-- .../LogBoxInspectorStackFrames-test.js | 30 ++-- .../__tests__/LogBoxLogNotification-test.js | 15 +- .../LogBoxContainer-test.js.snap | 6 + .../LogBoxInspector-test.js.snap | 2 + 12 files changed, 234 insertions(+), 179 deletions(-) diff --git a/Libraries/LogBox/Data/LogBoxData.js b/Libraries/LogBox/Data/LogBoxData.js index 11ddce922a8..c58330f18f7 100644 --- a/Libraries/LogBox/Data/LogBoxData.js +++ b/Libraries/LogBox/Data/LogBoxData.js @@ -203,13 +203,14 @@ export function addLog(log: LogData): void { const stack = parseErrorStack(errorForStackTrace); appendNewLog( - new LogBoxLog( - log.level, - log.message, + new LogBoxLog({ + level: log.level, + message: log.message, + isComponentError: false, stack, - log.category, - log.componentStack, - ), + category: log.category, + componentStack: log.componentStack, + }), ); } catch (error) { reportLogBoxError(error); @@ -222,25 +223,7 @@ export function addException(error: ExtendedExceptionData): void { // otherwise spammy logs would pause rendering. setImmediate(() => { try { - const { - category, - message, - codeFrame, - componentStack, - stack, - level, - } = parseLogBoxException(error); - - appendNewLog( - new LogBoxLog( - level, - message, - stack, - category, - componentStack != null ? componentStack : [], - codeFrame, - ), - ); + appendNewLog(new LogBoxLog(parseLogBoxException(error))); } catch (loggingError) { reportLogBoxError(loggingError); } diff --git a/Libraries/LogBox/Data/LogBoxLog.js b/Libraries/LogBox/Data/LogBoxLog.js index 271d6ce20eb..465720a54ec 100644 --- a/Libraries/LogBox/Data/LogBoxLog.js +++ b/Libraries/LogBox/Data/LogBoxLog.js @@ -24,6 +24,16 @@ type SymbolicationStatus = 'NONE' | 'PENDING' | 'COMPLETE' | 'FAILED'; export type LogLevel = 'warn' | 'error' | 'fatal' | 'syntax'; +export type LogBoxLogData = $ReadOnly<{| + level: LogLevel, + message: Message, + stack: Stack, + category: string, + componentStack: ComponentStack, + codeFrame?: ?CodeFrame, + isComponentError: boolean, +|}>; + export type SymbolicationRequest = $ReadOnly<{| abort: () => void, |}>; @@ -36,6 +46,7 @@ class LogBoxLog { count: number; level: LogLevel; codeFrame: ?CodeFrame; + isComponentError: boolean; symbolicated: | $ReadOnly<{|error: null, stack: null, status: 'NONE'|}> | $ReadOnly<{|error: null, stack: null, status: 'PENDING'|}> @@ -46,20 +57,14 @@ class LogBoxLog { status: 'NONE', }; - constructor( - level: LogLevel, - message: Message, - stack: Stack, - category: string, - componentStack: ComponentStack, - codeFrame?: ?CodeFrame, - ) { - this.level = level; - this.message = message; - this.stack = stack; - this.category = category; - this.componentStack = componentStack; - this.codeFrame = codeFrame; + constructor(data: LogBoxLogData) { + this.level = data.level; + this.message = data.message; + this.stack = data.stack; + this.category = data.category; + this.componentStack = data.componentStack; + this.codeFrame = data.codeFrame; + this.isComponentError = data.isComponentError; this.count = 1; } diff --git a/Libraries/LogBox/Data/__tests__/LogBoxLog-test.js b/Libraries/LogBox/Data/__tests__/LogBoxLog-test.js index a17565d24ae..6f6e2048fd5 100644 --- a/Libraries/LogBox/Data/__tests__/LogBoxLog-test.js +++ b/Libraries/LogBox/Data/__tests__/LogBoxLog-test.js @@ -19,18 +19,19 @@ jest.mock('../LogBoxSymbolication', () => { }); function getLogBoxLog() { - return new (require('../LogBoxLog')).default( - 'warn', - {content: '...', substitutions: []}, - createStack(['A', 'B', 'C']), - 'Message category...', - [{component: 'LogBoxLog', location: 'LogBoxLog.js:1'}], - { + return new (require('../LogBoxLog')).default({ + level: 'warn', + isComponentError: false, + message: {content: '...', substitutions: []}, + stack: createStack(['A', 'B', 'C']), + category: 'Message category...', + componentStack: [{component: 'LogBoxLog', location: 'LogBoxLog.js:1'}], + codeFrame: { fileName: '/path/to/RKJSModules/Apps/CrashReact/CrashReactApp.js', location: {row: 199, column: 0}, content: '', }, - ); + }); } function getLogBoxSymbolication(): {| diff --git a/Libraries/LogBox/Data/__tests__/parseLogBoxLog-test.js b/Libraries/LogBox/Data/__tests__/parseLogBoxLog-test.js index 8fed42107cf..b4d311e8175 100644 --- a/Libraries/LogBox/Data/__tests__/parseLogBoxLog-test.js +++ b/Libraries/LogBox/Data/__tests__/parseLogBoxLog-test.js @@ -167,15 +167,16 @@ describe('parseLogBoxLog', () => { | ^ 200 |`, name: '', + isComponentError: false, componentStack: '', stack: [], id: 0, isFatal: true, - isComponentError: false, }; expect(parseLogBoxException(error)).toEqual({ level: 'syntax', + isComponentError: false, codeFrame: { fileName: '/path/to/RKJSModules/Apps/CrashReact/CrashReactApp.js', location: {row: 199, column: 0}, @@ -190,6 +191,7 @@ describe('parseLogBoxLog', () => { substitutions: [], }, stack: [], + componentStack: [], category: '/path/to/RKJSModules/Apps/CrashReact/CrashReactApp.js-199-0', }); }); @@ -218,6 +220,7 @@ describe('parseLogBoxLog', () => { expect(parseLogBoxException(error)).toEqual({ level: 'error', category: '### Error', + isComponentError: false, message: { content: '### Error', substitutions: [], @@ -247,7 +250,48 @@ describe('parseLogBoxLog', () => { it('parses a fatal exception', () => { const error = { id: 0, + isFatal: true, isComponentError: false, + message: '### Fatal', + originalMessage: '### Fatal', + componentStack: null, + name: '', + stack: [ + { + column: 1, + file: 'foo.js', + lineNumber: 1, + methodName: 'bar', + collapse: false, + }, + ], + }; + + expect(parseLogBoxException(error)).toEqual({ + level: 'fatal', + category: '### Fatal', + isComponentError: false, + message: { + content: '### Fatal', + substitutions: [], + }, + componentStack: [], + stack: [ + { + column: 1, + file: 'foo.js', + lineNumber: 1, + methodName: 'bar', + collapse: false, + }, + ], + }); + }); + + it('parses a render error', () => { + const error = { + id: 0, + isComponentError: true, isFatal: true, message: '### Fatal', originalMessage: '### Fatal', @@ -267,6 +311,7 @@ describe('parseLogBoxLog', () => { expect(parseLogBoxException(error)).toEqual({ level: 'fatal', category: '### Fatal', + isComponentError: true, message: { content: '### Fatal', substitutions: [], @@ -316,6 +361,7 @@ describe('parseLogBoxLog', () => { "TransformError SyntaxError: /path/to/RKJSModules/Apps/CrashReact/CrashReactApp.js: 'import' and 'export' may only appear at the top level (199:0)", substitutions: [], }, + isComponentError: false, componentStack: [], stack: [ { diff --git a/Libraries/LogBox/Data/parseLogBoxLog.js b/Libraries/LogBox/Data/parseLogBoxLog.js index 2481b9efe1e..9ddd67eaa31 100644 --- a/Libraries/LogBox/Data/parseLogBoxLog.js +++ b/Libraries/LogBox/Data/parseLogBoxLog.js @@ -12,9 +12,8 @@ import UTFSequence from '../../UTFSequence'; import stringifySafe from '../../Utilities/stringifySafe'; -import type {LogLevel} from './LogBoxLog'; import type {ExceptionData} from '../../Core/NativeExceptionsManager'; -import type {Stack} from './LogBoxSymbolication'; +import type {LogBoxLogData} from './LogBoxLog'; export type ExtendedExceptionData = ExceptionData & {isComponentError: boolean}; export type Category = string; @@ -139,14 +138,7 @@ export function parseComponentStack(message: string): ComponentStack { export function parseLogBoxException( error: ExtendedExceptionData, -): {| - level: LogLevel, - category: Category, - message: Message, - codeFrame?: CodeFrame, - stack: Stack, - componentStack?: ComponentStack, -|} { +): LogBoxLogData { const message = error.originalMessage != null ? error.originalMessage : 'Unknown'; const match = message.match( @@ -155,8 +147,9 @@ export function parseLogBoxException( if (!match) { return { - level: error.isFatal ? 'fatal' : 'error', + level: error.isFatal || error.isComponentError ? 'fatal' : 'error', stack: error.stack, + isComponentError: error.isComponentError, componentStack: error.componentStack != null ? parseComponentStack(error.componentStack) @@ -169,6 +162,8 @@ export function parseLogBoxException( return { level: 'syntax', stack: [], + isComponentError: false, + componentStack: [], codeFrame: { fileName, location: { diff --git a/Libraries/LogBox/UI/__tests__/LogBoxContainer-test.js b/Libraries/LogBox/UI/__tests__/LogBoxContainer-test.js index c26713a4d77..982c3bfdc88 100644 --- a/Libraries/LogBox/UI/__tests__/LogBoxContainer-test.js +++ b/Libraries/LogBox/UI/__tests__/LogBoxContainer-test.js @@ -43,16 +43,17 @@ describe('LogBoxContainer', () => { selectedLogIndex={-1} logs={ new Set([ - new LogBoxLog( - 'warn', - { + new LogBoxLog({ + level: 'warn', + isComponentError: false, + message: { content: 'Some kind of message', substitutions: [], }, - [], - 'Some kind of message', - [], - ), + stack: [], + category: 'Some kind of message', + componentStack: [], + }), ]) } />, @@ -71,26 +72,28 @@ describe('LogBoxContainer', () => { selectedLogIndex={-1} logs={ new Set([ - new LogBoxLog( - 'warn', - { + new LogBoxLog({ + level: 'warn', + isComponentError: false, + message: { content: 'Some kind of message', substitutions: [], }, - [], - 'Some kind of message', - [], - ), - new LogBoxLog( - 'warn', - { + stack: [], + category: 'Some kind of message', + componentStack: [], + }), + new LogBoxLog({ + level: 'warn', + isComponentError: false, + message: { content: 'Some kind of message (latest)', substitutions: [], }, - [], - 'Some kind of message (latest)', - [], - ), + stack: [], + category: 'Some kind of message (latest)', + componentStack: [], + }), ]) } />, @@ -109,26 +112,28 @@ describe('LogBoxContainer', () => { selectedLogIndex={-1} logs={ new Set([ - new LogBoxLog( - 'error', - { + new LogBoxLog({ + level: 'error', + isComponentError: false, + message: { content: 'Some kind of message', substitutions: [], }, - [], - 'Some kind of message', - [], - ), - new LogBoxLog( - 'error', - { + stack: [], + category: 'Some kind of message', + componentStack: [], + }), + new LogBoxLog({ + level: 'error', + isComponentError: false, + message: { content: 'Some kind of message (latest)', substitutions: [], }, - [], - 'Some kind of message (latest)', - [], - ), + stack: [], + category: 'Some kind of message (latest)', + componentStack: [], + }), ]) } />, @@ -147,26 +152,28 @@ describe('LogBoxContainer', () => { selectedLogIndex={-1} logs={ new Set([ - new LogBoxLog( - 'warn', - { + new LogBoxLog({ + level: 'warn', + isComponentError: false, + message: { content: 'Some kind of message', substitutions: [], }, - [], - 'Some kind of message', - [], - ), - new LogBoxLog( - 'error', - { + stack: [], + category: 'Some kind of message', + componentStack: [], + }), + new LogBoxLog({ + level: 'error', + isComponentError: false, + message: { content: 'Some kind of message (latest)', substitutions: [], }, - [], - 'Some kind of message (latest)', - [], - ), + stack: [], + category: 'Some kind of message (latest)', + componentStack: [], + }), ]) } />, @@ -186,16 +193,17 @@ describe('LogBoxContainer', () => { selectedLogIndex={0} logs={ new Set([ - new LogBoxLog( - 'fatal', - { + new LogBoxLog({ + level: 'fatal', + isComponentError: false, + message: { content: 'Should be selected', substitutions: [], }, - [], - 'Some kind of message', - [], - ), + stack: [], + category: 'Some kind of message', + componentStack: [], + }), ]) } />, @@ -215,16 +223,17 @@ describe('LogBoxContainer', () => { selectedLogIndex={0} logs={ new Set([ - new LogBoxLog( - 'syntax', - { + new LogBoxLog({ + level: 'syntax', + isComponentError: false, + message: { content: 'Should be selected', substitutions: [], }, - [], - 'Some kind of syntax error message', - [], - { + stack: [], + category: 'Some kind of syntax error message', + componentStack: [], + codeFrame: { fileName: '/path/to/RKJSModules/Apps/CrashReact/CrashReactApp.js', location: {row: 199, column: 0}, @@ -234,7 +243,7 @@ describe('LogBoxContainer', () => { | ^ 200 |`, }, - ), + }), ]) } />, diff --git a/Libraries/LogBox/UI/__tests__/LogBoxInspector-test.js b/Libraries/LogBox/UI/__tests__/LogBoxInspector-test.js index d43e07f99d5..229dd409c47 100644 --- a/Libraries/LogBox/UI/__tests__/LogBoxInspector-test.js +++ b/Libraries/LogBox/UI/__tests__/LogBoxInspector-test.js @@ -17,36 +17,39 @@ const LogBoxLog = require('../../Data/LogBoxLog').default; const render = require('../../../../jest/renderer'); const logs = [ - new LogBoxLog( - 'warn', - { + new LogBoxLog({ + level: 'warn', + isComponentError: false, + message: { content: 'Some kind of message (first)', substitutions: [], }, - [], - 'Some kind of message (first)', - [], - ), - new LogBoxLog( - 'error', - { + stack: [], + category: 'Some kind of message (first)', + componentStack: [], + }), + new LogBoxLog({ + level: 'error', + isComponentError: false, + message: { content: 'Some kind of message (second)', substitutions: [], }, - [], - 'Some kind of message (second)', - [], - ), - new LogBoxLog( - 'fatal', - { + stack: [], + category: 'Some kind of message (second)', + componentStack: [], + }), + new LogBoxLog({ + level: 'fatal', + isComponentError: false, + message: { content: 'Some kind of message (third)', substitutions: [], }, - [], - 'Some kind of message (third)', - [], - ), + stack: [], + category: 'Some kind of message (third)', + componentStack: [], + }), ]; describe('LogBoxContainer', () => { diff --git a/Libraries/LogBox/UI/__tests__/LogBoxInspectorReactFrames-test.js b/Libraries/LogBox/UI/__tests__/LogBoxInspectorReactFrames-test.js index d79d2ae2b7c..add2b9593a4 100644 --- a/Libraries/LogBox/UI/__tests__/LogBoxInspectorReactFrames-test.js +++ b/Libraries/LogBox/UI/__tests__/LogBoxInspectorReactFrames-test.js @@ -22,16 +22,17 @@ describe('LogBoxInspectorReactFrames', () => { const output = render.shallowRender( , ); @@ -43,21 +44,22 @@ describe('LogBoxInspectorReactFrames', () => { const output = render.shallowRender( , ); diff --git a/Libraries/LogBox/UI/__tests__/LogBoxInspectorStackFrames-test.js b/Libraries/LogBox/UI/__tests__/LogBoxInspectorStackFrames-test.js index 60d80c40889..472c0b048bb 100644 --- a/Libraries/LogBox/UI/__tests__/LogBoxInspectorStackFrames-test.js +++ b/Libraries/LogBox/UI/__tests__/LogBoxInspectorStackFrames-test.js @@ -17,13 +17,14 @@ const LogBoxInspectorStackFrames = require('../LogBoxInspectorStackFrames') const LogBoxLog = require('../../Data/LogBoxLog').default; const render = require('../../../../jest/renderer'); -const log = new LogBoxLog( - 'warn', - { +const log = new LogBoxLog({ + level: 'warn', + isComponentError: false, + message: { content: 'Some kind of message (latest)', substitutions: [], }, - [ + stack: [ { column: 1, file: 'dependency.js', @@ -39,20 +40,21 @@ const log = new LogBoxLog( collapse: false, }, ], - 'Some kind of message (latest)', - [], -); + category: 'Some kind of message (latest)', + componentStack: [], +}); -const logNoStackFrames = new LogBoxLog( - 'warn', - { +const logNoStackFrames = new LogBoxLog({ + level: 'warn', + isComponentError: false, + message: { content: 'Some kind of message (latest)', substitutions: [], }, - [], - 'Some kind of message (latest)', - [], -); + stack: [], + category: 'Some kind of message (latest)', + componentStack: [], +}); describe('LogBoxInspectorStackFrame', () => { it('should render stack frames with 1 frame collapsed', () => { diff --git a/Libraries/LogBox/UI/__tests__/LogBoxLogNotification-test.js b/Libraries/LogBox/UI/__tests__/LogBoxLogNotification-test.js index 8adee4f11c1..52678497656 100644 --- a/Libraries/LogBox/UI/__tests__/LogBoxLogNotification-test.js +++ b/Libraries/LogBox/UI/__tests__/LogBoxLogNotification-test.js @@ -16,16 +16,17 @@ const LogBoxLogNotification = require('../LogBoxLogNotification').default; const LogBoxLog = require('../../Data/LogBoxLog').default; const render = require('../../../../jest/renderer'); -const log = new LogBoxLog( - 'warn', - { +const log = new LogBoxLog({ + level: 'warn', + isComponentError: false, + message: { content: 'Some kind of message', substitutions: [], }, - [], - 'Some kind of message', - [], -); + stack: [], + category: 'Some kind of message', + componentStack: [], +}); describe('LogBoxLogNotification', () => { it('should render log', () => { diff --git a/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxContainer-test.js.snap b/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxContainer-test.js.snap index 76aa260360f..96fc0325a68 100644 --- a/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxContainer-test.js.snap +++ b/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxContainer-test.js.snap @@ -28,6 +28,7 @@ exports[`LogBoxContainer should render both an error and warning notification 1` "codeFrame": undefined, "componentStack": Array [], "count": 1, + "isComponentError": false, "level": "warn", "message": Object { "content": "Some kind of message", @@ -63,6 +64,7 @@ exports[`LogBoxContainer should render both an error and warning notification 1` "codeFrame": undefined, "componentStack": Array [], "count": 1, + "isComponentError": false, "level": "error", "message": Object { "content": "Some kind of message (latest)", @@ -115,6 +117,7 @@ exports[`LogBoxContainer should render selected fatal error even when disabled 1 "codeFrame": undefined, "componentStack": Array [], "count": 1, + "isComponentError": false, "level": "fatal", "message": Object { "content": "Should be selected", @@ -168,6 +171,7 @@ exports[`LogBoxContainer should render selected syntax error even when disabled }, "componentStack": Array [], "count": 1, + "isComponentError": false, "level": "syntax", "message": Object { "content": "Should be selected", @@ -218,6 +222,7 @@ exports[`LogBoxContainer should render the latest error notification 1`] = ` "codeFrame": undefined, "componentStack": Array [], "count": 1, + "isComponentError": false, "level": "error", "message": Object { "content": "Some kind of message (latest)", @@ -274,6 +279,7 @@ exports[`LogBoxContainer should render the latest warning notification 1`] = ` "codeFrame": undefined, "componentStack": Array [], "count": 1, + "isComponentError": false, "level": "warn", "message": Object { "content": "Some kind of message (latest)", diff --git a/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxInspector-test.js.snap b/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxInspector-test.js.snap index 863d7529d2d..88506088c21 100644 --- a/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxInspector-test.js.snap +++ b/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxInspector-test.js.snap @@ -23,6 +23,7 @@ exports[`LogBoxContainer should render fatal with selectedIndex 2 1`] = ` "codeFrame": undefined, "componentStack": Array [], "count": 1, + "isComponentError": false, "level": "fatal", "message": Object { "content": "Some kind of message (third)", @@ -71,6 +72,7 @@ exports[`LogBoxContainer should render warning with selectedIndex 0 1`] = ` "codeFrame": undefined, "componentStack": Array [], "count": 1, + "isComponentError": false, "level": "warn", "message": Object { "content": "Some kind of message (first)",