Skip to content

Commit

Permalink
Centralize public access to LogBox in LogBox module
Browse files Browse the repository at this point in the history
Summary:
Some components are using `LogBoxData` directly, forcing logs to be shown on the screen even when LogBox is uninstalled. This changes all accesses to `LogBoxData` to go through `LogBox` so `uninstall` is used correctly.

It also changes when LogBox is installed, moving it from `AppContainer` to `InitializeCore` (which happens earlier) so we can capture more logs in LogBox.

Changelog: [General][Changed] Initialized LogBox earlier and centralized access in LogBox module

Reviewed By: rickhanlonii

Differential Revision: D27999361

fbshipit-source-id: 1115ef6b71e08cc33743d205da0064fbe9a74a0e
  • Loading branch information
rubennorte authored and facebook-github-bot committed May 4, 2021
1 parent ab45509 commit 8abe737
Show file tree
Hide file tree
Showing 7 changed files with 238 additions and 66 deletions.
4 changes: 2 additions & 2 deletions Libraries/Core/ExceptionsManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,8 @@ function reportException(
}

if (__DEV__ && isHandledByLogBox) {
const LogBoxData = require('../LogBox/Data/LogBoxData');
LogBoxData.addException({
const LogBox = require('../LogBox/LogBox');
LogBox.addException({
...data,
isComponentError: !!e.isComponentError,
});
Expand Down
1 change: 1 addition & 0 deletions Libraries/Core/InitializeCore.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ require('./setUpSegmentFetcher');
if (__DEV__) {
require('./checkNativeVersion');
require('./setUpDeveloperTools');
require('../LogBox/LogBox').install();
}

const GlobalPerformanceLogger = require('../Utilities/GlobalPerformanceLogger');
Expand Down
161 changes: 114 additions & 47 deletions Libraries/LogBox/LogBox.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,56 +11,69 @@
import Platform from '../Utilities/Platform';
import RCTLog from '../Utilities/RCTLog';

import type {IgnorePattern} from './Data/LogBoxData';
import type {IgnorePattern, LogData} from './Data/LogBoxData';
import type {ExtendedExceptionData} from './Data/parseLogBoxLog';

export type {LogData, ExtendedExceptionData, IgnorePattern};

let LogBox;

interface ILogBox {
install(): void;
uninstall(): void;
isInstalled(): boolean;
ignoreLogs($ReadOnlyArray<IgnorePattern>): void;
ignoreAllLogs(?boolean): void;
clearAllLogs(): void;
addLog(log: LogData): void;
addException(error: ExtendedExceptionData): void;
}

/**
* LogBox displays logs in the app.
*/
if (__DEV__) {
const LogBoxData = require('./Data/LogBoxData');
const {parseLogBoxLog, parseInterpolation} = require('./Data/parseLogBoxLog');

// LogBox needs to insert itself early,
// in order to access the component stacks appended by React DevTools.
const {error, warn} = console;
let errorImpl = error.bind(console);
let warnImpl = warn.bind(console);
let originalConsoleError;
let originalConsoleWarn;
let consoleErrorImpl;
let consoleWarnImpl;

(console: any).error = function(...args) {
errorImpl(...args);
};
(console: any).warn = function(...args) {
warnImpl(...args);
};
let isLogBoxInstalled: boolean = false;

LogBox = {
ignoreLogs: (patterns: $ReadOnlyArray<IgnorePattern>): void => {
LogBoxData.addIgnorePatterns(patterns);
},
install(): void {
if (isLogBoxInstalled) {
return;
}

ignoreAllLogs: (value?: ?boolean): void => {
LogBoxData.setDisabled(value == null ? true : value);
},
isLogBoxInstalled = true;

uninstall: (): void => {
errorImpl = error;
warnImpl = warn;
delete (console: any).disableLogBox;
},

install: (): void => {
// Trigger lazy initialization of module.
require('../NativeModules/specs/NativeLogBox');

errorImpl = function(...args) {
registerError(...args);
};
// IMPORTANT: we only overwrite `console.error` and `console.warn` once.
// When we uninstall we keep the same reference and only change its
// internal implementation
const isFirstInstall = originalConsoleError == null;
if (isFirstInstall) {
originalConsoleError = console.error.bind(console);
originalConsoleWarn = console.warn.bind(console);

// $FlowExpectedError[cannot-write]
console.error = (...args) => {
consoleErrorImpl(...args);
};
// $FlowExpectedError[cannot-write]
console.warn = (...args) => {
consoleWarnImpl(...args);
};
}

warnImpl = function(...args) {
registerWarning(...args);
};
consoleErrorImpl = registerError;
consoleWarnImpl = registerWarning;

if ((console: any).disableYellowBox === true) {
LogBoxData.setDisabled(true);
Expand Down Expand Up @@ -88,6 +101,50 @@ if (__DEV__) {
registerWarning(...args);
});
},

uninstall(): void {
if (!isLogBoxInstalled) {
return;
}

isLogBoxInstalled = false;

// IMPORTANT: we don't re-assign to `console` in case the method has been
// decorated again after installing LogBox. E.g.:
// Before uninstalling: original > LogBox > OtherErrorHandler
// After uninstalling: original > LogBox (noop) > OtherErrorHandler
consoleErrorImpl = originalConsoleError;
consoleWarnImpl = originalConsoleWarn;
delete (console: any).disableLogBox;
},

isInstalled(): boolean {
return isLogBoxInstalled;
},

ignoreLogs(patterns: $ReadOnlyArray<IgnorePattern>): void {
LogBoxData.addIgnorePatterns(patterns);
},

ignoreAllLogs(value?: ?boolean): void {
LogBoxData.setDisabled(value == null ? true : value);
},

clearAllLogs(): void {
LogBoxData.clear();
},

addLog(log: LogData): void {
if (isLogBoxInstalled) {
LogBoxData.addLog(log);
}
},

addException(error: ExtendedExceptionData): void {
if (isLogBoxInstalled) {
LogBoxData.addException(error);
}
},
};

const isRCTLogAdviceWarning = (...args) => {
Expand All @@ -103,7 +160,7 @@ if (__DEV__) {
const registerWarning = (...args): void => {
// Let warnings within LogBox itself fall through.
if (LogBoxData.isLogBoxErrorMessage(String(args[0]))) {
error.call(console, ...args);
originalConsoleError(...args);
return;
}

Expand All @@ -113,7 +170,7 @@ if (__DEV__) {

if (!LogBoxData.isMessageIgnored(message.content)) {
// Be sure to pass LogBox warnings through.
warn.call(console, ...args);
originalConsoleWarn(...args);

LogBoxData.addLog({
level: 'warn',
Expand All @@ -131,7 +188,7 @@ if (__DEV__) {
const registerError = (...args): void => {
// Let errors within LogBox itself fall through.
if (LogBoxData.isLogBoxErrorMessage(args[0])) {
error.call(console, ...args);
originalConsoleError(...args);
return;
}

Expand All @@ -144,7 +201,7 @@ if (__DEV__) {
//
// 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);
originalConsoleError(...args);
return;
}

Expand All @@ -169,7 +226,7 @@ if (__DEV__) {
// Interpolate the message so they are formatted for adb and other CLIs.
// This is different than the message.content above because it includes component stacks.
const interpolated = parseInterpolation(args);
error.call(console, interpolated.message.content);
originalConsoleError(interpolated.message.content);

LogBoxData.addLog({
level,
Expand All @@ -184,28 +241,38 @@ if (__DEV__) {
};
} else {
LogBox = {
ignoreLogs: (patterns: $ReadOnlyArray<IgnorePattern>): void => {
install(): void {
// Do nothing.
},

uninstall(): void {
// Do nothing.
},

isInstalled(): boolean {
return false;
},

ignoreLogs(patterns: $ReadOnlyArray<IgnorePattern>): void {
// Do nothing.
},

ignoreAllLogs(value?: ?boolean): void {
// Do nothing.
},

ignoreAllLogs: (value?: ?boolean): void => {
clearAllLogs(): void {
// Do nothing.
},

install: (): void => {
addLog(log: LogData): void {
// Do nothing.
},

uninstall: (): void => {
addException(error: ExtendedExceptionData): void {
// Do nothing.
},
};
}

module.exports = (LogBox: {
ignoreLogs($ReadOnlyArray<IgnorePattern>): void,
ignoreAllLogs(?boolean): void,
install(): void,
uninstall(): void,
...
});
module.exports = (LogBox: ILogBox);
109 changes: 107 additions & 2 deletions Libraries/LogBox/__tests__/LogBox-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,13 @@ function mockFilterResult(returnValues) {

describe('LogBox', () => {
const {error, warn} = console;
let consoleError;
let consoleWarn;

beforeEach(() => {
jest.resetModules();
console.error = jest.fn();
console.warn = jest.fn();
console.error = consoleError = jest.fn();
console.warn = consoleWarn = jest.fn();
console.disableYellowBox = false;
});

Expand Down Expand Up @@ -276,4 +278,107 @@ describe('LogBox', () => {
},
});
});

it('ignores console methods after uninstalling', () => {
jest.mock('../Data/LogBoxData');

LogBox.install();
LogBox.uninstall();

console.log('Test');
console.warn('Test');
console.error('Test');

expect(LogBoxData.addLog).not.toHaveBeenCalled();
});

it('does not add logs after uninstalling', () => {
jest.mock('../Data/LogBoxData');

LogBox.install();
LogBox.uninstall();

LogBox.addLog({
level: 'warn',
category: 'test',
message: {content: 'Some warning', substitutions: []},
componentStack: [],
});

expect(LogBoxData.addLog).not.toHaveBeenCalled();
});

it('does not add exceptions after uninstalling', () => {
jest.mock('../Data/LogBoxData');

LogBox.install();
LogBox.uninstall();

LogBox.addException({
message: 'Some error',
originalMessage: null,
name: 'Error',
componentStack: null,
stack: [],
id: 12,
isFatal: true,
isComponentError: false,
});

expect(LogBoxData.addException).not.toHaveBeenCalled();
});

it('preserves decorations of console.error after installing/uninstalling', () => {
LogBox.install();

const originalConsoleError = console.error;
console.error = message => {
originalConsoleError('Custom: ' + message);
};

console.error('before uninstalling');

expect(consoleError).toHaveBeenCalledWith('Custom: before uninstalling');

LogBox.uninstall();

console.error('after uninstalling');

expect(consoleError).toHaveBeenCalledWith('Custom: after uninstalling');

LogBox.install();

console.error('after installing for the second time');

expect(consoleError).toHaveBeenCalledWith(
'Custom: after installing for the second time',
);
});

it('preserves decorations of console.warn after installing/uninstalling', () => {
LogBox.install();

const originalConsoleWarn = console.warn;
console.warn = message => {
originalConsoleWarn('Custom: ' + message);
};

console.warn('before uninstalling');

expect(consoleWarn).toHaveBeenCalledWith('Custom: before uninstalling');

LogBox.uninstall();

console.warn('after uninstalling');

expect(consoleWarn).toHaveBeenCalledWith('Custom: after uninstalling');

LogBox.install();

console.warn('after installing for the second time');

expect(consoleWarn).toHaveBeenCalledWith(
'Custom: after installing for the second time',
);
});
});
Loading

0 comments on commit 8abe737

Please sign in to comment.