Add log level to support errors

Summary:
This diff adds a level to LogBox logs so that we can store and display errors in later diffs

Changelog: [Internal]

Reviewed By: cpojer

Differential Revision: D18091101

fbshipit-source-id: 21661d28a7945bdcb56702e2a03ab3612c11fe35

Libraries/LogBox/Data/LogBoxData.js

@@ -12,6 +12,7 @@
 
 import LogBoxLog from './LogBoxLog';
 import parseLogBoxLog from './parseLogBoxLog';
+import type {LogLevel} from './LogBoxLog';
 
 export type LogBoxLogs = Set<LogBoxLog>;
 
@@ -52,7 +53,7 @@ function handleUpdate(): void {
   }
 }
 
-export function add(args: $ReadOnlyArray<mixed>): void {
+export function add(level: LogLevel, args: $ReadOnlyArray<mixed>): void {
   // This is carried over from the old YellowBox, but it is not clear why.
   if (typeof args[0] === 'string' && args[0].startsWith('(ADVICE)')) {
     return;
@@ -75,7 +76,7 @@ export function add(args: $ReadOnlyArray<mixed>): void {
   if (lastLog && lastLog.category === category) {
     lastLog.incrementCount();
   } else {
-    logs.add(new LogBoxLog(message, stack, category, componentStack));
+    logs.add(new LogBoxLog(level, message, stack, category, componentStack));
   }
 
   handleUpdate();

Libraries/LogBox/Data/LogBoxLog.js

@@ -15,6 +15,8 @@ import * as LogBoxSymbolication from './LogBoxSymbolication';
 import type {Category, Message, ComponentStack} from './parseLogBoxLog';
 import type {Stack} from './LogBoxSymbolication';
 
+export type LogLevel = 'warn' | 'error';
+
 export type SymbolicationRequest = $ReadOnly<{|
   abort: () => void,
 |}>;
@@ -25,6 +27,7 @@ class LogBoxLog {
   componentStack: ComponentStack;
   stack: Stack;
   count: number;
+  level: LogLevel;
   symbolicated:
     | $ReadOnly<{|error: null, stack: null, status: 'NONE'|}>
     | $ReadOnly<{|error: null, stack: null, status: 'PENDING'|}>
@@ -36,11 +39,13 @@ class LogBoxLog {
   };
 
   constructor(
+    level: LogLevel,
     message: Message,
     stack: Stack,
     category: string,
     componentStack: ComponentStack,
   ) {
+    this.level = level;
     this.message = message;
     this.stack = stack;
     this.category = category;

Libraries/LogBox/Data/__tests__/LogBoxData-test.js

@@ -42,7 +42,7 @@ describe('LogBoxData', () => {
   });
 
   it('adds and dismisses logs', () => {
-    LogBoxData.add(['A']);
+    LogBoxData.add('warn', ['A']);
 
     expect(registry().length).toBe(1);
     expect(registry()[0]).toBeDefined();
@@ -53,9 +53,9 @@ describe('LogBoxData', () => {
   });
 
   it('clears all logs', () => {
-    LogBoxData.add(['A']);
+    LogBoxData.add('warn', ['A']);
-    LogBoxData.add(['B']);
+    LogBoxData.add('warn', ['B']);
-    LogBoxData.add(['C']);
+    LogBoxData.add('warn', ['C']);
 
     expect(registry().length).toBe(3);
 
@@ -64,9 +64,9 @@ describe('LogBoxData', () => {
   });
 
   it('keeps logs in chronological order', () => {
-    LogBoxData.add(['A']);
+    LogBoxData.add('warn', ['A']);
-    LogBoxData.add(['B']);
+    LogBoxData.add('warn', ['B']);
-    LogBoxData.add(['C']);
+    LogBoxData.add('warn', ['C']);
 
     let logs = registry();
     expect(logs.length).toBe(3);
@@ -74,7 +74,7 @@ describe('LogBoxData', () => {
     expect(logs[1].category).toEqual('B');
     expect(logs[2].category).toEqual('C');
 
-    LogBoxData.add(['A']);
+    LogBoxData.add('warn', ['A']);
 
     // Expect `A` to be added to the end of the registry.
     logs = registry();
@@ -86,8 +86,8 @@ describe('LogBoxData', () => {
   });
 
   it('increments the count of previous log with matching category', () => {
-    LogBoxData.add(['A']);
+    LogBoxData.add('warn', ['A']);
-    LogBoxData.add(['B']);
+    LogBoxData.add('warn', ['B']);
 
     let logs = registry();
     expect(logs.length).toBe(2);
@@ -96,7 +96,7 @@ describe('LogBoxData', () => {
     expect(logs[1].category).toEqual('B');
     expect(logs[1].count).toBe(1);
 
-    LogBoxData.add(['B']);
+    LogBoxData.add('warn', ['B']);
 
     // Expect `B` to be rolled into the last log.
     logs = registry();
@@ -108,9 +108,9 @@ describe('LogBoxData', () => {
   });
 
   it('ignores logs matching patterns', () => {
-    LogBoxData.add(['A!']);
+    LogBoxData.add('warn', ['A!']);
-    LogBoxData.add(['B?']);
+    LogBoxData.add('warn', ['B?']);
-    LogBoxData.add(['C!']);
+    LogBoxData.add('warn', ['C!']);
     expect(filteredRegistry().length).toBe(3);
 
     LogBoxData.addIgnorePatterns(['!']);
@@ -121,9 +121,9 @@ describe('LogBoxData', () => {
   });
 
   it('ignores logs matching regexs or pattern', () => {
-    LogBoxData.add(['There are 4 dogs']);
+    LogBoxData.add('warn', ['There are 4 dogs']);
-    LogBoxData.add(['There are 3 cats']);
+    LogBoxData.add('warn', ['There are 3 cats']);
-    LogBoxData.add(['There are H cats']);
+    LogBoxData.add('warn', ['There are H cats']);
     expect(filteredRegistry().length).toBe(3);
 
     LogBoxData.addIgnorePatterns(['dogs']);
@@ -137,9 +137,9 @@ describe('LogBoxData', () => {
   });
 
   it('ignores all logs when disabled', () => {
-    LogBoxData.add(['A!']);
+    LogBoxData.add('warn', ['A!']);
-    LogBoxData.add(['B?']);
+    LogBoxData.add('warn', ['B?']);
-    LogBoxData.add(['C!']);
+    LogBoxData.add('warn', ['C!']);
     expect(registry().length).toBe(3);
 
     LogBoxData.setDisabled(true);
@@ -150,56 +150,56 @@ describe('LogBoxData', () => {
   });
 
   it('groups consecutive logs by format string categories', () => {
-    LogBoxData.add(['%s', 'A']);
+    LogBoxData.add('warn', ['%s', 'A']);
     expect(registry().length).toBe(1);
     expect(registry()[0].count).toBe(1);
 
-    LogBoxData.add(['%s', 'B']);
+    LogBoxData.add('warn', ['%s', 'B']);
     expect(registry().length).toBe(1);
     expect(registry()[0].count).toBe(2);
 
-    LogBoxData.add(['A']);
+    LogBoxData.add('warn', ['A']);
     expect(registry().length).toBe(2);
     expect(registry()[1].count).toBe(1);
 
-    LogBoxData.add(['B']);
+    LogBoxData.add('warn', ['B']);
     expect(registry().length).toBe(3);
     expect(registry()[2].count).toBe(1);
   });
 
   it('groups warnings with consideration for arguments', () => {
-    LogBoxData.add(['A', 'B']);
+    LogBoxData.add('warn', ['A', 'B']);
     expect(registry().length).toBe(1);
     expect(registry()[0].count).toBe(1);
 
-    LogBoxData.add(['A', 'B']);
+    LogBoxData.add('warn', ['A', 'B']);
     expect(registry().length).toBe(1);
     expect(registry()[0].count).toBe(2);
 
-    LogBoxData.add(['A', 'C']);
+    LogBoxData.add('warn', ['A', 'C']);
     expect(registry().length).toBe(2);
     expect(registry()[1].count).toBe(1);
 
-    LogBoxData.add(['%s', 'A', 'A']);
+    LogBoxData.add('warn', ['%s', 'A', 'A']);
     expect(registry().length).toBe(3);
     expect(registry()[2].count).toBe(1);
 
-    LogBoxData.add(['%s', 'B', 'A']);
+    LogBoxData.add('warn', ['%s', 'B', 'A']);
     expect(registry().length).toBe(3);
     expect(registry()[2].count).toBe(2);
 
-    LogBoxData.add(['%s', 'B', 'B']);
+    LogBoxData.add('warn', ['%s', 'B', 'B']);
     expect(registry().length).toBe(4);
     expect(registry()[3].count).toBe(1);
   });
 
   it('ignores logs starting with "(ADVICE)"', () => {
-    LogBoxData.add(['(ADVICE) ...']);
+    LogBoxData.add('warn', ['(ADVICE) ...']);
     expect(registry().length).toBe(0);
   });
 
   it('does not ignore logs formatted to start with "(ADVICE)"', () => {
-    LogBoxData.add(['%s ...', '(ADVICE)']);
+    LogBoxData.add('warn', ['%s ...', '(ADVICE)']);
     expect(registry().length).toBe(1);
   });
 
@@ -218,8 +218,8 @@ describe('LogBoxData', () => {
     const {observer} = observe();
     expect(observer.mock.calls.length).toBe(1);
 
-    LogBoxData.add(['A']);
+    LogBoxData.add('warn', ['A']);
-    LogBoxData.add(['B']);
+    LogBoxData.add('warn', ['B']);
     jest.runAllImmediates();
     expect(observer.mock.calls.length).toBe(2);
 
@@ -244,7 +244,7 @@ describe('LogBoxData', () => {
     const {observer} = observe();
     expect(observer.mock.calls.length).toBe(1);
 
-    LogBoxData.add(['A']);
+    LogBoxData.add('warn', ['A']);
     jest.runAllImmediates();
     expect(observer.mock.calls.length).toBe(2);
 
@@ -263,7 +263,7 @@ describe('LogBoxData', () => {
     const {observer} = observe();
     expect(observer.mock.calls.length).toBe(1);
 
-    LogBoxData.add(['A']);
+    LogBoxData.add('warn', ['A']);
     jest.runAllImmediates();
     expect(observer.mock.calls.length).toBe(2);
 

Libraries/LogBox/Data/__tests__/LogBoxLog-test.js

@@ -19,6 +19,7 @@ jest.mock('../LogBoxSymbolication', () => {
 
 function getLogBoxLog() {
   return new (require('../LogBoxLog')).default(
+    'warn',
     {content: '...', substitutions: []},
     createStack(['A', 'B', 'C']),
     'Message category...',

Libraries/LogBox/LogBox.js

@@ -59,13 +59,15 @@ if (__DEV__) {
         error.call(console, ...args);
         // Show LogBox for the `warning` module.
         if (typeof args[0] === 'string' && args[0].startsWith('Warning: ')) {
-          registerLog(...args);
+          registerWarning(...args);
+        } else {
+          registerError(...args);
         }
       };
 
       warnImpl = function(...args) {
         warn.call(console, ...args);
-        registerLog(...args);
+        registerWarning(...args);
       };
 
       if ((console: any).disableLogBox === true) {
@@ -82,7 +84,7 @@ if (__DEV__) {
       }
 
       RCTLog.setWarningHandler((...args) => {
-        registerLog(...args);
+        registerWarning(...args);
       });
     }
 
@@ -130,8 +132,12 @@ if (__DEV__) {
     }
   };
 
-  const registerLog = (...args): void => {
+  const registerWarning = (...args): void => {
-    LogBoxData.add(args);
+    LogBoxData.add('warn', args);
+  };
+
+  const registerError = (...args): void => {
+    LogBoxData.add('error', args);
   };
 } else {
   LogBoxComponent = class extends React.Component<Props, State> {

Libraries/LogBox/UI/__tests__/LogBoxContainer-test.js

@@ -37,6 +37,7 @@ describe('LogBoxContainer', () => {
         logs={
           new Set([
             new LogBoxLog(
+              'warn',
               {
                 content: 'Some kind of message',
                 substitutions: [],
@@ -46,6 +47,7 @@ describe('LogBoxContainer', () => {
               [],
             ),
             new LogBoxLog(
+              'warn',
               {
                 content: 'Some kind of message (latest)',
                 substitutions: [],

Libraries/LogBox/UI/__tests__/LogBoxInspector-test.js

@@ -18,6 +18,7 @@ const render = require('../../../../jest/renderer');
 
 const logs = [
   new LogBoxLog(
+    'warn',
     {
       content: 'Some kind of message (first)',
       substitutions: [],
@@ -27,6 +28,7 @@ const logs = [
     [],
   ),
   new LogBoxLog(
+    'warn',
     {
       content: 'Some kind of message (second)',
       substitutions: [],

Libraries/LogBox/UI/__tests__/LogBoxInspectorReactFrames-test.js

@@ -23,6 +23,7 @@ describe('LogBoxInspectorReactFrames', () => {
       <LogBoxInspectorReactFrames
         log={
           new LogBoxLog(
+            'warn',
             {
               content: 'Some kind of message',
               substitutions: [],
@@ -43,6 +44,7 @@ describe('LogBoxInspectorReactFrames', () => {
       <LogBoxInspectorReactFrames
         log={
           new LogBoxLog(
+            'warn',
             {
               content: 'Some kind of message',
               substitutions: [],

Libraries/LogBox/UI/__tests__/LogBoxInspectorStackFrames-test.js

@@ -18,6 +18,7 @@ const LogBoxLog = require('../../Data/LogBoxLog').default;
 const render = require('../../../../jest/renderer');
 
 const log = new LogBoxLog(
+  'warn',
   {
     content: 'Some kind of message (latest)',
     substitutions: [],

Libraries/LogBox/UI/__tests__/LogBoxLogNotification-test.js

@@ -17,6 +17,7 @@ const LogBoxLog = require('../../Data/LogBoxLog').default;
 const render = require('../../../../jest/renderer');
 
 const log = new LogBoxLog(
+  'warn',
   {
     content: 'Some kind of message',
     substitutions: [],

Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxContainer-test.js.snap

@@ -28,6 +28,7 @@ exports[`LogBoxContainer should render the latest log 1`] = `
           "category": "Some kind of message (latest)",
           "componentStack": Array [],
           "count": 1,
+          "level": "warn",
           "message": Object {
             "content": "Some kind of message (latest)",
             "substitutions": Array [],

Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxInspector-test.js.snap

@@ -21,6 +21,7 @@ exports[`LogBoxContainer should render first log with selectedIndex 0 1`] = `
         "category": "Some kind of message (first)",
         "componentStack": Array [],
         "count": 1,
+        "level": "warn",
         "message": Object {
           "content": "Some kind of message (first)",
           "substitutions": Array [],
@@ -65,6 +66,7 @@ exports[`LogBoxContainer should render second log with selectedIndex 1 1`] = `
         "category": "Some kind of message (second)",
         "componentStack": Array [],
         "count": 1,
+        "level": "warn",
         "message": Object {
           "content": "Some kind of message (second)",
           "substitutions": Array [],

Libraries/LogBox/__tests__/LogBox-test.js

@@ -71,9 +71,9 @@ describe('LogBox', () => {
     LogBox.install();
 
     console.error('...');
-    expect(LogBoxData.add).not.toBeCalled();
+    expect(LogBoxData.add).toBeCalledWith('error', ['...']);
 
     console.error('Warning: ...');
-    expect(LogBoxData.add).toBeCalled();
+    expect(LogBoxData.add).toBeCalledWith('warn', ['Warning: ...']);
   });
 });