Fix error handling for nested exceptions

Reviewed By: mhorowitz

Differential Revision: D4551110

fbshipit-source-id: e593c1ef0dea27e95a387bcde6250aeb22d2e9cc

React/CxxBridge/RCTCxxBridge.mm

@@ -342,40 +342,6 @@ struct RCTInstanceCallback : public InstanceCallback {
   }
 }
 
-static NSError *tryAndReturnError(dispatch_block_t block) {
-  // TODO #10487027: This is mostly duplicated in RCTMessageThread.
-  try {
-    @try {
-      block();
-      return nil;
-    }
-    @catch (NSException *exception) {
-      NSString *message =
-        [NSString stringWithFormat:@"Exception '%@' was thrown from JS thread", exception];
-      return RCTErrorWithMessage(message);
-    }
-    @catch (id exception) {
-      // This will catch any other ObjC exception, but no C++ exceptions
-      return RCTErrorWithMessage(@"non-std ObjC Exception");
-    }
-  } catch (const JSException &ex) {
-    // This is a special case.  We want to extract the stack
-    // information and pass it to the redbox.  This will lose the C++
-    // stack, but it's of limited value.
-    NSDictionary *errorInfo = @{
-      RCTJSRawStackTraceKey: @(ex.getStack().c_str()),
-      NSLocalizedDescriptionKey: [@"Unhandled JS Exception: " stringByAppendingString:@(ex.what())]
-    };
-    return [NSError errorWithDomain:RCTErrorDomain code:1 userInfo:errorInfo];
-  } catch (const std::exception &ex) {
-    return RCTErrorWithMessage(@(ex.what()));
-  } catch (...) {
-    // On a 64-bit platform, this would catch ObjC exceptions, too, but not on
-    // 32-bit platforms, so we catch those with id exceptions above.
-    return RCTErrorWithMessage(@"non-std C++ exception");
-  }
-}
-
 - (void)_tryAndHandleError:(dispatch_block_t)block
 {
   NSError *error = tryAndReturnError(block);
@@ -1325,9 +1291,8 @@ struct ValueEncoder<NSArray *> {
     return nil;
   }
 
-  __block JSValue *ret = nil;
-
   RCT_PROFILE_BEGIN_EVENT(0, @"callFunctionOnModule", (@{ @"module": module, @"method": method }));
+  __block JSValue *ret = nil;
   NSError *errorObj = tryAndReturnError(^{
     Value result = self->_reactInstance->callFunctionSync(
       [module UTF8String], [method UTF8String], arguments);

React/CxxBridge/RCTMessageThread.mm

@@ -12,7 +12,8 @@
 #include <condition_variable>
 #include <mutex>
 
-#include <React/RCTUtils.h>
+#import <React/RCTCxxUtils.h>
+#import <React/RCTUtils.h>
 #include <jschelpers/JSCHelpers.h>
 
 // A note about the implementation: This class is not used
@@ -58,34 +59,9 @@ void RCTMessageThread::runSync(std::function<void()> func) {
 }
 
 void RCTMessageThread::tryFunc(const std::function<void()>& func) {
-  try {
-    @try {
-      func();
-    }
-    @catch (NSException *exception) {
-      NSString *message =
-        [NSString stringWithFormat:@"Exception '%@' was thrown from JS thread", exception];
-      m_errorBlock(RCTErrorWithMessage(message));
-    }
-    @catch (id exception) {
-      // This will catch any other ObjC exception, but no C++ exceptions
-      m_errorBlock(RCTErrorWithMessage(@"non-std ObjC Exception"));
-    }
-  } catch (const facebook::react::JSException &ex) {
-    // This is a special case.  We want to extract the stack
-    // information and pass it to the redbox.  This will lose the C++
-    // stack, but it's of limited value.
-    NSDictionary *errorInfo = @{
-      RCTJSRawStackTraceKey: @(ex.getStack().c_str()),
-      NSLocalizedDescriptionKey: [@"Unhandled JS Exception: " stringByAppendingString:@(ex.what())]
-    };
-    m_errorBlock([NSError errorWithDomain:RCTErrorDomain code:1 userInfo:errorInfo]);
-  } catch (const std::exception &ex) {
-    m_errorBlock(RCTErrorWithMessage(@(ex.what())));
-  } catch (...) {
-    // On a 64-bit platform, this would catch ObjC exceptions, too, but not on
-    // 32-bit platforms, so we catch those with id exceptions above.
-    m_errorBlock(RCTErrorWithMessage(@"non-std C++ exception"));
+  NSError *error = tryAndReturnError(func);
+  if (error) {
+    m_errorBlock(error);
   }
 }
 

React/CxxModule/RCTCxxUtils.h

@@ -17,3 +17,10 @@ id RCTConvertFollyDynamic(const folly::dynamic &dyn);
 + (folly::dynamic)folly_dynamic:(id)json;
 
 @end
+
+namespace facebook {
+namespace react {
+
+NSError *tryAndReturnError(const std::function<void()>& func);
+
+} }

React/CxxModule/RCTCxxUtils.mm

@@ -10,7 +10,11 @@
 #import "RCTCxxUtils.h"
 
 #import <React/RCTFollyConvert.h>
+#import <React/RCTUtils.h>
 
+#include <jschelpers/Value.h>
+
+using namespace facebook::react;
 using namespace react::CxxUtils;
 
 id RCTConvertFollyDynamic(const folly::dynamic &dyn) {
@@ -33,3 +37,58 @@ id RCTConvertFollyDynamic(const folly::dynamic &dyn) {
 }
 
 @end
+
+namespace facebook {
+namespace react {
+
+static NSError *errorWithException(const std::exception& e)
+{
+  NSString *msg = @(e.what());
+  NSMutableDictionary *errorInfo = [NSMutableDictionary dictionary];
+
+  const JSException *jsException = dynamic_cast<const JSException*>(&e);
+  if (jsException) {
+    errorInfo[RCTJSRawStackTraceKey] = @(jsException->getStack().c_str());
+    msg = [@"Unhandled JS Exception: " stringByAppendingString:msg];
+  }
+
+  NSError *nestedError;
+  try {
+    std::rethrow_if_nested(e);
+  } catch(const std::exception& e) {
+    nestedError = errorWithException(e);
+  } catch(...) {}
+
+  if (nestedError) {
+    msg = [NSString stringWithFormat:@"%@\n\n%@", msg, [nestedError localizedDescription]];
+  }
+
+  errorInfo[NSLocalizedDescriptionKey] = msg;
+  return [NSError errorWithDomain:RCTErrorDomain code:1 userInfo:errorInfo];
+}
+
+NSError *tryAndReturnError(const std::function<void()>& func) {
+  try {
+    @try {
+      func();
+      return nil;
+    }
+    @catch (NSException *exception) {
+      NSString *message =
+      [NSString stringWithFormat:@"Exception '%@' was thrown from JS thread", exception];
+      return RCTErrorWithMessage(message);
+    }
+    @catch (id exception) {
+      // This will catch any other ObjC exception, but no C++ exceptions
+      return RCTErrorWithMessage(@"non-std ObjC Exception");
+    }
+  } catch (const std::exception &ex) {
+    return errorWithException(ex);
+  } catch (...) {
+    // On a 64-bit platform, this would catch ObjC exceptions, too, but not on
+    // 32-bit platforms, so we catch those with id exceptions above.
+    return RCTErrorWithMessage(@"non-std C++ exception");
+  }
+}
+
+} }

ReactCommon/cxxreact/JSCExecutor.cpp

@@ -525,7 +525,7 @@ void JSCExecutor::callFunction(const std::string& moduleId, const std::string& m
       });
     } catch (...) {
       std::throw_with_nested(
-        std::runtime_error("Error calling function: " + moduleId + ":" + methodId));
+        std::runtime_error("Error calling " + moduleId + "." + methodId));
     }
   }();
 
@@ -542,7 +542,7 @@ void JSCExecutor::invokeCallback(const double callbackId, const folly::dynamic&
       });
     } catch (...) {
       std::throw_with_nested(
-        std::runtime_error(folly::to<std::string>("Error invoking callback.", callbackId)));
+        std::runtime_error(folly::to<std::string>("Error invoking callback ", callbackId)));
     }
   }();
 

