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

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,

Libraries/LogBox/Data/parseLogBoxLog.js

@@ -239,21 +239,50 @@ export function parseLogBoxException(
     };
   }
 
-  const level = message.match(/^TransformError /)
+  if (message.match(/^TransformError /)) {
-    ? 'syntax'
+    return {
-    : error.isFatal || error.isComponentError
+      level: 'syntax',
-    ? 'fatal'
+      stack: error.stack,
-    : 'error';
+      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:
+    ...parseLogBoxLog([message]),
-      error.componentStack != null
-        ? parseComponentStack(error.componentStack)
-        : [],
-    ...parseInterpolation([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);

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;
       }