From 8987d86718b534511d2d35ac4a74f9e1f4ba9fa0 Mon Sep 17 00:00:00 2001 From: Pieter De Baets Date: Wed, 15 Feb 2017 06:12:46 -0800 Subject: [PATCH] Fix error handling for nested exceptions Reviewed By: mhorowitz Differential Revision: D4551110 fbshipit-source-id: e593c1ef0dea27e95a387bcde6250aeb22d2e9cc --- React/CxxBridge/RCTCxxBridge.mm | 37 +---------------- React/CxxBridge/RCTMessageThread.mm | 34 +++------------- React/CxxModule/RCTCxxUtils.h | 7 ++++ React/CxxModule/RCTCxxUtils.mm | 59 ++++++++++++++++++++++++++++ ReactCommon/cxxreact/JSCExecutor.cpp | 4 +- ReactCommon/jschelpers/Value.cpp | 2 +- ReactCommon/jschelpers/Value.h | 16 ++++---- 7 files changed, 84 insertions(+), 75 deletions(-) diff --git a/React/CxxBridge/RCTCxxBridge.mm b/React/CxxBridge/RCTCxxBridge.mm index 7ecb7f37f..8fc006362 100644 --- a/React/CxxBridge/RCTCxxBridge.mm +++ b/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 { 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); diff --git a/React/CxxBridge/RCTMessageThread.mm b/React/CxxBridge/RCTMessageThread.mm index 205ece517..409c003b3 100644 --- a/React/CxxBridge/RCTMessageThread.mm +++ b/React/CxxBridge/RCTMessageThread.mm @@ -12,7 +12,8 @@ #include #include -#include +#import +#import #include // A note about the implementation: This class is not used @@ -58,34 +59,9 @@ void RCTMessageThread::runSync(std::function func) { } void RCTMessageThread::tryFunc(const std::function& 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); } } diff --git a/React/CxxModule/RCTCxxUtils.h b/React/CxxModule/RCTCxxUtils.h index f63c53676..4755e3678 100644 --- a/React/CxxModule/RCTCxxUtils.h +++ b/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& func); + +} } diff --git a/React/CxxModule/RCTCxxUtils.mm b/React/CxxModule/RCTCxxUtils.mm index bba4cfca2..c2475defc 100644 --- a/React/CxxModule/RCTCxxUtils.mm +++ b/React/CxxModule/RCTCxxUtils.mm @@ -10,7 +10,11 @@ #import "RCTCxxUtils.h" #import +#import +#include + +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(&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& 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"); + } +} + +} } diff --git a/ReactCommon/cxxreact/JSCExecutor.cpp b/ReactCommon/cxxreact/JSCExecutor.cpp index 61f2d83f6..7f1e6c1f5 100644 --- a/ReactCommon/cxxreact/JSCExecutor.cpp +++ b/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("Error invoking callback.", callbackId))); + std::runtime_error(folly::to("Error invoking callback ", callbackId))); } }(); diff --git a/ReactCommon/jschelpers/Value.cpp b/ReactCommon/jschelpers/Value.cpp index 12b46dc25..b228da367 100644 --- a/ReactCommon/jschelpers/Value.cpp +++ b/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); } diff --git a/ReactCommon/jschelpers/Value.h b/ReactCommon/jschelpers/Value.h index 2a890d37e..f2879bd1d 100644 --- a/ReactCommon/jschelpers/Value.h +++ b/ReactCommon/jschelpers/Value.h @@ -8,10 +8,9 @@ #include #include - #include -#include #include +#include 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_; };