ReactCommon/jschelpers/Value.cpp

@@ -152,7 +152,7 @@ Value Value::makeError(JSContextRef ctx, const char *error)
   JSObjectRef errorObj = JSC_JSObjectMakeError(ctx, 1, args, &exn);
   if (!errorObj) {
     std::string exceptionText = Value(ctx, exn).toString().str();
-    throwJSExecutionException("Exception calling object as function: %s", exceptionText.c_str());
+    throwJSExecutionException("%s", exceptionText.c_str());
   }
   return Value(ctx, errorObj);
 }

ReactCommon/jschelpers/Value.h

@@ -8,10 +8,9 @@
 #include <vector>
 
 #include <folly/dynamic.h>
-
 #include <jschelpers/JavaScriptCore.h>
-#include <jschelpers/noncopyable.h>
 #include <jschelpers/Unicode.h>
+#include <jschelpers/noncopyable.h>
 
 namespace facebook {
 namespace react {
@@ -19,21 +18,24 @@ namespace react {
 class Value;
 class Context;
 
-class JSException : public std::runtime_error {
+class JSException : public std::exception {
 public:
   explicit JSException(const char* msg)
-    : std::runtime_error(msg)
-    , stack_("") {}
+    : msg_(msg), stack_("") {}
 
   JSException(const char* msg, const char* stack)
-    : std::runtime_error(msg)
-    , stack_(stack) {}
+    : msg_(msg), stack_(stack) {}
 
   const std::string& getStack() const {
     return stack_;
   }
 
+  virtual const char* what() const noexcept override {
+    return msg_.c_str();
+  }
+
 private:
+  std::string msg_;
   std::string stack_;
 };