Skip to content

Commit

Permalink
Cleanup console and ErrorUtils
Browse files Browse the repository at this point in the history 
Reviewed By: davidaurelio

Differential Revision: D3981005

fbshipit-source-id: 3c95e62177f78785c7f971dd632126dbfb83746b
  • Loading branch information
@javache
javache authored and Facebook Github Bot committed Oct 11, 2016
1 parent 08e715b commit 3137ba9
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 80 deletions.
83 changes: 43 additions & 40 deletions Libraries/JavaScriptAppEngine/Initialization/ExceptionsManager.js
View file
Expand Up @@ -11,41 +11,43 @@
*/
'use strict';

let exceptionID = 0;

/**
* Handles the developer-visible aspect of errors and exceptions
*/
let exceptionID = 0;
function reportException(e: Error, isFatal: bool) {
const parseErrorStack = require('parseErrorStack');
const symbolicateStackTrace = require('symbolicateStackTrace');
const RCTExceptionsManager = require('NativeModules').ExceptionsManager;

const currentExceptionID = ++exceptionID;
if (RCTExceptionsManager) {
const {ExceptionsManager} = require('NativeModules');
if (ExceptionsManager) {
const parseErrorStack = require('parseErrorStack');
const stack = parseErrorStack(e);
const currentExceptionID = ++exceptionID;
if (isFatal) {
RCTExceptionsManager.reportFatalException(e.message, stack, currentExceptionID);
ExceptionsManager.reportFatalException(e.message, stack, currentExceptionID);
} else {
RCTExceptionsManager.reportSoftException(e.message, stack, currentExceptionID);
ExceptionsManager.reportSoftException(e.message, stack, currentExceptionID);
}
if (__DEV__) {
const symbolicateStackTrace = require('symbolicateStackTrace');
symbolicateStackTrace(stack).then(
(prettyStack) => {
if (prettyStack) {
RCTExceptionsManager.updateExceptionMessage(e.message, prettyStack, currentExceptionID);
ExceptionsManager.updateExceptionMessage(e.message, prettyStack, currentExceptionID);
} else {
throw new Error('The stack is null');
}
}
).catch(
(error) =>
console.warn('Unable to symbolicate stack trace: ' + error.message)
(error) => console.warn('Unable to symbolicate stack trace: ' + error.message)
);
}
}
}

declare var console: typeof console & {
_errorOriginal: Function,
reportErrorsAsExceptions: boolean,
};

/**
* Logs exceptions to the (native) console and displays them
*/
Expand All @@ -57,15 +59,37 @@ function handleException(e: Error, isFatal: boolean) {
if (!e.message) {
e = new Error(e);
}

if (typeof console._errorOriginal === 'function') {
if (console._errorOriginal) {
console._errorOriginal(e.message);
} else {
console.error(e.message);
}
reportException(e, isFatal);
}

function reactConsoleErrorHandler() {
console._errorOriginal.apply(console, arguments);
if (!console.reportErrorsAsExceptions) {
return;
}

if (arguments[0] && arguments[0].stack) {
reportException(arguments[0], /* isFatal */ false);
} else {
const stringifySafe = require('stringifySafe');
const str = Array.prototype.map.call(arguments, stringifySafe).join(', ');
if (str.slice(0, 10) === '"Warning: ') {
// React warnings use console.error so that a stack trace is shown, but
// we don't (currently) want these to show a redbox
// (Note: Logic duplicated in polyfills/console.js.)
return;
}
const error : any = new Error('console.error: ' + str);
error.framesToPop = 1;
reportException(error, /* isFatal */ false);
}
}

/**
* Shows a redbox with stacktrace for all console.error messages. Disable by
* setting `console.reportErrorsAsExceptions = false;` in your app.
Expand All @@ -76,33 +100,12 @@ function installConsoleErrorReporter() {
return; // already installed
}
// Flow doesn't like it when you set arbitrary values on a global object
(console: any)._errorOriginal = console.error.bind(console);
console.error = function reactConsoleError() {
// Flow doesn't like it when you set arbitrary values on a global object
(console: any)._errorOriginal.apply(null, arguments);
if (!console.reportErrorsAsExceptions) {
return;
}

if (arguments[0] && arguments[0].stack) {
reportException(arguments[0], /* isFatal */ false);
} else {
const stringifySafe = require('stringifySafe');
const str = Array.prototype.map.call(arguments, stringifySafe).join(', ');
if (str.slice(0, 10) === '"Warning: ') {
// React warnings use console.error so that a stack trace is shown, but
// we don't (currently) want these to show a redbox
// (Note: Logic duplicated in polyfills/console.js.)
return;
}
const error : any = new Error('console.error: ' + str);
error.framesToPop = 1;
reportException(error, /* isFatal */ false);
}
};
console._errorOriginal = console.error.bind(console);

This comment has been minimized.

Sign in to view

Copy link
@jondot

jondot Oct 29, 2016 •
edited

Contributor

Show Flow complain here?. Is it worth bringing back (console: any)? or maybe console defined as any globally?
I'm currently using Flow 0.34 and thinking of submitting a PR to get rid of 3 errors it spits (it sees assigning to console.error as a contravariant (write) use, because previously it identified it as covariant (read))
console.error = reactConsoleErrorHandler;
if (console.reportErrorsAsExceptions === undefined) {
// Individual apps can disable this
// Flow doesn't like it when you set arbitrary values on a global object
(console: any).reportErrorsAsExceptions = true; // Individual apps can disable this
console.reportErrorsAsExceptions = true;
}
}

