Report Java stack from errors from sync native module calls

Reviewed By: mhorowitz

Differential Revision: D5069794

fbshipit-source-id: ede314034a2eb6b063a22dbd6e5d13c8ad66e20c

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<T>(array: Array<T>, 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);
 }
 

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<StackFrame> {
+function parseErrorStack(e: ExtendedError): Array<StackFrame> {
   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;
 }
 

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);
   }

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();
     }

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;
   }

ReactAndroid/src/main/jni/react/jni/OnLoad.cpp

@@ -152,13 +152,44 @@ static void logPerfMarker(const ReactMarker::ReactMarkerId markerId, const char*
   }
 }
 
+static ExceptionHandling::ExtractedEror extractJniError(const std::exception& ex, const char *context) {
+  auto jniEx = dynamic_cast<const jni::JniException *>(&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<std::string>(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<std::string>("Java exception in '", context, "'\n\n", jniEx->what());
+  return {.message = msg, .stack = stackStr.str()};
+}
+
 }
 
 extern "C" JNIEXPORT jint JNI_OnLoad(JavaVM* vm, void* reserved) {
   return initialize(vm, [] {
     gloginit::initialize();
     // Inject some behavior into react/
     ReactMarker::logTaggedMarker = logPerfMarker;
+    ExceptionHandling::platformErrorExtractor = extractJniError;
     JSCNativeHooks::loggingHook = nativeLoggingHook;
     JSCNativeHooks::nowHook = nativePerformanceNow;
     JSCNativeHooks::installPerfHooks = addNativePerfLoggingHooks;

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<std::string>("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<std::string>("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<std::string>("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");
   }
 }
 

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<ExtractedEror(const std::exception &ex, const char *context)>;
+  extern PlatformErrorExtractor platformErrorExtractor;
+}
+
 using JSFunction = std::function<JSValueRef(JSContextRef, JSObjectRef, size_t, const JSValueRef[])>;
 
 JSObjectRef makeFunction(

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 {

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));