From ec0c65c4b2bc535fb9bb6e6cb668858ff74188f0 Mon Sep 17 00:00:00 2001 From: Rick Hanlon Date: Wed, 1 Apr 2020 16:40:37 -0700 Subject: [PATCH] Improve component stack parsing Summary: Update the error log message parsing to fix missing component stacks in console.errors. Changelog: [Internal] Reviewed By: cpojer Differential Revision: D20801985 fbshipit-source-id: ae544200315a8c3c0310e8370bc38b0546734f38 --- .../Data/__tests__/parseLogBoxLog-test.js | 81 ++++++++++++++++++- Libraries/LogBox/Data/parseLogBoxLog.js | 59 +++++++++++--- Libraries/LogBox/LogBox.js | 8 +- 3 files changed, 134 insertions(+), 14 deletions(-) diff --git a/Libraries/LogBox/Data/__tests__/parseLogBoxLog-test.js b/Libraries/LogBox/Data/__tests__/parseLogBoxLog-test.js index 2f66d15645..30c489a344 100644 --- a/Libraries/LogBox/Data/__tests__/parseLogBoxLog-test.js +++ b/Libraries/LogBox/Data/__tests__/parseLogBoxLog-test.js @@ -143,6 +143,32 @@ describe('parseLogBoxLog', () => { }); }); + it('detects a component stack in the first argument', () => { + expect( + parseLogBoxLog([ + 'Some kind of message\n in MyComponent (at filename.js:1)\n in MyOtherComponent (at filename2.js:1)', + ]), + ).toEqual({ + componentStack: [ + { + content: 'MyComponent', + fileName: 'filename.js', + location: {column: -1, row: 1}, + }, + { + content: 'MyOtherComponent', + fileName: 'filename2.js', + location: {column: -1, row: 1}, + }, + ], + category: 'Some kind of message', + message: { + content: 'Some kind of message', + substitutions: [], + }, + }); + }); + it('detects a component stack in the second argument', () => { expect( parseLogBoxLog([ @@ -479,7 +505,7 @@ Please follow the instructions at: fburl.com/rn-remote-assets`, }); }); - it('parses a error log', () => { + it('parses an error log with `error.componentStack`', () => { const error = { id: 0, isFatal: false, @@ -532,6 +558,59 @@ Please follow the instructions at: fburl.com/rn-remote-assets`, }); }); + it('parses an error log with a component stack in the message', () => { + const error = { + id: 0, + isFatal: false, + isComponentError: false, + message: + 'Some kind of message\n in MyComponent (at filename.js:1)\n in MyOtherComponent (at filename2.js:1)', + originalMessage: + 'Some kind of message\n in MyComponent (at filename.js:1)\n in MyOtherComponent (at filename2.js:1)', + name: '', + componentStack: null, + stack: [ + { + column: 1, + file: 'foo.js', + lineNumber: 1, + methodName: 'bar', + collapse: false, + }, + ], + }; + expect(parseLogBoxException(error)).toEqual({ + level: 'error', + isComponentError: false, + stack: [ + { + collapse: false, + column: 1, + file: 'foo.js', + lineNumber: 1, + methodName: 'bar', + }, + ], + componentStack: [ + { + content: 'MyComponent', + fileName: 'filename.js', + location: {column: -1, row: 1}, + }, + { + content: 'MyOtherComponent', + fileName: 'filename2.js', + location: {column: -1, row: 1}, + }, + ], + category: 'Some kind of message', + message: { + content: 'Some kind of message', + substitutions: [], + }, + }); + }); + it('parses a fatal exception', () => { const error = { id: 0, diff --git a/Libraries/LogBox/Data/parseLogBoxLog.js b/Libraries/LogBox/Data/parseLogBoxLog.js index cdc72a6f79..0d7c83d375 100644 --- a/Libraries/LogBox/Data/parseLogBoxLog.js +++ b/Libraries/LogBox/Data/parseLogBoxLog.js @@ -239,21 +239,50 @@ export function parseLogBoxException( }; } - const level = message.match(/^TransformError /) - ? 'syntax' - : error.isFatal || error.isComponentError - ? 'fatal' - : 'error'; + if (message.match(/^TransformError /)) { + return { + level: 'syntax', + stack: error.stack, + isComponentError: error.isComponentError, + componentStack: [], + message: { + content: message, + substitutions: [], + }, + category: message, + }; + } + const componentStack = error.componentStack; + if (error.isFatal || error.isComponentError) { + return { + level: 'fatal', + stack: error.stack, + isComponentError: error.isComponentError, + componentStack: + componentStack != null ? parseComponentStack(componentStack) : [], + ...parseInterpolation([message]), + }; + } + + if (componentStack != null) { + // It is possible that console errors have a componentStack. + return { + level: 'error', + stack: error.stack, + isComponentError: error.isComponentError, + componentStack: parseComponentStack(componentStack), + ...parseInterpolation([message]), + }; + } + + // Most `console.error` calls won't have a componentStack. We parse them like + // regular logs which have the component stack burried in the message. return { - level: level, + level: 'error', stack: error.stack, isComponentError: error.isComponentError, - componentStack: - error.componentStack != null - ? parseComponentStack(error.componentStack) - : [], - ...parseInterpolation([message]), + ...parseLogBoxLog([message]), }; } @@ -286,7 +315,13 @@ export function parseLogBoxLog( if (componentStack.length === 0) { // Try finding the component stack elsewhere. for (const arg of args) { - if (typeof arg === 'string' && /^\n {4}in/.exec(arg)) { + if (typeof arg === 'string' && /\n {4}in /.exec(arg)) { + // Strip out any messages before the component stack. + const messageEndIndex = arg.indexOf('\n in '); + if (messageEndIndex > 0) { + argsWithoutComponentStack.push(arg.slice(0, messageEndIndex)); + } + componentStack = parseComponentStack(arg); } else { argsWithoutComponentStack.push(arg); diff --git a/Libraries/LogBox/LogBox.js b/Libraries/LogBox/LogBox.js index eb00b87930..92f5133638 100644 --- a/Libraries/LogBox/LogBox.js +++ b/Libraries/LogBox/LogBox.js @@ -138,7 +138,13 @@ if (__DEV__) { try { if (!isWarningModuleWarning(...args)) { - // Only show LogBox for the `warning` module, otherwise pass through and skip. + // Only show LogBox for the 'warning' module, otherwise pass through. + // By passing through, this will get picked up by the React console override, + // potentially adding the component stack. React then passes it back to the + // React Native ExceptionsManager, which reports it to LogBox as an error. + // + // The 'warning' module needs to be handled here because React internally calls + // `console.error('Warning: ')` with the component stack already included. error.call(console, ...args); return; }