Expand Down
24 changes: 8 additions & 16 deletions packager/react-packager/src/Resolver/polyfills/console.js
View file
Expand Up @@ -16,7 +16,7 @@

/* eslint-disable */

var inspect = (function() {
const inspect = (function() {
// Copyright Joyent, Inc. and other Node contributors.
//
// Permission is hereby granted, free of charge, to any person obtaining a
Expand Down Expand Up @@ -356,8 +356,8 @@ var inspect = (function() {
})();


var OBJECT_COLUMN_NAME = '(index)';
var LOG_LEVELS = {
const OBJECT_COLUMN_NAME = '(index)';
const LOG_LEVELS = {
trace: 0,
info: 1,
warn: 2,
Expand All @@ -371,7 +371,7 @@ function setupConsole(global) {

function getNativeLogFunction(level) {
return function() {
var str;
let str;
if (arguments.length === 1 && typeof arguments[0] === 'string') {
str = arguments[0];
} else {
Expand All @@ -380,7 +380,7 @@ function setupConsole(global) {
}).join(', ');
}

var logLevel = level;
let logLevel = level;
if (str.slice(0, 9) === 'Warning: ' && logLevel >= LOG_LEVELS.error) {
// React warnings use console.error so that a stack trace is shown,
// but we don't (currently) want these to show a redbox
Expand All @@ -391,7 +391,7 @@ function setupConsole(global) {
};
}

var repeat = function(element, n) {
function repeat(element, n) {
return Array.apply(null, Array(n)).map(function() { return element; });
};

Expand Down Expand Up @@ -431,7 +431,7 @@ function setupConsole(global) {

// Join all elements in the row into a single string with | separators
// (appends extra spaces to each cell to make separators | alligned)
var joinRow = function(row, space) {
function joinRow(row, space) {
var cells = row.map(function(cell, i) {
var extraSpaces = repeat(' ', columnWidths[i] - cell.length).join('');
return cell + extraSpaces;
Expand Down Expand Up @@ -465,7 +465,7 @@ function setupConsole(global) {
Object.defineProperty(global, 'originalConsole', descriptor);
}

var console = {
global.console = {
error: getNativeLogFunction(LOG_LEVELS.error),
info: getNativeLogFunction(LOG_LEVELS.info),
log: getNativeLogFunction(LOG_LEVELS.info),
Expand All @@ -475,14 +475,6 @@ function setupConsole(global) {
table: consoleTablePolyfill
};

// don't reassign to the original descriptor. breaks on ios7
Object.defineProperty(global, 'console', {
value: console,
configurable: descriptor ? descriptor.configurable : true,
enumerable: descriptor ? descriptor.enumerable : true,
writable: descriptor ? descriptor.writable : true,
});

// If available, also call the original `console` method since that is
// sometimes useful. Ex: on OS X, this will let you see rich output in
// the Safari Web Inspector console.
Expand Down
45 changes: 21 additions & 24 deletions packager/react-packager/src/Resolver/polyfills/error-guard.js
View file
Expand Up @@ -13,30 +13,40 @@
* before any of the modules, this ErrorUtils must be defined (and the handler
* set) globally before requiring anything.
*/

/* eslint strict:0 */
var ErrorUtils = {
_inGuard: 0,
_globalHandler: null,
const _inGuard = 0;

/**
* This is the error handler that is called when we encounter an exception
* when loading a module. This will report any errors encountered before
* ExceptionsManager is configured.
*/
const _globalHandler = function onError(e) {

This comment has been minimized.

Sign in to view

Copy link
@trabianmatt

trabianmatt Nov 8, 2016

@javache - any reason this is a const given its potential reassignment below?
throw e;
};

const ErrorUtils = {
setGlobalHandler: function(fun) {
ErrorUtils._globalHandler = fun;
_globalHandler = fun;
},
getGlobalHandler: function() {
return ErrorUtils._globalHandler;
return _globalHandler;
},
reportError: function(error) {
ErrorUtils._globalHandler && ErrorUtils._globalHandler(error);
_globalHandler && _globalHandler(error);
},
reportFatalError: function(error) {
ErrorUtils._globalHandler && ErrorUtils._globalHandler(error, true);
_globalHandler && _globalHandler(error, true);
},
applyWithGuard: function(fun, context, args) {
try {
ErrorUtils._inGuard++;
_inGuard++;
return fun.apply(context, args);
} catch (e) {
ErrorUtils.reportError(e);
} finally {
ErrorUtils._inGuard--;
_inGuard--;
}
},
applyWithGuardIfNeeded: function(fun, context, args) {
Expand All @@ -47,7 +57,7 @@ var ErrorUtils = {
}
},
inGuard: function() {
return ErrorUtils._inGuard;
return _inGuard;
},
guard: function(fun, name, context) {
if (typeof fun !== 'function') {
Expand All @@ -70,18 +80,5 @@ var ErrorUtils = {
return guarded;
}
};
global.ErrorUtils = ErrorUtils;

/**
* This is the error handler that is called when we encounter an exception
* when loading a module. This will report any errors encountered before
* ExceptionsManager is configured.
*/
function setupErrorGuard() {
var onError = function(e) {
throw e;
};
global.ErrorUtils.setGlobalHandler(onError);
}

setupErrorGuard();
global.ErrorUtils = ErrorUtils;

0 comments on commit 3137ba9

Please sign in to comment.