diff --git a/Libraries/BatchedBridge/NativeModules.js b/Libraries/BatchedBridge/NativeModules.js index 791cbbccffe1f8..8a000e51509eea 100644 --- a/Libraries/BatchedBridge/NativeModules.js +++ b/Libraries/BatchedBridge/NativeModules.js @@ -15,6 +15,8 @@ const BatchedBridge = require('BatchedBridge'); const invariant = require('fbjs/lib/invariant'); +import type {ExtendedError} from 'parseErrorStack'; + type ModuleConfig = [ string, /* name */ ?Object, /* constants */ @@ -113,13 +115,13 @@ function arrayContains(array: Array, value: T): boolean { return array.indexOf(value) !== -1; } -function createErrorFromErrorData(errorData: {message: string}): Error { +function createErrorFromErrorData(errorData: {message: string}): ExtendedError { const { message, ...extraErrorInfo } = errorData; - const error = new Error(message); - (error:any).framesToPop = 1; + const error : ExtendedError = new Error(message); + error.framesToPop = 1; return Object.assign(error, extraErrorInfo); } diff --git a/Libraries/Core/Devtools/parseErrorStack.js b/Libraries/Core/Devtools/parseErrorStack.js index bd2ef9a9d85f93..db22d7910157b9 100644 --- a/Libraries/Core/Devtools/parseErrorStack.js +++ b/Libraries/Core/Devtools/parseErrorStack.js @@ -18,20 +18,22 @@ export type StackFrame = { methodName: string, }; -var stacktraceParser = require('stacktrace-parser'); +export type ExtendedError = Error & { + framesToPop?: number, +}; -function parseErrorStack(e: Error): Array { +function parseErrorStack(e: ExtendedError): Array { if (!e || !e.stack) { return []; } - var stack = Array.isArray(e.stack) ? e.stack : stacktraceParser.parse(e.stack); + const stacktraceParser = require('stacktrace-parser'); + const stack = Array.isArray(e.stack) ? e.stack : stacktraceParser.parse(e.stack); - var framesToPop = typeof e.framesToPop === 'number' ? e.framesToPop : 0; + let framesToPop = typeof e.framesToPop === 'number' ? e.framesToPop : 0; while (framesToPop--) { stack.shift(); } - return stack; } diff --git a/Libraries/Core/ExceptionsManager.js b/Libraries/Core/ExceptionsManager.js index 6891f509ffbbc6..56ced328d2114a 100644 --- a/Libraries/Core/ExceptionsManager.js +++ b/Libraries/Core/ExceptionsManager.js @@ -11,11 +11,13 @@ */ 'use strict'; +import type {ExtendedError} from 'parseErrorStack'; + /** * Handles the developer-visible aspect of errors and exceptions */ let exceptionID = 0; -function reportException(e: Error, isFatal: bool) { +function reportException(e: ExtendedError, isFatal: bool) { const {ExceptionsManager} = require('NativeModules'); if (ExceptionsManager) { const parseErrorStack = require('parseErrorStack'); @@ -84,7 +86,7 @@ function reactConsoleErrorHandler() { // (Note: Logic duplicated in polyfills/console.js.) return; } - const error : any = new Error('console.error: ' + str); + const error : ExtendedError = new Error('console.error: ' + str); error.framesToPop = 1; reportException(error, /* isFatal */ false); } diff --git a/Libraries/Core/Timers/JSTimers.js b/Libraries/Core/Timers/JSTimers.js index 8109a96bc83bbd..88b7ae38c77c49 100644 --- a/Libraries/Core/Timers/JSTimers.js +++ b/Libraries/Core/Timers/JSTimers.js @@ -16,10 +16,12 @@ const JSTimersExecution = require('JSTimersExecution'); const Platform = require('Platform'); -const {Timing} = require('NativeModules'); const performanceNow = require('fbjs/lib/performanceNow'); +const {Timing} = require('NativeModules'); + import type {JSTimerType} from 'JSTimersExecution'; +import type {ExtendedError} from 'parseErrorStack'; // Returns a free index if one is available, and the next consecutive index otherwise. function _getFreeIndex(): number { @@ -38,9 +40,9 @@ function _allocateCallback(func: Function, type: JSTimerType): number { JSTimersExecution.types[freeIndex] = type; if (__DEV__) { const parseErrorStack = require('parseErrorStack'); - const e = (new Error() : any); - e.framesToPop = 1; - const stack = parseErrorStack(e); + const error : ExtendedError = new Error(); + error.framesToPop = 1; + const stack = parseErrorStack(error); if (stack) { JSTimersExecution.identifiers[freeIndex] = stack.shift(); } diff --git a/ReactAndroid/src/main/java/com/facebook/react/devsupport/StackTraceHelper.java b/ReactAndroid/src/main/java/com/facebook/react/devsupport/StackTraceHelper.java index 6adf57855c9e68..c9c4abbb170914 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/devsupport/StackTraceHelper.java +++ b/ReactAndroid/src/main/java/com/facebook/react/devsupport/StackTraceHelper.java @@ -17,6 +17,7 @@ import com.facebook.react.bridge.ReadableArray; import com.facebook.react.bridge.ReadableMap; +import com.facebook.react.bridge.ReadableType; import com.facebook.react.common.MapBuilder; import com.facebook.react.devsupport.interfaces.StackFrame; @@ -123,18 +124,23 @@ public static StackFrame[] convertJsStackTrace(@Nullable ReadableArray stack) { int size = stack != null ? stack.size() : 0; StackFrame[] result = new StackFrame[size]; for (int i = 0; i < size; i++) { - ReadableMap frame = stack.getMap(i); - String methodName = frame.getString("methodName"); - String fileName = frame.getString("file"); - int lineNumber = -1; - if (frame.hasKey(LINE_NUMBER_KEY) && !frame.isNull(LINE_NUMBER_KEY)) { - lineNumber = frame.getInt(LINE_NUMBER_KEY); - } - int columnNumber = -1; - if (frame.hasKey(COLUMN_KEY) && !frame.isNull(COLUMN_KEY)) { - columnNumber = frame.getInt(COLUMN_KEY); + ReadableType type = stack.getType(i); + if (type == ReadableType.Map) { + ReadableMap frame = stack.getMap(i); + String methodName = frame.getString("methodName"); + String fileName = frame.getString("file"); + int lineNumber = -1; + if (frame.hasKey(LINE_NUMBER_KEY) && !frame.isNull(LINE_NUMBER_KEY)) { + lineNumber = frame.getInt(LINE_NUMBER_KEY); + } + int columnNumber = -1; + if (frame.hasKey(COLUMN_KEY) && !frame.isNull(COLUMN_KEY)) { + columnNumber = frame.getInt(COLUMN_KEY); + } + result[i] = new StackFrameImpl(fileName, methodName, lineNumber, columnNumber); + } else if (type == ReadableType.String) { + result[i] = new StackFrameImpl(null, stack.getString(i), -1, -1); } - result[i] = new StackFrameImpl(fileName, methodName, lineNumber, columnNumber); } return result; } diff --git a/ReactAndroid/src/main/jni/react/jni/OnLoad.cpp b/ReactAndroid/src/main/jni/react/jni/OnLoad.cpp index b01f5ce6228494..11e05d64c39fb7 100644 --- a/ReactAndroid/src/main/jni/react/jni/OnLoad.cpp +++ b/ReactAndroid/src/main/jni/react/jni/OnLoad.cpp @@ -152,6 +152,36 @@ static void logPerfMarker(const ReactMarker::ReactMarkerId markerId, const char* } } +static ExceptionHandling::ExtractedEror extractJniError(const std::exception& ex, const char *context) { + auto jniEx = dynamic_cast(&ex); + if (!jniEx) { + return {}; + } + + auto stackTrace = jniEx->getThrowable()->getStackTrace(); + std::ostringstream stackStr; + for (int i = 0, count = stackTrace->size(); i < count; ++i) { + auto frame = stackTrace->getElement(i); + + auto methodName = folly::to(frame->getClassName(), ".", + frame->getMethodName()); + + // Cut off stack traces at the Android looper, to keep them simple + if (methodName == "android.os.Looper.loop") { + break; + } + + stackStr << std::move(methodName) << '@' << frame->getFileName(); + if (frame->getLineNumber() > 0) { + stackStr << ':' << frame->getLineNumber(); + } + stackStr << std::endl; + } + + auto msg = folly::to("Java exception in '", context, "'\n\n", jniEx->what()); + return {.message = msg, .stack = stackStr.str()}; +} + } extern "C" JNIEXPORT jint JNI_OnLoad(JavaVM* vm, void* reserved) { @@ -159,6 +189,7 @@ extern "C" JNIEXPORT jint JNI_OnLoad(JavaVM* vm, void* reserved) { gloginit::initialize(); // Inject some behavior into react/ ReactMarker::logTaggedMarker = logPerfMarker; + ExceptionHandling::platformErrorExtractor = extractJniError; JSCNativeHooks::loggingHook = nativeLoggingHook; JSCNativeHooks::nowHook = nativePerformanceNow; JSCNativeHooks::installPerfHooks = addNativePerfLoggingHooks; diff --git a/ReactCommon/jschelpers/JSCHelpers.cpp b/ReactCommon/jschelpers/JSCHelpers.cpp index 8e20404151cbac..61e3b49094f68e 100644 --- a/ReactCommon/jschelpers/JSCHelpers.cpp +++ b/ReactCommon/jschelpers/JSCHelpers.cpp @@ -1,5 +1,5 @@ // Copyright 2004-present Facebook. All Rights Reserved. - + #include "JSCHelpers.h" #ifdef WITH_FBSYSTRACE @@ -74,14 +74,14 @@ void JSException::buildMessage(JSContextRef ctx, JSValueRef exn, JSStringRef sou msgBuilder << errorMsg << ": "; } - Value exnValue = Value(ctx, exn); - msgBuilder << exnValue.toString().str(); + Object exnObject = Value(ctx, exn).asObject(); + Value exnMessage = exnObject.getProperty("message"); + msgBuilder << (exnMessage.isString() ? exnMessage : (Value)exnObject).toString().str(); // The null/empty-ness of source tells us if the JS came from a // file/resource, or was a constructed statement. The location // info will include that source, if any. std::string locationInfo = sourceURL != nullptr ? String::ref(ctx, sourceURL).str() : ""; - Object exnObject = exnValue.asObject(); auto line = exnObject.getProperty("line"); if (line != nullptr && line.isNumber()) { if (locationInfo.empty() && line.asInteger() != 1) { @@ -112,6 +112,20 @@ void JSException::buildMessage(JSContextRef ctx, JSValueRef exn, JSStringRef sou } } +namespace ExceptionHandling { + +#if __clang__ +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wglobal-constructors" +#endif + +PlatformErrorExtractor platformErrorExtractor; + +#if __clang__ +#pragma clang diagnostic pop +#endif + +} JSObjectRef makeFunction( JSContextRef ctx, @@ -188,20 +202,25 @@ JSValueRef evaluateSourceCode(JSContextRef context, JSSourceCodeRef source, JSSt #endif JSValueRef translatePendingCppExceptionToJSError(JSContextRef ctx, const char *exceptionLocation) { - std::ostringstream msg; try { throw; } catch (const std::bad_alloc& ex) { throw; // We probably shouldn't try to handle this in JS } catch (const std::exception& ex) { - msg << "C++ Exception in '" << exceptionLocation << "': " << ex.what(); - return Value::makeError(ctx, msg.str().c_str()); + if (ExceptionHandling::platformErrorExtractor) { + auto extractedEror = ExceptionHandling::platformErrorExtractor(ex, exceptionLocation); + if (extractedEror.message.length() > 0) { + return Value::makeError(ctx, extractedEror.message.c_str(), extractedEror.stack.c_str()); + } + } + auto msg = folly::to("C++ exception in '", exceptionLocation, "'\n\n", ex.what()); + return Value::makeError(ctx, msg.c_str()); } catch (const char* ex) { - msg << "C++ Exception (thrown as a char*) in '" << exceptionLocation << "': " << ex; - return Value::makeError(ctx, msg.str().c_str()); + auto msg = folly::to("C++ exception (thrown as a char*) in '", exceptionLocation, "'\n\n", ex); + return Value::makeError(ctx, msg.c_str()); } catch (...) { - msg << "Unknown C++ Exception in '" << exceptionLocation << "'"; - return Value::makeError(ctx, msg.str().c_str()); + auto msg = folly::to("Unknown C++ exception in '", exceptionLocation, "'"); + return Value::makeError(ctx, msg.c_str()); } } @@ -210,7 +229,7 @@ JSValueRef translatePendingCppExceptionToJSError(JSContextRef ctx, JSObjectRef j auto functionName = Object(ctx, jsFunctionCause).getProperty("name").toString().str(); return translatePendingCppExceptionToJSError(ctx, functionName.c_str()); } catch (...) { - return Value::makeError(ctx, "Failed to get function name while handling exception"); + return Value::makeError(ctx, "Failed to translate native exception"); } } diff --git a/ReactCommon/jschelpers/JSCHelpers.h b/ReactCommon/jschelpers/JSCHelpers.h index 02a78d9107144a..c2e19a39db7f77 100644 --- a/ReactCommon/jschelpers/JSCHelpers.h +++ b/ReactCommon/jschelpers/JSCHelpers.h @@ -44,6 +44,17 @@ class JSException : public std::exception { void buildMessage(JSContextRef ctx, JSValueRef exn, JSStringRef sourceURL, const char* errorMsg); }; +namespace ExceptionHandling { + struct ExtractedEror { + std::string message; + // Stacktrace formatted like JS stack + // method@filename[:line[:column]] + std::string stack; + }; + using PlatformErrorExtractor = std::function; + extern PlatformErrorExtractor platformErrorExtractor; +} + using JSFunction = std::function; JSObjectRef makeFunction( diff --git a/ReactCommon/jschelpers/Value.cpp b/ReactCommon/jschelpers/Value.cpp index ab46577d992c28..fcf7c5d19b6dea 100644 --- a/ReactCommon/jschelpers/Value.cpp +++ b/ReactCommon/jschelpers/Value.cpp @@ -140,15 +140,27 @@ String Value::toString() const { return String::adopt(context(), jsStr); } -Value Value::makeError(JSContextRef ctx, const char *error) +Value Value::makeError(JSContextRef ctx, const char *error, const char *stack) { - JSValueRef exn; - JSValueRef args[] = { Value(ctx, String(ctx, error)) }; - JSObjectRef errorObj = JSC_JSObjectMakeError(ctx, 1, args, &exn); - if (!errorObj) { - throw JSException(ctx, exn, "Exception making error"); + auto errorMsg = Value(ctx, String(ctx, error)); + JSValueRef args[] = {errorMsg}; + if (stack) { + // Using this instead of JSObjectMakeError to actually get a stack property. + // MakeError only sets it stack when returning from the invoked function, so we + // can't extend it here. + auto errorConstructor = Object::getGlobalObject(ctx).getProperty("Error").asObject(); + auto jsError = errorConstructor.callAsConstructor({errorMsg}); + auto fullStack = std::string(stack) + jsError.getProperty("stack").toString().str(); + jsError.setProperty("stack", String(ctx, fullStack.c_str())); + return jsError; + } else { + JSValueRef exn; + JSObjectRef errorObj = JSC_JSObjectMakeError(ctx, 1, args, &exn); + if (!errorObj) { + throw JSException(ctx, exn, "Exception making error"); + } + return Value(ctx, errorObj); } - return Value(ctx, errorObj); } Object::operator Value() const { diff --git a/ReactCommon/jschelpers/Value.h b/ReactCommon/jschelpers/Value.h index b8f5603d2b86a0..b2b326fb297890 100644 --- a/ReactCommon/jschelpers/Value.h +++ b/ReactCommon/jschelpers/Value.h @@ -315,7 +315,9 @@ class Value : public noncopyable { RN_EXPORT String toString() const; - RN_EXPORT static Value makeError(JSContextRef ctx, const char *error); + // Create an error, optionally adding an additional number of lines to the stack. + // Stack must be empty or newline terminated. + RN_EXPORT static Value makeError(JSContextRef ctx, const char *error, const char *stack = nullptr); static Value makeNumber(JSContextRef ctx, double value) { return Value(ctx, JSC_JSValueMakeNumber(ctx, value));