From 0c83407dd2a748d31bc2dd17477c6dff59f1ef54 Mon Sep 17 00:00:00 2001 From: Pieter De Baets Date: Thu, 5 Nov 2015 12:20:15 -0800 Subject: [PATCH] Simplify logging exceptions from JS to native Reviewed By: vjeux Differential Revision: D2615559 fb-gh-sync-id: ee931b3691251c8b6276699c6f927e47d8e8fd97 --- .../Initialization/ExceptionsManager.js | 83 +++++++------------ .../InitializeJavaScriptAppEngine.js | 34 ++++---- React/Executors/RCTContextExecutor.m | 52 +++++------- 3 files changed, 65 insertions(+), 104 deletions(-) diff --git a/Libraries/JavaScriptAppEngine/Initialization/ExceptionsManager.js b/Libraries/JavaScriptAppEngine/Initialization/ExceptionsManager.js index 41b94be0b..8e652caf0 100644 --- a/Libraries/JavaScriptAppEngine/Initialization/ExceptionsManager.js +++ b/Libraries/JavaScriptAppEngine/Initialization/ExceptionsManager.js @@ -21,12 +21,13 @@ var sourceMapPromise; var exceptionID = 0; -function reportException(e: Error, isFatal: bool, stack?: any) { +/** + * Handles the developer-visible aspect of errors and exceptions + */ +function reportException(e: Error, isFatal: bool) { var currentExceptionID = ++exceptionID; if (RCTExceptionsManager) { - if (!stack) { - stack = parseErrorStack(e); - } + var stack = parseErrorStack(e); if (isFatal) { RCTExceptionsManager.reportFatalException(e.message, stack, currentExceptionID); } else { @@ -47,6 +48,9 @@ function reportException(e: Error, isFatal: bool, stack?: any) { } } +/** + * Logs exceptions to the (native) console and displays them + */ function handleException(e: Error, isFatal: boolean) { // Workaround for reporting errors caused by `throw 'some string'` // Unfortunately there is no way to figure out the stacktrace in this @@ -55,19 +59,9 @@ function handleException(e: Error, isFatal: boolean) { if (!e.message) { e = new Error(e); } - var stack = parseErrorStack(e); - var msg = - 'Error: ' + e.message + - '\n stack: \n' + stackToString(stack) + - '\n URL: ' + (e: any).sourceURL + - '\n line: ' + (e: any).line + - '\n message: ' + e.message; - if (console.errorOriginal) { - console.errorOriginal(msg); - } else { - console.error(msg); - } - reportException(e, isFatal, stack); + + (console._errorOriginal || console.error)(e.message); + reportException(e, isFatal); } /** @@ -75,54 +69,35 @@ function handleException(e: Error, isFatal: boolean) { * setting `console.reportErrorsAsExceptions = false;` in your app. */ function installConsoleErrorReporter() { - if (console.reportException) { + // Enable reportErrorsAsExceptions + if (console._errorOriginal) { return; // already installed } - console.reportException = reportException; - console.errorOriginal = console.error.bind(console); + console._errorOriginal = console.error.bind(console); console.error = function reactConsoleError() { - // Note that when using the built-in context executor on iOS (i.e., not - // Chrome debugging), console.error is already stubbed out to cause a - // redbox via RCTNativeLoggingHook. - console.errorOriginal.apply(null, arguments); + console._errorOriginal.apply(null, arguments); if (!console.reportErrorsAsExceptions) { return; } - var 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; + + if (arguments[0] && arguments[0].stack) { + reportException(arguments[0], /* isFatal */ false); + } else { + var 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; + } + var error : any = new Error('console.error: ' + str); + error.framesToPop = 1; + reportException(error, /* isFatal */ false); } - var error: any = new Error('console.error: ' + str); - error.framesToPop = 1; - reportException(error, /* isFatal */ false); }; if (console.reportErrorsAsExceptions === undefined) { console.reportErrorsAsExceptions = true; // Individual apps can disable this } } -function stackToString(stack) { - var maxLength = Math.max.apply(null, stack.map(frame => frame.methodName.length)); - return stack.map(frame => stackFrameToString(frame, maxLength)).join('\n'); -} - -function stackFrameToString(stackFrame, maxLength) { - var fileNameParts = stackFrame.file.split('/'); - var fileName = fileNameParts[fileNameParts.length - 1]; - - if (fileName.length > 18) { - fileName = fileName.substr(0, 17) + '\u2026'; /* ... */ - } - - var spaces = fillSpaces(maxLength - stackFrame.methodName.length); - return ' ' + stackFrame.methodName + spaces + ' ' + fileName + ':' + stackFrame.lineNumber; -} - -function fillSpaces(n) { - return new Array(n + 1).join(' '); -} - module.exports = { handleException, installConsoleErrorReporter }; diff --git a/Libraries/JavaScriptAppEngine/Initialization/InitializeJavaScriptAppEngine.js b/Libraries/JavaScriptAppEngine/Initialization/InitializeJavaScriptAppEngine.js index b3de8921e..a38350e86 100644 --- a/Libraries/JavaScriptAppEngine/Initialization/InitializeJavaScriptAppEngine.js +++ b/Libraries/JavaScriptAppEngine/Initialization/InitializeJavaScriptAppEngine.js @@ -32,12 +32,10 @@ if (typeof window === 'undefined') { window = GLOBAL; } -function handleError(e, isFatal) { - try { - require('ExceptionsManager').handleException(e, isFatal); - } catch(ee) { - console.log('Failed to print error: ', ee.message); - } +function setUpConsole() { + // ExceptionsManager transitively requires Promise so we install it after + var ExceptionsManager = require('ExceptionsManager'); + ExceptionsManager.installConsoleErrorReporter(); } /** @@ -72,21 +70,19 @@ function polyfillGlobal(name, newValue, scope=GLOBAL) { Object.defineProperty(scope, name, {...descriptor, value: newValue}); } -function setUpRedBoxErrorHandler() { +function setUpErrorHandler() { + function handleError(e, isFatal) { + try { + require('ExceptionsManager').handleException(e, isFatal); + } catch(ee) { + console.log('Failed to print error: ', ee.message); + } + } + var ErrorUtils = require('ErrorUtils'); ErrorUtils.setGlobalHandler(handleError); } -function setUpRedBoxConsoleErrorHandler() { - // ExceptionsManager transitively requires Promise so we install it after - var ExceptionsManager = require('ExceptionsManager'); - var Platform = require('Platform'); - // TODO (#6925182): Enable console.error redbox on Android - if (__DEV__ && Platform.OS === 'ios') { - ExceptionsManager.installConsoleErrorReporter(); - } -} - function setUpFlowChecker() { if (__DEV__) { var checkFlowAtRuntime = require('checkFlowAtRuntime'); @@ -187,12 +183,12 @@ function setUpDevTools() { } setUpProcessEnv(); -setUpRedBoxErrorHandler(); +setUpConsole(); setUpTimers(); setUpAlert(); setUpPromise(); +setUpErrorHandler(); setUpXHR(); -setUpRedBoxConsoleErrorHandler(); setUpGeolocation(); setUpWebSockets(); setUpProfile(); diff --git a/React/Executors/RCTContextExecutor.m b/React/Executors/RCTContextExecutor.m index 8c2aa9947..9b2283c91 100644 --- a/React/Executors/RCTContextExecutor.m +++ b/React/Executors/RCTContextExecutor.m @@ -33,7 +33,7 @@ #if RCT_JSC_PROFILER #include -static NSString * const RCTJSCProfilerEnabledDefaultsKey = @"RCTJSCProfilerEnabled"; +static NSString *const RCTJSCProfilerEnabledDefaultsKey = @"RCTJSCProfilerEnabled"; #ifndef RCT_JSC_PROFILER_DYLIB #define RCT_JSC_PROFILER_DYLIB [[[NSBundle mainBundle] pathForResource:[NSString stringWithFormat:@"RCTJSCProfiler.ios%zd", [[[UIDevice currentDevice] systemVersion] integerValue]] ofType:@"dylib" inDirectory:@"RCTJSCProfiler"] UTF8String] @@ -119,26 +119,14 @@ RCT_EXPORT_MODULE() static JSValueRef RCTNativeLoggingHook(JSContextRef context, __unused JSObjectRef object, __unused JSObjectRef thisObject, size_t argumentCount, const JSValueRef arguments[], JSValueRef *exception) { if (argumentCount > 0) { - JSStringRef messageRef = JSValueToStringCopy(context, arguments[0], exception); - if (!messageRef) { - return JSValueMakeUndefined(context); - } - NSString *message = (__bridge_transfer NSString *)JSStringCopyCFString(kCFAllocatorDefault, messageRef); - JSStringRelease(messageRef); - NSRegularExpression *regex = [NSRegularExpression regularExpressionWithPattern: - @"( stack: )?([_a-z0-9]*)@?(http://|file:///)[a-z.0-9:/_-]+/([a-z0-9_]+).bundle(:[0-9]+:[0-9]+)" - options:NSRegularExpressionCaseInsensitive - error:NULL]; - message = [regex stringByReplacingMatchesInString:message - options:0 - range:(NSRange){0, message.length} - withTemplate:@"[$4$5] \t$2"]; + NSString *message = RCTJSValueToNSString(context, arguments[0], exception); RCTLogLevel level = RCTLogLevelInfo; if (argumentCount > 1) { - level = MAX(level, JSValueToNumber(context, arguments[1], exception) - 1); + level = MAX(level, JSValueToNumber(context, arguments[1], exception)); } - RCTGetLogFunction()(level, nil, nil, message); + + _RCTLog(level, @"%@", message); } return JSValueMakeUndefined(context); @@ -152,18 +140,22 @@ static JSValueRef RCTNoop(JSContextRef context, __unused JSObjectRef object, __u return JSValueMakeUndefined(context); } -static NSString *RCTJSValueToNSString(JSContextRef context, JSValueRef value) +static NSString *RCTJSValueToNSString(JSContextRef context, JSValueRef value, JSValueRef *exception) { - JSStringRef JSString = JSValueToStringCopy(context, value, NULL); + JSStringRef JSString = JSValueToStringCopy(context, value, exception); + if (!JSString) { + return nil; + } + CFStringRef string = JSStringCopyCFString(kCFAllocatorDefault, JSString); JSStringRelease(JSString); return (__bridge_transfer NSString *)string; } -static NSString *RCTJSValueToJSONString(JSContextRef context, JSValueRef value, unsigned indent) +static NSString *RCTJSValueToJSONString(JSContextRef context, JSValueRef value, JSValueRef *exception, unsigned indent) { - JSStringRef JSString = JSValueCreateJSONString(context, value, indent, NULL); + JSStringRef JSString = JSValueCreateJSONString(context, value, indent, exception); CFStringRef string = JSStringCopyCFString(kCFAllocatorDefault, JSString); JSStringRelease(JSString); @@ -172,14 +164,14 @@ static NSString *RCTJSValueToJSONString(JSContextRef context, JSValueRef value, static NSError *RCTNSErrorFromJSError(JSContextRef context, JSValueRef jsError) { - NSString *errorMessage = jsError ? RCTJSValueToNSString(context, jsError) : @"unknown JS error"; - NSString *details = jsError ? RCTJSValueToJSONString(context, jsError, 2) : @"no details"; + NSString *errorMessage = jsError ? RCTJSValueToNSString(context, jsError, NULL) : @"unknown JS error"; + NSString *details = jsError ? RCTJSValueToJSONString(context, jsError, NULL, 2) : @"no details"; return [NSError errorWithDomain:@"JS" code:1 userInfo:@{NSLocalizedDescriptionKey: errorMessage, NSLocalizedFailureReasonErrorKey: details}]; } #if RCT_DEV -static JSValueRef RCTNativeTraceBeginSection(JSContextRef context, __unused JSObjectRef object, __unused JSObjectRef thisObject, size_t argumentCount, const JSValueRef arguments[], __unused JSValueRef *exception) +static JSValueRef RCTNativeTraceBeginSection(JSContextRef context, __unused JSObjectRef object, __unused JSObjectRef thisObject, size_t argumentCount, const JSValueRef arguments[], JSValueRef *exception) { static int profileCounter = 1; NSString *profileName; @@ -189,14 +181,14 @@ static JSValueRef RCTNativeTraceBeginSection(JSContextRef context, __unused JSOb if (JSValueIsNumber(context, arguments[0])) { tag = JSValueToNumber(context, arguments[0], NULL); } else { - profileName = RCTJSValueToNSString(context, arguments[0]); + profileName = RCTJSValueToNSString(context, arguments[0], exception); } } else { profileName = [NSString stringWithFormat:@"Profile %d", profileCounter++]; } if (argumentCount > 1 && JSValueIsString(context, arguments[1])) { - profileName = RCTJSValueToNSString(context, arguments[1]); + profileName = RCTJSValueToNSString(context, arguments[1], exception); } if (profileName) { @@ -206,13 +198,11 @@ static JSValueRef RCTNativeTraceBeginSection(JSContextRef context, __unused JSOb return JSValueMakeUndefined(context); } -static JSValueRef RCTNativeTraceEndSection(JSContextRef context, __unused JSObjectRef object, __unused JSObjectRef thisObject, __unused size_t argumentCount, __unused const JSValueRef arguments[], __unused JSValueRef *exception) +static JSValueRef RCTNativeTraceEndSection(JSContextRef context, __unused JSObjectRef object, __unused JSObjectRef thisObject, __unused size_t argumentCount, __unused const JSValueRef arguments[], JSValueRef *exception) { if (argumentCount > 0) { - JSValueRef *error = NULL; - double tag = JSValueToNumber(context, arguments[0], error); - - if (error == NULL) { + double tag = JSValueToNumber(context, arguments[0], exception); + if (exception == NULL) { RCTProfileEndEvent((uint64_t)tag, @"console", nil); } }