From 097525df332b1d539efe48b0aa8eb1a16f4c0874 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Thu, 27 Apr 2023 18:08:35 +0200 Subject: [PATCH 01/55] Add iOS reject promise if turbo module method throws exception --- .../react-native/React/Base/RCTBridgeModule.h | 2 + .../platform/ios/ReactCommon/RCTTurboModule.h | 5 +- .../ios/ReactCommon/RCTTurboModule.mm | 64 +++++++++++++++++-- .../TurboModule/SampleTurboModuleExample.js | 10 ++- 4 files changed, 66 insertions(+), 15 deletions(-) diff --git a/packages/react-native/React/Base/RCTBridgeModule.h b/packages/react-native/React/Base/RCTBridgeModule.h index cc429c410f58..fa8cb3748a8b 100644 --- a/packages/react-native/React/Base/RCTBridgeModule.h +++ b/packages/react-native/React/Base/RCTBridgeModule.h @@ -46,6 +46,8 @@ typedef void (^RCTPromiseResolveBlock)(id result); */ typedef void (^RCTPromiseRejectBlock)(NSString *code, NSString *message, NSError *error); +typedef void (^RCTInternalPromiseRejectBlock)(NSException *exception); + RCT_EXTERN_C_BEGIN typedef struct RCTMethodInfo { diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.h b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.h index b971372e9b83..dda0886262f3 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.h +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.h @@ -86,9 +86,10 @@ class JSI_EXPORT ObjCTurboModule : public TurboModule { TurboModuleMethodValueKind returnType, const char *methodName, NSInvocation *inv, - NSMutableArray *retainedObjectsForInvocation); + NSMutableArray *retainedObjectsForInvocation, + RCTInternalPromiseRejectBlock optionalInternalRejectBlock); - using PromiseInvocationBlock = void (^)(RCTPromiseResolveBlock resolveWrapper, RCTPromiseRejectBlock rejectWrapper); + using PromiseInvocationBlock = void (^)(RCTPromiseResolveBlock resolveWrapper, RCTPromiseRejectBlock rejectWrapper, RCTInternalPromiseRejectBlock internalRejectWrapper); jsi::Value createPromise(jsi::Runtime &runtime, std::string methodName, PromiseInvocationBlock invoke); }; diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm index 1bf1658dcf20..8246ae2572d6 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm @@ -356,8 +356,50 @@ static int32_t getUniqueId() rejectWasCalled = YES; }; + + RCTInternalPromiseRejectBlock internalRejectBlock = ^(NSException *exception) { + if (resolveWasCalled) { + RCTLogError(@"%s: Tried to reject a promise after it's already been resolved.", moduleMethod.c_str()); + return; + } + + if (rejectWasCalled) { + RCTLogError(@"%s: Tried to reject a promise more than once.", moduleMethod.c_str()); + return; + } + + auto strongResolveWrapper = weakResolveWrapper.lock(); + auto strongRejectWrapper = weakRejectWrapper.lock(); + if (!strongResolveWrapper || !strongRejectWrapper) { + return; + } - invokeCopy(resolveBlock, rejectBlock); + NSDictionary *jsError = @{ + @"name": exception.name, + @"message": exception.reason, + @"stackSymbols": exception.callStackSymbols, + @"stackReturnAddresses": exception.callStackReturnAddresses, + }; + strongRejectWrapper->jsInvoker().invokeAsync([weakResolveWrapper, weakRejectWrapper, jsError, blockGuard]() { + auto strongResolveWrapper2 = weakResolveWrapper.lock(); + auto strongRejectWrapper2 = weakRejectWrapper.lock(); + if (!strongResolveWrapper2 || !strongRejectWrapper2) { + return; + } + + jsi::Runtime &rt = strongRejectWrapper2->runtime(); + jsi::Value arg = convertNSDictionaryToJSIObject(rt, jsError); + strongRejectWrapper2->callback().call(rt, arg); + + strongResolveWrapper2->destroy(); + strongRejectWrapper2->destroy(); + (void)blockGuard; + }); + + rejectWasCalled = YES; + }; + + invokeCopy(resolveBlock, rejectBlock, internalRejectBlock); return jsi::Value::undefined(); }); @@ -378,7 +420,8 @@ static int32_t getUniqueId() TurboModuleMethodValueKind returnType, const char *methodName, NSInvocation *inv, - NSMutableArray *retainedObjectsForInvocation) + NSMutableArray *retainedObjectsForInvocation, + RCTInternalPromiseRejectBlock optionalInternalRejectBlock) { __block id result; jsi::Runtime *rt = &runtime; @@ -403,7 +446,13 @@ static int32_t getUniqueId() @try { [inv invokeWithTarget:strongModule]; } @catch (NSException *exception) { - throw convertNSExceptionToJSError(runtime, exception); + if (wasMethodSync) { + throw convertNSExceptionToJSError(runtime, exception); + } else { + if (optionalInternalRejectBlock != nil) { + optionalInternalRejectBlock(exception); + } + } } @finally { [retainedObjectsForInvocation removeAllObjects]; } @@ -695,18 +744,19 @@ static int32_t getUniqueId() ? createPromise( runtime, methodNameStr, - ^(RCTPromiseResolveBlock resolveBlock, RCTPromiseRejectBlock rejectBlock) { + ^(RCTPromiseResolveBlock resolveBlock, RCTPromiseRejectBlock rejectBlock, RCTInternalPromiseRejectBlock internalRejectBlock) { RCTPromiseResolveBlock resolveCopy = [resolveBlock copy]; RCTPromiseRejectBlock rejectCopy = [rejectBlock copy]; - + RCTInternalPromiseRejectBlock internalRejectCopy = [internalRejectBlock copy]; + [inv setArgument:(void *)&resolveCopy atIndex:count + 2]; [inv setArgument:(void *)&rejectCopy atIndex:count + 3]; [retainedObjectsForInvocation addObject:resolveCopy]; [retainedObjectsForInvocation addObject:rejectCopy]; // The return type becomes void in the ObjC side. - performMethodInvocation(runtime, VoidKind, methodName, inv, retainedObjectsForInvocation); + performMethodInvocation(runtime, VoidKind, methodName, inv, retainedObjectsForInvocation, internalRejectCopy); }) - : performMethodInvocation(runtime, returnType, methodName, inv, retainedObjectsForInvocation); + : performMethodInvocation(runtime, returnType, methodName, inv, retainedObjectsForInvocation, nil); if (isMethodSync(returnType)) { TurboModulePerfLogger::syncMethodCallEnd(moduleName, methodName); diff --git a/packages/rn-tester/js/examples/TurboModule/SampleTurboModuleExample.js b/packages/rn-tester/js/examples/TurboModule/SampleTurboModuleExample.js index 57a2be7f5824..b061e429276f 100644 --- a/packages/rn-tester/js/examples/TurboModule/SampleTurboModuleExample.js +++ b/packages/rn-tester/js/examples/TurboModule/SampleTurboModuleExample.js @@ -92,12 +92,10 @@ class SampleTurboModuleExample extends React.Component<{||}, State> { } }, promiseThrows: () => { - try { - // $FlowFixMe[unused-promise] - NativeSampleTurboModule.promiseThrows?.(); - } catch (e) { - return e.message; - } + // $FlowFixMe[unused-promise] + NativeSampleTurboModule.promiseThrows?.() + .then(() => {}) + .catch(e => this._setResult('promiseThrows', e.message)) }, voidFuncAssert: () => { try { From 5892e50e035303133583adc89a4aa8520a9bde35 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Mon, 15 May 2023 20:45:07 +0200 Subject: [PATCH 02/55] Add Android reject async func on error --- .../android/ReactCommon/JavaTurboModule.cpp | 17 ++++++++++++++++- .../TurboModule/SampleTurboModuleExample.js | 8 ++++---- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp index 477d144773b8..445597356252 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp @@ -812,6 +812,9 @@ jsi::Value JavaTurboModule::invokeJavaMethod( promiseConstructorArgs[1].getObject(runtime).getFunction( runtime); + auto rejectWeakWrapper = + CallbackWrapper::createWeak(std::move(rejectJSIFn), runtime, jsInvoker_); + auto resolve = createJavaCallbackFromJSIFunction( std::move(resolveJSIFn), runtime, jsInvoker_) .release(); @@ -843,12 +846,14 @@ jsi::Value JavaTurboModule::invokeJavaMethod( nativeInvoker_->invokeAsync( [jargs, + rejectWeakWrapper = std::move(rejectWeakWrapper), globalRefs, methodID, instance_ = jni::make_weak(instance_), moduleNameStr, methodNameStr, id = getUniqueId()]() mutable -> void { + auto rejectStrongWrapper = rejectWeakWrapper.lock(); auto instance = instance_.lockLocal(); if (!instance) { @@ -871,7 +876,17 @@ jsi::Value JavaTurboModule::invokeJavaMethod( } catch (...) { TMPL::asyncMethodCallExecutionFail( moduleName, methodName, id); - throw; + if (rejectStrongWrapper) { + auto exception = std::current_exception(); + auto throwable = jni::getJavaExceptionForCppException(exception); + auto jsError = convertThrowableToJSError(rejectStrongWrapper->runtime(), throwable); + rejectStrongWrapper->callback().call( + rejectStrongWrapper->runtime(), + jsError.value(), + 1); + } else { + throw; + } } for (auto globalRef : globalRefs) { diff --git a/packages/rn-tester/js/examples/TurboModule/SampleTurboModuleExample.js b/packages/rn-tester/js/examples/TurboModule/SampleTurboModuleExample.js index b061e429276f..b79c96eba751 100644 --- a/packages/rn-tester/js/examples/TurboModule/SampleTurboModuleExample.js +++ b/packages/rn-tester/js/examples/TurboModule/SampleTurboModuleExample.js @@ -92,10 +92,9 @@ class SampleTurboModuleExample extends React.Component<{||}, State> { } }, promiseThrows: () => { - // $FlowFixMe[unused-promise] NativeSampleTurboModule.promiseThrows?.() .then(() => {}) - .catch(e => this._setResult('promiseThrows', e.message)) + .catch(e => this._setResult('promiseThrows', e.message)); }, voidFuncAssert: () => { try { @@ -113,8 +112,9 @@ class SampleTurboModuleExample extends React.Component<{||}, State> { }, promiseAssert: () => { try { - // $FlowFixMe[unused-promise] - NativeSampleTurboModule.promiseAssert?.(); + NativeSampleTurboModule.promiseAssert?.() + .then(() => {}) + .catch(e => this._setResult('promiseAssert', e.message)); } catch (e) { return e.message; } From 97606c94a30579ce3a36b388e2a376d005f8c22d Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Tue, 16 May 2023 17:15:14 +0200 Subject: [PATCH 03/55] add(ios): js stack to rejection caused by native exception or native rejection --- packages/react-native/React/Base/RCTUtils.m | 2 +- .../ios/ReactCommon/RCTTurboModule.mm | 80 +++++++++---------- 2 files changed, 40 insertions(+), 42 deletions(-) diff --git a/packages/react-native/React/Base/RCTUtils.m b/packages/react-native/React/Base/RCTUtils.m index 4e862e9c6c3e..1d8cb9fc97fc 100644 --- a/packages/react-native/React/Base/RCTUtils.m +++ b/packages/react-native/React/Base/RCTUtils.m @@ -489,7 +489,7 @@ BOOL RCTClassOverridesInstanceMethod(Class cls, SEL selector) NSArray *stackTrace = [NSThread callStackSymbols]; NSMutableDictionary *userInfo; NSMutableDictionary *errorInfo = [NSMutableDictionary dictionaryWithObject:stackTrace - forKey:@"nativeStackIOS"]; + forKey:@"stackSymbols"]; if (error) { errorMessage = error.localizedDescription ?: @"Unknown error from a native module"; diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm index c7f8e5a80fa8..0f2947156dcc 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm @@ -239,6 +239,23 @@ static int32_t getUniqueId() return {runtime, std::move(error)}; } +/** + * Creates JSErrorValue with given stack trace. + */ +static jsi::Value createRejectJSErrorValue(jsi::Runtime &runtime, NSDictionary *reason, std::string jsStack) { + jsi::Object cause = convertNSDictionaryToJSIObject(runtime, reason); + jsi::Value error; + NSString *message = reason[@"message"]; + if (message) { + error = createJSRuntimeError(runtime, "Exception in HostFunction: " + std::string([message UTF8String])); + } else { + error = createJSRuntimeError(runtime, "Exception in HostFunction: "); + } + error.asObject(runtime).setProperty(runtime, "stack", jsStack); + error.asObject(runtime).setProperty(runtime, "cause", std::move(cause)); + return error; +} + } jsi::Value ObjCTurboModule::createPromise(jsi::Runtime &runtime, std::string methodName, PromiseInvocationBlock invoke) @@ -248,6 +265,14 @@ static int32_t getUniqueId() } jsi::Function Promise = runtime.global().getPropertyAsFunction(runtime, "Promise"); + std::string jsStack; + try { + jsStack = createJSRuntimeError(runtime, "") + .asObject(runtime).getProperty(runtime, "stack").asString(runtime).utf8(runtime); + } catch (...) { + // Stack unavailable + } + std::string moduleName = name_; // Note: the passed invoke() block is not retained by default, so let's retain it here to help keep it longer. @@ -257,7 +282,7 @@ static int32_t getUniqueId() runtime, jsi::PropNameID::forAscii(runtime, "fn"), 2, - [invokeCopy, jsInvoker = jsInvoker_, moduleName, methodName]( + [invokeCopy, jsInvoker = jsInvoker_, moduleName, methodName, jsStack]( jsi::Runtime &rt, const jsi::Value &thisVal, const jsi::Value *args, size_t count) { std::string moduleMethod = moduleName + "." + methodName + "()"; @@ -324,7 +349,7 @@ static int32_t getUniqueId() resolveWasCalled = YES; }; - RCTPromiseRejectBlock rejectBlock = ^(NSString *code, NSString *message, NSError *error) { + auto handlePromiseRejection = ^(NSDictionary *reason) { if (resolveWasCalled) { RCTLogError(@"%s: Tried to reject a promise after it's already been resolved.", moduleMethod.c_str()); return; @@ -341,8 +366,7 @@ static int32_t getUniqueId() return; } - NSDictionary *jsError = RCTJSErrorFromCodeMessageAndNSError(code, message, error); - strongRejectWrapper->jsInvoker().invokeAsync([weakResolveWrapper, weakRejectWrapper, jsError, blockGuard]() { + strongRejectWrapper->jsInvoker().invokeAsync([weakResolveWrapper, weakRejectWrapper, blockGuard, reason, jsStack]() { auto strongResolveWrapper2 = weakResolveWrapper.lock(); auto strongRejectWrapper2 = weakRejectWrapper.lock(); if (!strongResolveWrapper2 || !strongRejectWrapper2) { @@ -350,8 +374,7 @@ static int32_t getUniqueId() } jsi::Runtime &rt = strongRejectWrapper2->runtime(); - jsi::Value arg = convertNSDictionaryToJSIObject(rt, jsError); - strongRejectWrapper2->callback().call(rt, arg); + strongRejectWrapper2->callback().call(rt, createRejectJSErrorValue(rt, reason, jsStack)); strongResolveWrapper2->destroy(); strongRejectWrapper2->destroy(); @@ -360,47 +383,19 @@ static int32_t getUniqueId() rejectWasCalled = YES; }; - - RCTInternalPromiseRejectBlock internalRejectBlock = ^(NSException *exception) { - if (resolveWasCalled) { - RCTLogError(@"%s: Tried to reject a promise after it's already been resolved.", moduleMethod.c_str()); - return; - } - - if (rejectWasCalled) { - RCTLogError(@"%s: Tried to reject a promise more than once.", moduleMethod.c_str()); - return; - } - auto strongResolveWrapper = weakResolveWrapper.lock(); - auto strongRejectWrapper = weakRejectWrapper.lock(); - if (!strongResolveWrapper || !strongRejectWrapper) { - return; - } + RCTPromiseRejectBlock rejectBlock = ^(NSString *code, NSString *message, NSError *error) { + NSDictionary *reason = RCTJSErrorFromCodeMessageAndNSError(code, message, error); + handlePromiseRejection(reason); + }; - NSDictionary *jsError = @{ + RCTInternalPromiseRejectBlock internalRejectBlock = ^(NSException *exception) { + handlePromiseRejection(@{ @"name": exception.name, @"message": exception.reason, @"stackSymbols": exception.callStackSymbols, @"stackReturnAddresses": exception.callStackReturnAddresses, - }; - strongRejectWrapper->jsInvoker().invokeAsync([weakResolveWrapper, weakRejectWrapper, jsError, blockGuard]() { - auto strongResolveWrapper2 = weakResolveWrapper.lock(); - auto strongRejectWrapper2 = weakRejectWrapper.lock(); - if (!strongResolveWrapper2 || !strongRejectWrapper2) { - return; - } - - jsi::Runtime &rt = strongRejectWrapper2->runtime(); - jsi::Value arg = convertNSDictionaryToJSIObject(rt, jsError); - strongRejectWrapper2->callback().call(rt, arg); - - strongResolveWrapper2->destroy(); - strongRejectWrapper2->destroy(); - (void)blockGuard; }); - - rejectWasCalled = YES; }; invokeCopy(resolveBlock, rejectBlock, internalRejectBlock); @@ -448,11 +443,14 @@ static int32_t getUniqueId() @try { [inv invokeWithTarget:strongModule]; } @catch (NSException *exception) { - if (wasMethodSync) { + if (isSync) { throw convertNSExceptionToJSError(runtime, exception); } else { if (optionalInternalRejectBlock != nil) { optionalInternalRejectBlock(exception); + } else { + // JSFunction returning void + throw; } } } @finally { From 2eb0a460f822a8ccaf9906bd3f3e879fb3548e3b Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Wed, 17 May 2023 17:25:51 +0200 Subject: [PATCH 04/55] Fix android promise reject and add JS stack trace --- .../android/ReactCommon/JavaTurboModule.cpp | 82 +++++++++++++------ 1 file changed, 57 insertions(+), 25 deletions(-) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp index 445597356252..19e2059801b7 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp @@ -61,10 +61,19 @@ struct JNIArgs { std::vector globalRefs_; }; +jsi::Value createJSRuntimeError( + jsi::Runtime &runtime, + const std::string &message) { + return runtime.global() + .getPropertyAsFunction(runtime, "Error") + .call(runtime, message); +} + jni::local_ref createJavaCallbackFromJSIFunction( jsi::Function &&function, jsi::Runtime &rt, - const std::shared_ptr &jsInvoker) { + const std::shared_ptr &jsInvoker, + const std::string& jsStack) { auto weakWrapper = CallbackWrapper::createWeak(std::move(function), rt, jsInvoker); @@ -80,7 +89,8 @@ jni::local_ref createJavaCallbackFromJSIFunction( return JCxxCallbackImpl::newObjectCxxArgs( [weakWrapper = std::move(weakWrapper), callbackWrapperOwner = std::move(callbackWrapperOwner), - wrapperWasCalled = false](folly::dynamic responses) mutable { + wrapperWasCalled = false, + jsStack](folly::dynamic responses) mutable { if (wrapperWasCalled) { throw std::runtime_error( "Callback arg cannot be called more than once"); @@ -94,23 +104,40 @@ jni::local_ref createJavaCallbackFromJSIFunction( strongWrapper->jsInvoker().invokeAsync( [weakWrapper = std::move(weakWrapper), callbackWrapperOwner = std::move(callbackWrapperOwner), - responses = std::move(responses)]() { + responses = std::move(responses), + jsStack]() { auto strongWrapper2 = weakWrapper.lock(); if (!strongWrapper2) { return; } + jsi::Runtime &rt = strongWrapper2->runtime(); std::vector args; args.reserve(responses.size()); for (const auto &val : responses) { args.emplace_back( - jsi::valueFromDynamic(strongWrapper2->runtime(), val)); + jsi::valueFromDynamic(rt, val)); } - strongWrapper2->callback().call( - strongWrapper2->runtime(), - (const jsi::Value *)args.data(), - args.size()); + if (jsStack.empty()) { + strongWrapper2->callback().call( + strongWrapper2->runtime(), + (const jsi::Value *)args.data(), + args.size()); + } else { + std::string message; + const jsi::Value* cause = args.data(); + if (cause->isObject() && cause->asObject(rt).getProperty(rt, "message").isString()) { + message = cause->asObject(rt).getProperty(rt, "message").asString(rt).utf8(rt); + } else { + message = ""; + } + + jsi::Value error = createJSRuntimeError(rt, "Exception in HostFunction: " + message); + error.asObject(rt).setProperty(rt, "cause", *cause); + error.asObject(rt).setProperty(rt, "stack", jsStack); + strongWrapper2->callback().call(strongWrapper2->runtime(), error); + } }); wrapperWasCalled = true; @@ -355,7 +382,7 @@ JNIArgs convertJSIArgsToJNIArgs( } jsi::Function fn = arg->getObject(rt).getFunction(rt); jarg->l = makeGlobalIfNecessary( - createJavaCallbackFromJSIFunction(std::move(fn), rt, jsInvoker) + createJavaCallbackFromJSIFunction(std::move(fn), rt, jsInvoker, "") .release()); } else if (type == "Lcom/facebook/react/bridge/ReadableArray;") { if (!(arg->isObject() && arg->getObject(rt).isArray(rt))) { @@ -400,14 +427,6 @@ jsi::Value convertFromJMapToValue(JNIEnv *env, jsi::Runtime &rt, jobject arg) { return jsi::valueFromDynamic(rt, result->cthis()->consume()); } -jsi::Value createJSRuntimeError( - jsi::Runtime &runtime, - const std::string &message) { - return runtime.global() - .getPropertyAsFunction(runtime, "Error") - .call(runtime, message); -} - /** * Creates JSError with current JS runtime stack and Throwable stack trace. */ @@ -805,21 +824,31 @@ jsi::Value JavaTurboModule::invokeJavaMethod( throw std::invalid_argument("Promise fn arg count must be 2"); } + std::string jsStack; + try { + jsStack = createJSRuntimeError(runtime, "") + .asObject(runtime).getProperty(runtime, "stack").asString(runtime).utf8(runtime); + } catch (...) { + // Stack unavailable + } + jsi::Function resolveJSIFn = promiseConstructorArgs[0].getObject(runtime).getFunction( runtime); jsi::Function rejectJSIFn = promiseConstructorArgs[1].getObject(runtime).getFunction( runtime); - + jsi::Function rejectInternalJSIFn = + promiseConstructorArgs[1].getObject(runtime).getFunction( + runtime); auto rejectWeakWrapper = - CallbackWrapper::createWeak(std::move(rejectJSIFn), runtime, jsInvoker_); + CallbackWrapper::createWeak(std::move(rejectInternalJSIFn), runtime, jsInvoker_); auto resolve = createJavaCallbackFromJSIFunction( - std::move(resolveJSIFn), runtime, jsInvoker_) + std::move(resolveJSIFn), runtime, jsInvoker_, "") .release(); auto reject = createJavaCallbackFromJSIFunction( - std::move(rejectJSIFn), runtime, jsInvoker_) + std::move(rejectJSIFn), runtime, jsInvoker_, jsStack) .release(); jclass jPromiseImpl = @@ -847,6 +876,7 @@ jsi::Value JavaTurboModule::invokeJavaMethod( nativeInvoker_->invokeAsync( [jargs, rejectWeakWrapper = std::move(rejectWeakWrapper), + jsStack, globalRefs, methodID, instance_ = jni::make_weak(instance_), @@ -878,12 +908,14 @@ jsi::Value JavaTurboModule::invokeJavaMethod( moduleName, methodName, id); if (rejectStrongWrapper) { auto exception = std::current_exception(); + jsi::Runtime &runtime = rejectStrongWrapper->runtime(); auto throwable = jni::getJavaExceptionForCppException(exception); - auto jsError = convertThrowableToJSError(rejectStrongWrapper->runtime(), throwable); + auto jsError = convertThrowableToJSError(runtime, throwable); + jsError.value().asObject(runtime).setProperty(runtime, "stack", jsStack); rejectStrongWrapper->callback().call( - rejectStrongWrapper->runtime(), - jsError.value(), - 1); + rejectStrongWrapper->runtime(), + jsError.value(), + 1); } else { throw; } From a836691e7c46b03c8e772ee234b51f618940583c Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Thu, 18 May 2023 16:29:06 +0200 Subject: [PATCH 05/55] Refactor java callback --- .../android/ReactCommon/JavaTurboModule.cpp | 208 ++++++++++-------- .../ios/ReactCommon/RCTTurboModule.mm | 7 +- 2 files changed, 117 insertions(+), 98 deletions(-) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp index 19e2059801b7..3900ae8faa49 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp @@ -69,79 +69,101 @@ jsi::Value createJSRuntimeError( .call(runtime, message); } +jni::local_ref createJavaCallbackFromJSIFunction( + jsi::Function &&function, + jsi::Runtime &rt, + const std::shared_ptr &jsInvoker, + std::function&, const std::vector&)>&& executor) { + auto weakWrapper = + CallbackWrapper::createWeak(std::move(function), rt, jsInvoker); + + // This needs to be a shared_ptr because: + // 1. It cannot be unique_ptr. std::function is copyable but unique_ptr is + // not. + // 2. It cannot be weak_ptr since we need this object to live on. + // 3. It cannot be a value, because that would be deleted as soon as this + // function returns. + auto callbackWrapperOwner = + std::make_shared(weakWrapper); + + return JCxxCallbackImpl::newObjectCxxArgs( + [weakWrapper = std::move(weakWrapper), + callbackWrapperOwner = std::move(callbackWrapperOwner), + wrapperWasCalled = false, + executor = std::move(executor)](folly::dynamic responses) mutable { + if (wrapperWasCalled) { + throw std::runtime_error( + "Callback arg cannot be called more than once"); + } + + auto strongWrapper = weakWrapper.lock(); + if (!strongWrapper) { + return; + } + + strongWrapper->jsInvoker().invokeAsync( + [weakWrapper = std::move(weakWrapper), + callbackWrapperOwner = std::move(callbackWrapperOwner), + responses = std::move(responses), + executor = std::move(executor)]() { + auto strongWrapper2 = weakWrapper.lock(); + if (!strongWrapper2) { + return; + } + + jsi::Runtime &rt = strongWrapper2->runtime(); + std::vector args; + args.reserve(responses.size()); + for (const auto &val : responses) { + args.emplace_back( + jsi::valueFromDynamic(rt, val)); + } + + executor(strongWrapper2, args); + }); + + wrapperWasCalled = true; + }); +} + jni::local_ref createJavaCallbackFromJSIFunction( jsi::Function &&function, jsi::Runtime &rt, - const std::shared_ptr &jsInvoker, - const std::string& jsStack) { - auto weakWrapper = - CallbackWrapper::createWeak(std::move(function), rt, jsInvoker); - - // This needs to be a shared_ptr because: - // 1. It cannot be unique_ptr. std::function is copyable but unique_ptr is - // not. - // 2. It cannot be weak_ptr since we need this object to live on. - // 3. It cannot be a value, because that would be deleted as soon as this - // function returns. - auto callbackWrapperOwner = - std::make_shared(weakWrapper); - - return JCxxCallbackImpl::newObjectCxxArgs( - [weakWrapper = std::move(weakWrapper), - callbackWrapperOwner = std::move(callbackWrapperOwner), - wrapperWasCalled = false, - jsStack](folly::dynamic responses) mutable { - if (wrapperWasCalled) { - throw std::runtime_error( - "Callback arg cannot be called more than once"); - } - - auto strongWrapper = weakWrapper.lock(); - if (!strongWrapper) { - return; - } - - strongWrapper->jsInvoker().invokeAsync( - [weakWrapper = std::move(weakWrapper), - callbackWrapperOwner = std::move(callbackWrapperOwner), - responses = std::move(responses), - jsStack]() { - auto strongWrapper2 = weakWrapper.lock(); - if (!strongWrapper2) { - return; - } - - jsi::Runtime &rt = strongWrapper2->runtime(); - std::vector args; - args.reserve(responses.size()); - for (const auto &val : responses) { - args.emplace_back( - jsi::valueFromDynamic(rt, val)); - } - - if (jsStack.empty()) { - strongWrapper2->callback().call( - strongWrapper2->runtime(), - (const jsi::Value *)args.data(), - args.size()); - } else { - std::string message; - const jsi::Value* cause = args.data(); - if (cause->isObject() && cause->asObject(rt).getProperty(rt, "message").isString()) { - message = cause->asObject(rt).getProperty(rt, "message").asString(rt).utf8(rt); - } else { - message = ""; - } + const std::shared_ptr &jsInvoker) { + auto executor = []( + const std::shared_ptr& wrapper, + const std::vector& args) { + wrapper->callback().call( + wrapper->runtime(), + (const jsi::Value *)args.data(), + args.size()); + }; + return createJavaCallbackFromJSIFunction(std::move(function), rt, jsInvoker, std::move(executor)); +} - jsi::Value error = createJSRuntimeError(rt, "Exception in HostFunction: " + message); - error.asObject(rt).setProperty(rt, "cause", *cause); - error.asObject(rt).setProperty(rt, "stack", jsStack); - strongWrapper2->callback().call(strongWrapper2->runtime(), error); - } - }); +jni::local_ref createPromiseRejectJavaCallbackFromJSIFunction( + jsi::Function &&function, + jsi::Runtime &rt, + const std::shared_ptr &jsInvoker, + std::shared_ptr jsStack) { + auto executor = [jsStack = std::move(jsStack)]( + const std::shared_ptr& reject, + const std::vector& args) { + std::string message; + jsi::Runtime &rt2 = strongWrapper2->runtime(); + const jsi::Value* cause = args.data(); + if (cause->isObject() && cause->asObject(rt2).getProperty(rt2, "message").isString()) { + message = cause->asObject(rt2).getProperty(rt2, "message").asString(rt2).utf8(rt2); + } else { + message = ""; + } - wrapperWasCalled = true; - }); + jsi::Value error = createJSRuntimeError(rt2, "Exception in HostFunction: " + message); + error.asObject(rt2).setProperty(rt2, "cause", *cause); + error.asObject(rt2).setProperty(rt2, "stack", *jsStack); + reject->callback().call(rt2, error); + }; + return createJavaCallbackFromJSIFunction(std::move(function), rt, jsInvoker, std::move(executor)); } // This is used for generating short exception strings. @@ -382,7 +404,7 @@ JNIArgs convertJSIArgsToJNIArgs( } jsi::Function fn = arg->getObject(rt).getFunction(rt); jarg->l = makeGlobalIfNecessary( - createJavaCallbackFromJSIFunction(std::move(fn), rt, jsInvoker, "") + createJavaCallbackFromJSIFunction(std::move(fn), rt, jsInvoker) .release()); } else if (type == "Lcom/facebook/react/bridge/ReadableArray;") { if (!(arg->isObject() && arg->getObject(rt).isArray(rt))) { @@ -466,6 +488,20 @@ jsi::JSError convertThrowableToJSError( return {runtime, std::move(error)}; } +void rejectWithException( + std::shared_ptr &reject, + std::exception &exception, + std::string &jsStack, + ) { + jsi::Runtime &runtime = rejectStrongWrapper->runtime(); + auto throwable = jni::getJavaExceptionForCppException(exception); + auto jsError = convertThrowableToJSError(runtime, throwable); + jsError.value().asObject(runtime).setProperty(runtime, "stack", jsStack); + reject->callback().call( + runtime, + jsError.value()); +} + } // namespace jsi::Value JavaTurboModule::invokeJavaMethod( @@ -824,13 +860,8 @@ jsi::Value JavaTurboModule::invokeJavaMethod( throw std::invalid_argument("Promise fn arg count must be 2"); } - std::string jsStack; - try { - jsStack = createJSRuntimeError(runtime, "") + std::string jsStack = createJSRuntimeError(runtime, "") .asObject(runtime).getProperty(runtime, "stack").asString(runtime).utf8(runtime); - } catch (...) { - // Stack unavailable - } jsi::Function resolveJSIFn = promiseConstructorArgs[0].getObject(runtime).getFunction( @@ -841,15 +872,15 @@ jsi::Value JavaTurboModule::invokeJavaMethod( jsi::Function rejectInternalJSIFn = promiseConstructorArgs[1].getObject(runtime).getFunction( runtime); - auto rejectWeakWrapper = + auto rejectInternalWeakWrapper = CallbackWrapper::createWeak(std::move(rejectInternalJSIFn), runtime, jsInvoker_); auto resolve = createJavaCallbackFromJSIFunction( - std::move(resolveJSIFn), runtime, jsInvoker_, "") - .release(); - auto reject = createJavaCallbackFromJSIFunction( - std::move(rejectJSIFn), runtime, jsInvoker_, jsStack) - .release(); + std::move(resolveJSIFn), runtime, jsInvoker_) + .release(); + auto reject = createPromiseRejectJavaCallbackFromJSIFunction( + std::move(rejectJSIFn), runtime, jsInvoker_, jsStack) + .release(); jclass jPromiseImpl = env->FindClass("com/facebook/react/bridge/PromiseImpl"); @@ -875,15 +906,15 @@ jsi::Value JavaTurboModule::invokeJavaMethod( nativeInvoker_->invokeAsync( [jargs, - rejectWeakWrapper = std::move(rejectWeakWrapper), - jsStack, + rejectInternalWeakWrapper = std::move(rejectInternalWeakWrapper), + jsStack = std::move(jsStack), globalRefs, methodID, instance_ = jni::make_weak(instance_), moduleNameStr, methodNameStr, id = getUniqueId()]() mutable -> void { - auto rejectStrongWrapper = rejectWeakWrapper.lock(); + auto rejectInternalStrongWrapper = rejectInternalWeakWrapper.lock(); auto instance = instance_.lockLocal(); if (!instance) { @@ -908,14 +939,7 @@ jsi::Value JavaTurboModule::invokeJavaMethod( moduleName, methodName, id); if (rejectStrongWrapper) { auto exception = std::current_exception(); - jsi::Runtime &runtime = rejectStrongWrapper->runtime(); - auto throwable = jni::getJavaExceptionForCppException(exception); - auto jsError = convertThrowableToJSError(runtime, throwable); - jsError.value().asObject(runtime).setProperty(runtime, "stack", jsStack); - rejectStrongWrapper->callback().call( - rejectStrongWrapper->runtime(), - jsError.value(), - 1); + rejectWithException(rejectInternalStrongWrapper, exception, *jsStack); } else { throw; } diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm index 0f2947156dcc..24912a7913e4 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm @@ -265,13 +265,8 @@ static int32_t getUniqueId() } jsi::Function Promise = runtime.global().getPropertyAsFunction(runtime, "Promise"); - std::string jsStack; - try { - jsStack = createJSRuntimeError(runtime, "") + std::string jsStack = createJSRuntimeError(runtime, "") .asObject(runtime).getProperty(runtime, "stack").asString(runtime).utf8(runtime); - } catch (...) { - // Stack unavailable - } std::string moduleName = name_; From be2ea632b7e5253deaa7a18750bba25cf3dd5999 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Thu, 18 May 2023 16:43:47 +0200 Subject: [PATCH 06/55] Fix typos --- .../android/ReactCommon/JavaTurboModule.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp index 3900ae8faa49..d55cd58a0831 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp @@ -150,7 +150,7 @@ jni::local_ref createPromiseRejectJavaCallbackFromJS const std::shared_ptr& reject, const std::vector& args) { std::string message; - jsi::Runtime &rt2 = strongWrapper2->runtime(); + jsi::Runtime &rt2 = reject->runtime(); const jsi::Value* cause = args.data(); if (cause->isObject() && cause->asObject(rt2).getProperty(rt2, "message").isString()) { message = cause->asObject(rt2).getProperty(rt2, "message").asString(rt2).utf8(rt2); @@ -490,10 +490,10 @@ jsi::JSError convertThrowableToJSError( void rejectWithException( std::shared_ptr &reject, - std::exception &exception, - std::string &jsStack, + std::exception_ptr &exception, + std::string &jsStack ) { - jsi::Runtime &runtime = rejectStrongWrapper->runtime(); + jsi::Runtime &runtime = reject->runtime(); auto throwable = jni::getJavaExceptionForCppException(exception); auto jsError = convertThrowableToJSError(runtime, throwable); jsError.value().asObject(runtime).setProperty(runtime, "stack", jsStack); @@ -860,8 +860,8 @@ jsi::Value JavaTurboModule::invokeJavaMethod( throw std::invalid_argument("Promise fn arg count must be 2"); } - std::string jsStack = createJSRuntimeError(runtime, "") - .asObject(runtime).getProperty(runtime, "stack").asString(runtime).utf8(runtime); + std::shared_ptr jsStack = std::make_shared(createJSRuntimeError(runtime, "") + .asObject(runtime).getProperty(runtime, "stack").asString(runtime).utf8(runtime)); jsi::Function resolveJSIFn = promiseConstructorArgs[0].getObject(runtime).getFunction( @@ -937,7 +937,7 @@ jsi::Value JavaTurboModule::invokeJavaMethod( } catch (...) { TMPL::asyncMethodCallExecutionFail( moduleName, methodName, id); - if (rejectStrongWrapper) { + if (rejectInternalStrongWrapper) { auto exception = std::current_exception(); rejectWithException(rejectInternalStrongWrapper, exception, *jsStack); } else { From ccf54606a46eb952adbdd0219bbaeecff1061504 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Thu, 18 May 2023 16:50:49 +0200 Subject: [PATCH 07/55] iOS jsStack do not copy --- .../core/platform/ios/ReactCommon/RCTTurboModule.mm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm index 24912a7913e4..239ffa029ca9 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm @@ -277,7 +277,7 @@ static int32_t getUniqueId() runtime, jsi::PropNameID::forAscii(runtime, "fn"), 2, - [invokeCopy, jsInvoker = jsInvoker_, moduleName, methodName, jsStack]( + [invokeCopy, jsInvoker = jsInvoker_, moduleName, methodName, jsStack = std::move(jsStack)]( jsi::Runtime &rt, const jsi::Value &thisVal, const jsi::Value *args, size_t count) { std::string moduleMethod = moduleName + "." + methodName + "()"; @@ -361,7 +361,7 @@ static int32_t getUniqueId() return; } - strongRejectWrapper->jsInvoker().invokeAsync([weakResolveWrapper, weakRejectWrapper, blockGuard, reason, jsStack]() { + strongRejectWrapper->jsInvoker().invokeAsync([weakResolveWrapper, weakRejectWrapper, blockGuard, reason, jsStack = std::move(jsStack)]() { auto strongResolveWrapper2 = weakResolveWrapper.lock(); auto strongRejectWrapper2 = weakRejectWrapper.lock(); if (!strongResolveWrapper2 || !strongRejectWrapper2) { From 85f48d7fb8cd5af81ae9ff2acf4370069b518a65 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Thu, 18 May 2023 17:01:53 +0200 Subject: [PATCH 08/55] Update lint --- .../android/ReactCommon/JavaTurboModule.cpp | 170 +++++++++--------- 1 file changed, 85 insertions(+), 85 deletions(-) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp index d55cd58a0831..065bc93e4c8c 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp @@ -61,84 +61,76 @@ struct JNIArgs { std::vector globalRefs_; }; -jsi::Value createJSRuntimeError( - jsi::Runtime &runtime, - const std::string &message) { - return runtime.global() - .getPropertyAsFunction(runtime, "Error") - .call(runtime, message); -} jni::local_ref createJavaCallbackFromJSIFunction( - jsi::Function &&function, - jsi::Runtime &rt, - const std::shared_ptr &jsInvoker, - std::function&, const std::vector&)>&& executor) { - auto weakWrapper = - CallbackWrapper::createWeak(std::move(function), rt, jsInvoker); - - // This needs to be a shared_ptr because: - // 1. It cannot be unique_ptr. std::function is copyable but unique_ptr is - // not. - // 2. It cannot be weak_ptr since we need this object to live on. - // 3. It cannot be a value, because that would be deleted as soon as this - // function returns. - auto callbackWrapperOwner = - std::make_shared(weakWrapper); - - return JCxxCallbackImpl::newObjectCxxArgs( + jsi::Function &&function, + jsi::Runtime &rt, + const std::shared_ptr &jsInvoker, + std::function&, const std::vector&)>&& executor) { + auto weakWrapper = + CallbackWrapper::createWeak(std::move(function), rt, jsInvoker); + + // This needs to be a shared_ptr because: + // 1. It cannot be unique_ptr. std::function is copyable but unique_ptr is + // not. + // 2. It cannot be weak_ptr since we need this object to live on. + // 3. It cannot be a value, because that would be deleted as soon as this + // function returns. + auto callbackWrapperOwner = + std::make_shared(weakWrapper); + + return JCxxCallbackImpl::newObjectCxxArgs( + [weakWrapper = std::move(weakWrapper), + callbackWrapperOwner = std::move(callbackWrapperOwner), + wrapperWasCalled = false, + executor = std::move(executor)](folly::dynamic responses) mutable { + if (wrapperWasCalled) { + throw std::runtime_error( + "Callback arg cannot be called more than once"); + } + + auto strongWrapper = weakWrapper.lock(); + if (!strongWrapper) { + return; + } + + strongWrapper->jsInvoker().invokeAsync( [weakWrapper = std::move(weakWrapper), - callbackWrapperOwner = std::move(callbackWrapperOwner), - wrapperWasCalled = false, - executor = std::move(executor)](folly::dynamic responses) mutable { - if (wrapperWasCalled) { - throw std::runtime_error( - "Callback arg cannot be called more than once"); - } - - auto strongWrapper = weakWrapper.lock(); - if (!strongWrapper) { - return; - } - - strongWrapper->jsInvoker().invokeAsync( - [weakWrapper = std::move(weakWrapper), - callbackWrapperOwner = std::move(callbackWrapperOwner), - responses = std::move(responses), - executor = std::move(executor)]() { - auto strongWrapper2 = weakWrapper.lock(); - if (!strongWrapper2) { - return; - } - - jsi::Runtime &rt = strongWrapper2->runtime(); - std::vector args; - args.reserve(responses.size()); - for (const auto &val : responses) { - args.emplace_back( - jsi::valueFromDynamic(rt, val)); - } - - executor(strongWrapper2, args); - }); - - wrapperWasCalled = true; + callbackWrapperOwner = std::move(callbackWrapperOwner), + responses = std::move(responses), + executor = std::move(executor)]() { + auto strongWrapper2 = weakWrapper.lock(); + if (!strongWrapper2) { + return; + } + + std::vector args; + args.reserve(responses.size()); + for (const auto &val : responses) { + args.emplace_back( + jsi::valueFromDynamic(strongWrapper2->runtime(), val)); + } + + executor(strongWrapper2, args); }); + + wrapperWasCalled = true; + }); } jni::local_ref createJavaCallbackFromJSIFunction( jsi::Function &&function, jsi::Runtime &rt, const std::shared_ptr &jsInvoker) { - auto executor = []( - const std::shared_ptr& wrapper, - const std::vector& args) { - wrapper->callback().call( - wrapper->runtime(), - (const jsi::Value *)args.data(), - args.size()); - }; - return createJavaCallbackFromJSIFunction(std::move(function), rt, jsInvoker, std::move(executor)); + auto executor = []( + const std::shared_ptr& wrapper, + const std::vector& args) { + wrapper->callback().call( + wrapper->runtime(), + (const jsi::Value *)args.data(), + args.size()); + }; + return createJavaCallbackFromJSIFunction(std::move(function), rt, jsInvoker, std::move(executor)); } jni::local_ref createPromiseRejectJavaCallbackFromJSIFunction( @@ -146,24 +138,24 @@ jni::local_ref createPromiseRejectJavaCallbackFromJS jsi::Runtime &rt, const std::shared_ptr &jsInvoker, std::shared_ptr jsStack) { - auto executor = [jsStack = std::move(jsStack)]( - const std::shared_ptr& reject, - const std::vector& args) { - std::string message; - jsi::Runtime &rt2 = reject->runtime(); - const jsi::Value* cause = args.data(); - if (cause->isObject() && cause->asObject(rt2).getProperty(rt2, "message").isString()) { - message = cause->asObject(rt2).getProperty(rt2, "message").asString(rt2).utf8(rt2); - } else { - message = ""; - } + auto executor = [jsStack = std::move(jsStack)]( + const std::shared_ptr& reject, + const std::vector& args) { + std::string message; + jsi::Runtime &rt2 = reject->runtime(); + const jsi::Value* cause = args.data(); + if (cause->isObject() && cause->asObject(rt2).getProperty(rt2, "message").isString()) { + message = cause->asObject(rt2).getProperty(rt2, "message").asString(rt2).utf8(rt2); + } else { + message = ""; + } - jsi::Value error = createJSRuntimeError(rt2, "Exception in HostFunction: " + message); - error.asObject(rt2).setProperty(rt2, "cause", *cause); - error.asObject(rt2).setProperty(rt2, "stack", *jsStack); - reject->callback().call(rt2, error); - }; - return createJavaCallbackFromJSIFunction(std::move(function), rt, jsInvoker, std::move(executor)); + jsi::Value error = createJSRuntimeError(rt2, "Exception in HostFunction: " + message); + error.asObject(rt2).setProperty(rt2, "cause", *cause); + error.asObject(rt2).setProperty(rt2, "stack", *jsStack); + reject->callback().call(rt2, error); + }; + return createJavaCallbackFromJSIFunction(std::move(function), rt, jsInvoker, std::move(executor)); } // This is used for generating short exception strings. @@ -449,6 +441,14 @@ jsi::Value convertFromJMapToValue(JNIEnv *env, jsi::Runtime &rt, jobject arg) { return jsi::valueFromDynamic(rt, result->cthis()->consume()); } +jsi::Value createJSRuntimeError( + jsi::Runtime &runtime, + const std::string &message) { + return runtime.global() + .getPropertyAsFunction(runtime, "Error") + .call(runtime, message); +} + /** * Creates JSError with current JS runtime stack and Throwable stack trace. */ From 7205f1d10d0aedea904f074372c839bc2ee0c6b4 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Thu, 18 May 2023 17:03:37 +0200 Subject: [PATCH 09/55] More lint --- .../android/ReactCommon/JavaTurboModule.cpp | 28 +++++++++---------- .../ios/ReactCommon/RCTTurboModule.mm | 3 +- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp index 065bc93e4c8c..289f3793301b 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp @@ -61,7 +61,6 @@ struct JNIArgs { std::vector globalRefs_; }; - jni::local_ref createJavaCallbackFromJSIFunction( jsi::Function &&function, jsi::Runtime &rt, @@ -133,6 +132,8 @@ jni::local_ref createJavaCallbackFromJSIFunction( return createJavaCallbackFromJSIFunction(std::move(function), rt, jsInvoker, std::move(executor)); } +jsi::Value createJSRuntimeError(jsi::Runtime &runtime, const std::string &message); + jni::local_ref createPromiseRejectJavaCallbackFromJSIFunction( jsi::Function &&function, jsi::Runtime &rt, @@ -491,15 +492,14 @@ jsi::JSError convertThrowableToJSError( void rejectWithException( std::shared_ptr &reject, std::exception_ptr &exception, - std::string &jsStack - ) { - jsi::Runtime &runtime = reject->runtime(); - auto throwable = jni::getJavaExceptionForCppException(exception); - auto jsError = convertThrowableToJSError(runtime, throwable); - jsError.value().asObject(runtime).setProperty(runtime, "stack", jsStack); - reject->callback().call( - runtime, - jsError.value()); + std::string &jsStack) { + jsi::Runtime &runtime = reject->runtime(); + auto throwable = jni::getJavaExceptionForCppException(exception); + auto jsError = convertThrowableToJSError(runtime, throwable); + jsError.value().asObject(runtime).setProperty(runtime, "stack", jsStack); + reject->callback().call( + runtime, + jsError.value()); } } // namespace @@ -876,11 +876,11 @@ jsi::Value JavaTurboModule::invokeJavaMethod( CallbackWrapper::createWeak(std::move(rejectInternalJSIFn), runtime, jsInvoker_); auto resolve = createJavaCallbackFromJSIFunction( - std::move(resolveJSIFn), runtime, jsInvoker_) - .release(); + std::move(resolveJSIFn), runtime, jsInvoker_) + .release(); auto reject = createPromiseRejectJavaCallbackFromJSIFunction( - std::move(rejectJSIFn), runtime, jsInvoker_, jsStack) - .release(); + std::move(rejectJSIFn), runtime, jsInvoker_, jsStack) + .release(); jclass jPromiseImpl = env->FindClass("com/facebook/react/bridge/PromiseImpl"); diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm index 239ffa029ca9..0436983228f6 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm @@ -242,7 +242,8 @@ static int32_t getUniqueId() /** * Creates JSErrorValue with given stack trace. */ -static jsi::Value createRejectJSErrorValue(jsi::Runtime &runtime, NSDictionary *reason, std::string jsStack) { +static jsi::Value createRejectJSErrorValue(jsi::Runtime &runtime, NSDictionary *reason, std::string jsStack) +{ jsi::Object cause = convertNSDictionaryToJSIObject(runtime, reason); jsi::Value error; NSString *message = reason[@"message"]; From 99cb0b9c2fdf1b1f77b1efbab1b04f1052bcece2 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Tue, 6 Jun 2023 14:53:34 +0200 Subject: [PATCH 10/55] Add RCT_TRACE_PROMISE_REJECTION_ENABLED flag for iOS --- .../Libraries/AppDelegate/RCTAppSetupUtils.mm | 4 ++++ .../AppDelegate/React-RCTAppDelegate.podspec | 4 +++- packages/react-native/React/Base/RCTBridge.h | 4 ++++ packages/react-native/React/Base/RCTBridge.m | 11 +++++++++++ .../core/platform/ios/ReactCommon/RCTTurboModule.mm | 8 +++++++- 5 files changed, 29 insertions(+), 2 deletions(-) diff --git a/packages/react-native/Libraries/AppDelegate/RCTAppSetupUtils.mm b/packages/react-native/Libraries/AppDelegate/RCTAppSetupUtils.mm index fa96ea3b03a0..fe1171683471 100644 --- a/packages/react-native/Libraries/AppDelegate/RCTAppSetupUtils.mm +++ b/packages/react-native/Libraries/AppDelegate/RCTAppSetupUtils.mm @@ -56,6 +56,10 @@ void RCTAppSetupPrepareApp(UIApplication *application, BOOL turboModuleEnabled) RCTEnableTurboModule(turboModuleEnabled); #endif +#if RCT_TRACE_PROMISE_REJECTION_ENABLED + RCTEnableTracePromiseRejection(YES); +#endif + #if DEBUG // Disable idle timer in dev builds to avoid putting application in background and complicating // Metro reconnection logic. Users only need this when running the application using our CLI tooling. diff --git a/packages/react-native/Libraries/AppDelegate/React-RCTAppDelegate.podspec b/packages/react-native/Libraries/AppDelegate/React-RCTAppDelegate.podspec index 3cb77a170bb6..8579214cf6ed 100644 --- a/packages/react-native/Libraries/AppDelegate/React-RCTAppDelegate.podspec +++ b/packages/react-native/Libraries/AppDelegate/React-RCTAppDelegate.podspec @@ -20,10 +20,12 @@ folly_flags = ' -DFOLLY_NO_CONFIG -DFOLLY_MOBILE=1 -DFOLLY_USE_LIBCPP=1' folly_compiler_flags = folly_flags + ' ' + '-Wno-comma -Wno-shorten-64-to-32' is_new_arch_enabled = ENV["RCT_NEW_ARCH_ENABLED"] == "1" +trace_promise_rejections_enabled = ENV["RCT_TRACE_PROMISE_REJECTION_ENABLED"] == "1" +trace_promise_rejections_flag = (trace_promise_rejections_enabled ? " -DRCT_TRACE_PROMISE_REJECTION_ENABLED" : "") new_arch_enabled_flag = (is_new_arch_enabled ? " -DRCT_NEW_ARCH_ENABLED" : "") is_fabric_enabled = is_new_arch_enabled || ENV["RCT_FABRIC_ENABLED"] fabric_flag = (is_fabric_enabled ? " -DRN_FABRIC_ENABLED" : "") -other_cflags = "$(inherited)" + folly_flags + new_arch_enabled_flag + fabric_flag +other_cflags = "$(inherited)" + folly_flags + new_arch_enabled_flag + fabric_flag + trace_promise_rejections_flag use_hermes = ENV['USE_HERMES'] == '1' use_frameworks = ENV['USE_FRAMEWORKS'] != nil diff --git a/packages/react-native/React/Base/RCTBridge.h b/packages/react-native/React/Base/RCTBridge.h index 92545c4317eb..221e9753b17d 100644 --- a/packages/react-native/React/Base/RCTBridge.h +++ b/packages/react-native/React/Base/RCTBridge.h @@ -68,6 +68,10 @@ RCT_EXTERN void RCTEnableTurboModuleInteropBridgeProxy(BOOL enabled); RCT_EXTERN BOOL RCTTurboModuleInteropForAllTurboModulesEnabled(void); RCT_EXTERN void RCTEnableTurboModuleInteropForAllTurboModules(BOOL enabled); +// Trace Rejected Promises of Turbo Modules (store callers' js stack) +RCT_EXTERN BOOL RCTTracePromiseRejectionsEnabled(void); +RCT_EXTERN void RCTEnableTracePromiseRejection(BOOL enabled); + typedef enum { kRCTGlobalScope, kRCTGlobalScopeUsingRetainJSCallback, diff --git a/packages/react-native/React/Base/RCTBridge.m b/packages/react-native/React/Base/RCTBridge.m index b8300f27f073..65ce3a7a69ec 100644 --- a/packages/react-native/React/Base/RCTBridge.m +++ b/packages/react-native/React/Base/RCTBridge.m @@ -103,6 +103,17 @@ BOOL RCTTurboModuleEagerInitEnabled(void) return turboModuleEagerInitEnabled; } +static BOOL tracePromiseRejectionsEnabled = NO; +BOOL RCTTracePromiseRejectionsEnabled(void) +{ + return tracePromiseRejectionsEnabled; +} + +void RCTEnableTracePromiseRejection(BOOL enabled) +{ + tracePromiseRejectionsEnabled = enabled; +} + void RCTEnableTurboModuleEagerInit(BOOL enabled) { turboModuleEagerInitEnabled = enabled; diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm index 867981ece253..e8ff012ba5e0 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm @@ -264,8 +264,14 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s } jsi::Function Promise = runtime.global().getPropertyAsFunction(runtime, "Promise"); - std::string jsStack = createJSRuntimeError(runtime, "") + std::string jsStack; + if (RCTTracePromiseRejectionsEnabled()) { + jsStack = createJSRuntimeError(runtime, "") .asObject(runtime).getProperty(runtime, "stack").asString(runtime).utf8(runtime); + } else { + jsStack = ""; + } + std::string moduleName = name_; From 732dba92004d876604f90d352fdc2ec505137002 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Tue, 6 Jun 2023 15:37:01 +0200 Subject: [PATCH 11/55] Rename the iOS flag --- .../Libraries/AppDelegate/RCTAppSetupUtils.mm | 2 +- packages/react-native/React/Base/RCTBridge.h | 4 ++-- packages/react-native/React/Base/RCTBridge.m | 10 +++++----- .../core/platform/ios/ReactCommon/RCTTurboModule.mm | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/react-native/Libraries/AppDelegate/RCTAppSetupUtils.mm b/packages/react-native/Libraries/AppDelegate/RCTAppSetupUtils.mm index fe1171683471..b83e58e76783 100644 --- a/packages/react-native/Libraries/AppDelegate/RCTAppSetupUtils.mm +++ b/packages/react-native/Libraries/AppDelegate/RCTAppSetupUtils.mm @@ -57,7 +57,7 @@ void RCTAppSetupPrepareApp(UIApplication *application, BOOL turboModuleEnabled) #endif #if RCT_TRACE_PROMISE_REJECTION_ENABLED - RCTEnableTracePromiseRejection(YES); + RCTEnableTraceTurboModulePromiseRejection(YES); #endif #if DEBUG diff --git a/packages/react-native/React/Base/RCTBridge.h b/packages/react-native/React/Base/RCTBridge.h index 221e9753b17d..b5a4f80861d6 100644 --- a/packages/react-native/React/Base/RCTBridge.h +++ b/packages/react-native/React/Base/RCTBridge.h @@ -69,8 +69,8 @@ RCT_EXTERN BOOL RCTTurboModuleInteropForAllTurboModulesEnabled(void); RCT_EXTERN void RCTEnableTurboModuleInteropForAllTurboModules(BOOL enabled); // Trace Rejected Promises of Turbo Modules (store callers' js stack) -RCT_EXTERN BOOL RCTTracePromiseRejectionsEnabled(void); -RCT_EXTERN void RCTEnableTracePromiseRejection(BOOL enabled); +RCT_EXTERN BOOL RCTTraceTurboModulePromiseRejectionEnabled(void); +RCT_EXTERN void RCTEnableTraceTurboModulePromiseRejection(BOOL enabled); typedef enum { kRCTGlobalScope, diff --git a/packages/react-native/React/Base/RCTBridge.m b/packages/react-native/React/Base/RCTBridge.m index 65ce3a7a69ec..56b31ca127b7 100644 --- a/packages/react-native/React/Base/RCTBridge.m +++ b/packages/react-native/React/Base/RCTBridge.m @@ -103,15 +103,15 @@ BOOL RCTTurboModuleEagerInitEnabled(void) return turboModuleEagerInitEnabled; } -static BOOL tracePromiseRejectionsEnabled = NO; -BOOL RCTTracePromiseRejectionsEnabled(void) +static BOOL traceTurboModulePromiseRejectionEnabled = NO; +BOOL RCTTraceTurboModulePromiseRejectionEnabled(void) { - return tracePromiseRejectionsEnabled; + return traceTurboModulePromiseRejectionEnabled; } -void RCTEnableTracePromiseRejection(BOOL enabled) +void RCTEnableTraceTurboModulePromiseRejection(BOOL enabled) { - tracePromiseRejectionsEnabled = enabled; + traceTurboModulePromiseRejectionEnabled = enabled; } void RCTEnableTurboModuleEagerInit(BOOL enabled) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm index e8ff012ba5e0..8d84b00f4a66 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm @@ -265,7 +265,7 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s jsi::Function Promise = runtime.global().getPropertyAsFunction(runtime, "Promise"); std::string jsStack; - if (RCTTracePromiseRejectionsEnabled()) { + if (RCTTraceTurboModulePromiseRejectionEnabled()) { jsStack = createJSRuntimeError(runtime, "") .asObject(runtime).getProperty(runtime, "stack").asString(runtime).utf8(runtime); } else { From 239667dbcab0b3daf5e5d2dde30b1f5b22b60773 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Wed, 14 Jun 2023 13:50:00 +0200 Subject: [PATCH 12/55] Add android flag and fix internal reject invocation on Hermes --- .../react/config/ReactFeatureFlags.java | 6 +++ .../android/ReactCommon/JavaTurboModule.cpp | 47 ++++++++++++++----- .../react/uiapp/RNTesterApplication.java | 1 + 3 files changed, 43 insertions(+), 11 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java index 6eb5b53b12f5..c8416954bf91 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java @@ -159,4 +159,10 @@ public class ReactFeatureFlags { /** Report mount operations from the host platform to notify mount hooks. */ public static boolean enableMountHooks = false; + + /** + * Enables storing js caller stack when creating promise in native module. + * This is useful in case of Promise rejection and tracing the cause. + */ + public static boolean traceTurboModulePromiseRejectionEnabled = false; } diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp index 93f75a68bf8a..6164a3920339 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp @@ -27,6 +27,16 @@ namespace facebook::react { +constexpr static auto kReactFeatureFlagsJavaDescriptor = + "com/facebook/react/config/ReactFeatureFlags"; + +static bool getFeatureFlagBoolValue(const char *name) { + static const auto reactFeatureFlagsClass = + jni::findClassStatic(kReactFeatureFlagsJavaDescriptor); + const auto field = reactFeatureFlagsClass->getStaticField(name); + return reactFeatureFlagsClass->getStaticFieldValue(field); +} + namespace TMPL = TurboModulePerfLogger; JavaTurboModule::JavaTurboModule(const InitParams ¶ms) @@ -491,16 +501,30 @@ jsi::JSError convertThrowableToJSError( } void rejectWithException( - std::shared_ptr &reject, + std::weak_ptr &rejectWeak, std::exception_ptr &exception, std::string &jsStack) { - jsi::Runtime &runtime = reject->runtime(); + auto reject = rejectWeak.lock(); + if (reject) { + reject->jsInvoker().invokeAsync([ + rejectWeak = std::move(rejectWeak), + jsStack, + exception + ]() { + auto reject2 = rejectWeak.lock(); + if (reject2) { + jsi::Runtime &runtime = reject2->runtime(); auto throwable = jni::getJavaExceptionForCppException(exception); auto jsError = convertThrowableToJSError(runtime, throwable); jsError.value().asObject(runtime).setProperty(runtime, "stack", jsStack); - reject->callback().call( + reject2->callback().call( runtime, jsError.value()); + } + }); + } else { + throw; + } } } // namespace @@ -862,8 +886,14 @@ jsi::Value JavaTurboModule::invokeJavaMethod( throw std::invalid_argument("Promise fn arg count must be 2"); } - std::shared_ptr jsStack = std::make_shared(createJSRuntimeError(runtime, "") + bool traceTurboModulePromiseRejectionEnabled = getFeatureFlagBoolValue("traceTurboModulePromiseRejectionEnabled"); + std::shared_ptr jsStack; + if (traceTurboModulePromiseRejectionEnabled) { + jsStack = std::make_shared(createJSRuntimeError(runtime, "") .asObject(runtime).getProperty(runtime, "stack").asString(runtime).utf8(runtime)); + } else { + jsStack = std::make_shared(""); + } jsi::Function resolveJSIFn = promiseConstructorArgs[0].getObject(runtime).getFunction( @@ -909,7 +939,7 @@ jsi::Value JavaTurboModule::invokeJavaMethod( nativeMethodCallInvoker_->invokeAsync( methodName, [jargs, - rejectInternalWeakWrapper = std::move(rejectInternalWeakWrapper), + rejectInternalWeakWrapper = rejectInternalWeakWrapper, jsStack = std::move(jsStack), globalRefs, methodID, @@ -917,7 +947,6 @@ jsi::Value JavaTurboModule::invokeJavaMethod( moduleNameStr, methodNameStr, id = getUniqueId()]() mutable -> void { - auto rejectInternalStrongWrapper = rejectInternalWeakWrapper.lock(); auto instance = instance_.lockLocal(); if (!instance) { @@ -940,12 +969,8 @@ jsi::Value JavaTurboModule::invokeJavaMethod( } catch (...) { TMPL::asyncMethodCallExecutionFail( moduleName, methodName, id); - if (rejectInternalStrongWrapper) { auto exception = std::current_exception(); - rejectWithException(rejectInternalStrongWrapper, exception, *jsStack); - } else { - throw; - } + rejectWithException(rejectInternalWeakWrapper, exception, *jsStack); } for (auto globalRef : globalRefs) { diff --git a/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterApplication.java b/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterApplication.java index cb00882bbed8..80e0fb2dd261 100644 --- a/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterApplication.java +++ b/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterApplication.java @@ -129,6 +129,7 @@ protected Boolean isHermesEnabled() { @Override public void onCreate() { + ReactFeatureFlags.traceTurboModulePromiseRejectionEnabled = true; ReactFontManager.getInstance().addCustomFont(this, "Rubik", R.font.rubik); super.onCreate(); SoLoader.init(this, /* native exopackage */ false); From 882c81b1137cd25332b0216a2a8dd8875a1c91c9 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Wed, 14 Jun 2023 14:39:22 +0200 Subject: [PATCH 13/55] move RCTInternalPromiseRejectBlock to rct turbo module --- packages/react-native/React/Base/RCTBridgeModule.h | 2 -- .../core/platform/ios/ReactCommon/RCTTurboModule.h | 6 ++++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/react-native/React/Base/RCTBridgeModule.h b/packages/react-native/React/Base/RCTBridgeModule.h index f0bb5bff5cff..7e7cf91cd6cf 100644 --- a/packages/react-native/React/Base/RCTBridgeModule.h +++ b/packages/react-native/React/Base/RCTBridgeModule.h @@ -46,8 +46,6 @@ typedef void (^RCTPromiseResolveBlock)(id result); */ typedef void (^RCTPromiseRejectBlock)(NSString *code, NSString *message, NSError *error); -typedef void (^RCTInternalPromiseRejectBlock)(NSException *exception); - RCT_EXTERN_C_BEGIN typedef struct RCTMethodInfo { diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.h b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.h index d20569d81c95..7d930b2679df 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.h +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.h @@ -23,6 +23,12 @@ ((RCTTurboModuleEnabled() && [(klass) conformsToProtocol:@protocol(RCTTurboModule)])) #define RCT_IS_TURBO_MODULE_INSTANCE(module) RCT_IS_TURBO_MODULE_CLASS([(module) class]) +/** + * Block used to reject the JS promise waiting for a result + * in cases when the native method throws an exception. + */ +typedef void (^RCTInternalPromiseRejectBlock)(NSException *exception); + namespace facebook::react { class CallbackWrapper; From 9529dd958c8c2059df3ce249b5e1f8ae0889ae83 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Wed, 14 Jun 2023 14:40:14 +0200 Subject: [PATCH 14/55] Add js func returning void rethrow explanation --- .../core/platform/ios/ReactCommon/RCTTurboModule.mm | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm index 8d84b00f4a66..16636c99f70b 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm @@ -449,7 +449,8 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s if (optionalInternalRejectBlock != nil) { optionalInternalRejectBlock(exception); } else { - // JSFunction returning void + // Crash on native layer there is no promise in JS to reject. + // We executed JSFunction returning void asynchrounously. throw; } } From 40163ffc18a702519d08df1b3a422392b7d7497b Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Thu, 15 Jun 2023 11:28:32 +0200 Subject: [PATCH 15/55] Rename RCTInternalPromiseRejectBlock to RCTNSExceptionHandler --- .../core/platform/ios/ReactCommon/RCTTurboModule.h | 6 +++--- .../core/platform/ios/ReactCommon/RCTTurboModule.mm | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.h b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.h index 7d930b2679df..b57071b00171 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.h +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.h @@ -27,7 +27,7 @@ * Block used to reject the JS promise waiting for a result * in cases when the native method throws an exception. */ -typedef void (^RCTInternalPromiseRejectBlock)(NSException *exception); +typedef void (^RCTNSExceptionHandler)(NSException *exception); namespace facebook::react { @@ -144,9 +144,9 @@ class JSI_EXPORT ObjCTurboModule : public TurboModule { const char *methodName, NSInvocation *inv, NSMutableArray *retainedObjectsForInvocation, - RCTInternalPromiseRejectBlock optionalInternalRejectBlock); + RCTNSExceptionHandler optionalInternalRejectBlock); - using PromiseInvocationBlock = void (^)(RCTPromiseResolveBlock resolveWrapper, RCTPromiseRejectBlock rejectWrapper, RCTInternalPromiseRejectBlock internalRejectWrapper); + using PromiseInvocationBlock = void (^)(RCTPromiseResolveBlock resolveWrapper, RCTPromiseRejectBlock rejectWrapper, RCTNSExceptionHandler internalRejectWrapper); jsi::Value createPromise(jsi::Runtime &runtime, std::string methodName, PromiseInvocationBlock invoke); }; diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm index 16636c99f70b..a08f5b16c32b 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm @@ -389,7 +389,7 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s handlePromiseRejection(reason); }; - RCTInternalPromiseRejectBlock internalRejectBlock = ^(NSException *exception) { + RCTNSExceptionHandler internalRejectBlock = ^(NSException *exception) { handlePromiseRejection(@{ @"name": exception.name, @"message": exception.reason, @@ -420,7 +420,7 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s const char *methodName, NSInvocation *inv, NSMutableArray *retainedObjectsForInvocation, - RCTInternalPromiseRejectBlock optionalInternalRejectBlock) + RCTNSExceptionHandler optionalInternalRejectBlock) { __block id result; __weak id weakModule = instance_; @@ -766,10 +766,10 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s if (returnType == PromiseKind) { returnValue = createPromise( - runtime, methodNameStr, ^(RCTPromiseResolveBlock resolveBlock, RCTPromiseRejectBlock rejectBlock, RCTInternalPromiseRejectBlock internalRejectBlock) { + runtime, methodNameStr, ^(RCTPromiseResolveBlock resolveBlock, RCTPromiseRejectBlock rejectBlock, RCTNSExceptionHandler internalRejectBlock) { RCTPromiseResolveBlock resolveCopy = [resolveBlock copy]; RCTPromiseRejectBlock rejectCopy = [rejectBlock copy]; - RCTInternalPromiseRejectBlock internalRejectCopy = [internalRejectBlock copy]; + RCTNSExceptionHandler internalRejectCopy = [internalRejectBlock copy]; [inv setArgument:(void *)&resolveCopy atIndex:count + 2]; [inv setArgument:(void *)&rejectCopy atIndex:count + 3]; From 49b861d5a12c6e6b10973ba60d30c5dca2df02a0 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Thu, 15 Jun 2023 12:11:32 +0200 Subject: [PATCH 16/55] Add CallbackWrapperExecutor type --- .../platform/android/ReactCommon/JavaTurboModule.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp index 6164a3920339..bbfac9c522ae 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp @@ -73,11 +73,13 @@ struct JNIArgs { std::vector globalRefs_; }; +using CallbackWrapperExecutor = std::function&, const std::vector&)>; + jni::local_ref createJavaCallbackFromJSIFunction( jsi::Function &&function, jsi::Runtime &rt, const std::shared_ptr &jsInvoker, - std::function&, const std::vector&)>&& executor) { + CallbackWrapperExecutor&& callbackWrapperExecutor) { auto weakWrapper = CallbackWrapper::createWeak(std::move(function), rt, jsInvoker); @@ -94,7 +96,7 @@ jni::local_ref createJavaCallbackFromJSIFunction( [weakWrapper = std::move(weakWrapper), callbackWrapperOwner = std::move(callbackWrapperOwner), wrapperWasCalled = false, - executor = std::move(executor)](folly::dynamic responses) mutable { + callbackWrapperExecutor = std::move(callbackWrapperExecutor)](folly::dynamic responses) mutable { if (wrapperWasCalled) { LOG(FATAL) << "callback arg cannot be called more than once"; } @@ -108,7 +110,7 @@ jni::local_ref createJavaCallbackFromJSIFunction( [weakWrapper = std::move(weakWrapper), callbackWrapperOwner = std::move(callbackWrapperOwner), responses = std::move(responses), - executor = std::move(executor)]() { + callbackWrapperExecutor = std::move(callbackWrapperExecutor)]() { auto strongWrapper2 = weakWrapper.lock(); if (!strongWrapper2) { return; @@ -121,7 +123,7 @@ jni::local_ref createJavaCallbackFromJSIFunction( jsi::valueFromDynamic(strongWrapper2->runtime(), val)); } - executor(strongWrapper2, args); + callbackWrapperExecutor(strongWrapper2, args); }); wrapperWasCalled = true; From 4d40cebe4017509098d889fdbb75761e895a21e3 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Thu, 15 Jun 2023 14:51:17 +0200 Subject: [PATCH 17/55] Handle all throws from Obj-Cpp --- .../platform/ios/ReactCommon/RCTTurboModule.h | 6 +- .../ios/ReactCommon/RCTTurboModule.mm | 106 +++++++++++++----- .../ios/ReactCommon/RCTSampleTurboModule.mm | 6 +- 3 files changed, 87 insertions(+), 31 deletions(-) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.h b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.h index b57071b00171..cb438507f5b3 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.h +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.h @@ -27,7 +27,7 @@ * Block used to reject the JS promise waiting for a result * in cases when the native method throws an exception. */ -typedef void (^RCTNSExceptionHandler)(NSException *exception); +typedef void (^RCTNSDictionaryPromiseRejectBlock)(NSDictionary *exception); namespace facebook::react { @@ -144,9 +144,9 @@ class JSI_EXPORT ObjCTurboModule : public TurboModule { const char *methodName, NSInvocation *inv, NSMutableArray *retainedObjectsForInvocation, - RCTNSExceptionHandler optionalInternalRejectBlock); + RCTNSDictionaryPromiseRejectBlock optionalInternalRejectBlock); - using PromiseInvocationBlock = void (^)(RCTPromiseResolveBlock resolveWrapper, RCTPromiseRejectBlock rejectWrapper, RCTNSExceptionHandler internalRejectWrapper); + using PromiseInvocationBlock = void (^)(RCTPromiseResolveBlock resolveWrapper, RCTPromiseRejectBlock rejectWrapper, RCTNSDictionaryPromiseRejectBlock internalRejectWrapper); jsi::Value createPromise(jsi::Runtime &runtime, std::string methodName, PromiseInvocationBlock invoke); }; diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm index a08f5b16c32b..1f2a3f6f4ebb 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm @@ -237,6 +237,16 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s return {runtime, std::move(error)}; } +/** + * Creates JSError with current JS runtime and NSDictionary data as cause. + */ +static jsi::JSError convertNSDictionaryToJSError(jsi::Runtime &runtime, const std::string &message, NSDictionary *cause) +{ + jsi::Value error = createJSRuntimeError(runtime, "Exception in HostFunction: " + message); + error.asObject(runtime).setProperty(runtime, "cause", convertNSDictionaryToJSIObject(runtime, cause)); + return {runtime, std::move(error)}; +} + /** * Creates JSErrorValue with given stack trace. */ @@ -389,13 +399,8 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s handlePromiseRejection(reason); }; - RCTNSExceptionHandler internalRejectBlock = ^(NSException *exception) { - handlePromiseRejection(@{ - @"name": exception.name, - @"message": exception.reason, - @"stackSymbols": exception.callStackSymbols, - @"stackReturnAddresses": exception.callStackReturnAddresses, - }); + RCTNSDictionaryPromiseRejectBlock internalRejectBlock = ^(NSDictionary *dict) { + handlePromiseRejection(dict); }; invokeCopy(resolveBlock, rejectBlock, internalRejectBlock); @@ -405,6 +410,32 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s return Promise.callAsConstructor(runtime, fn); } +static void handleCause( + jsi::Runtime &runtime, + bool isSync, + NSDictionary *cause, + NSMutableArray *retainedObjectsForInvocation, + RCTNSDictionaryPromiseRejectBlock optionalInternalRejectBlock) +{ + [retainedObjectsForInvocation removeAllObjects]; + if (isSync) { + NSString *message = cause[@"message"]; + if (message) { + throw convertNSDictionaryToJSError(runtime, std::string([message UTF8String]), cause); + } else { + throw convertNSDictionaryToJSError(runtime, "", cause); + } + } else { + if (optionalInternalRejectBlock != nil) { + optionalInternalRejectBlock(cause); + } else { + // Crash on native layer there is no promise in JS to reject. + // We executed JSFunction returning void asynchrounously. + throw; + } + } +} + /** * Perform method invocation on a specific queue as configured by the module class. * This serves as a backward-compatible support for RCTBridgeModule's methodQueue API. @@ -420,7 +451,7 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s const char *methodName, NSInvocation *inv, NSMutableArray *retainedObjectsForInvocation, - RCTNSExceptionHandler optionalInternalRejectBlock) + RCTNSDictionaryPromiseRejectBlock optionalInternalRejectBlock) { __block id result; __weak id weakModule = instance_; @@ -440,22 +471,47 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s TurboModulePerfLogger::asyncMethodCallExecutionStart(moduleName, methodNameStr.c_str(), asyncCallCounter); } - @try { + try { [inv invokeWithTarget:strongModule]; - } @catch (NSException *exception) { - if (isSync) { - throw convertNSExceptionToJSError(runtime, exception); - } else { - if (optionalInternalRejectBlock != nil) { - optionalInternalRejectBlock(exception); - } else { - // Crash on native layer there is no promise in JS to reject. - // We executed JSFunction returning void asynchrounously. - throw; - } - } - } @finally { - [retainedObjectsForInvocation removeAllObjects]; + } catch (NSException *exception) { + NSDictionary *cause = @{ + @"name": exception.name, + @"message": exception.reason, + @"stackSymbols": exception.callStackSymbols, + @"stackReturnAddresses": exception.callStackReturnAddresses, + }; + handleCause(runtime, isSync, cause, retainedObjectsForInvocation, optionalInternalRejectBlock); + } catch (NSError *error) { + NSDictionary *cause = @{ + @"userInfo": error.userInfo, + @"domain": error.domain, + }; + handleCause(runtime, isSync, cause, retainedObjectsForInvocation, optionalInternalRejectBlock); + } catch (NSString *errorMessage) { + NSDictionary *cause = @{ + @"message": errorMessage, + }; + handleCause(runtime, isSync, cause, retainedObjectsForInvocation, optionalInternalRejectBlock); + } catch (id e) { + NSDictionary *cause = @{ + @"message": @"Unknown Objective-C Object thrown.", + }; + handleCause(runtime, isSync, cause, retainedObjectsForInvocation, optionalInternalRejectBlock); + } catch (const std::exception &exception) { + NSDictionary *cause = @{ + @"message": [NSString stringWithUTF8String:exception.what()], + }; + handleCause(runtime, isSync, cause, retainedObjectsForInvocation, optionalInternalRejectBlock); + } catch (const std::string &errorMessage) { + NSDictionary *cause = @{ + @"message": [NSString stringWithUTF8String:errorMessage.c_str()], + }; + handleCause(runtime, isSync, cause, retainedObjectsForInvocation, optionalInternalRejectBlock); + } catch (...) { + NSDictionary *cause = @{ + @"message": @"Unknown C++ exception thrown.", + }; + handleCause(runtime, isSync, cause, retainedObjectsForInvocation, optionalInternalRejectBlock); } if (!isSync) { @@ -766,10 +822,10 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s if (returnType == PromiseKind) { returnValue = createPromise( - runtime, methodNameStr, ^(RCTPromiseResolveBlock resolveBlock, RCTPromiseRejectBlock rejectBlock, RCTNSExceptionHandler internalRejectBlock) { + runtime, methodNameStr, ^(RCTPromiseResolveBlock resolveBlock, RCTPromiseRejectBlock rejectBlock, RCTNSDictionaryPromiseRejectBlock internalRejectBlock) { RCTPromiseResolveBlock resolveCopy = [resolveBlock copy]; RCTPromiseRejectBlock rejectCopy = [rejectBlock copy]; - RCTNSExceptionHandler internalRejectCopy = [internalRejectBlock copy]; + RCTNSDictionaryPromiseRejectBlock internalRejectCopy = [internalRejectBlock copy]; [inv setArgument:(void *)&resolveCopy atIndex:count + 2]; [inv setArgument:(void *)&rejectCopy atIndex:count + 3]; diff --git a/packages/react-native/ReactCommon/react/nativemodule/samples/platform/ios/ReactCommon/RCTSampleTurboModule.mm b/packages/react-native/ReactCommon/react/nativemodule/samples/platform/ios/ReactCommon/RCTSampleTurboModule.mm index 8a286dcfd336..a5f92e0ee4a6 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/samples/platform/ios/ReactCommon/RCTSampleTurboModule.mm +++ b/packages/react-native/ReactCommon/react/nativemodule/samples/platform/ios/ReactCommon/RCTSampleTurboModule.mm @@ -148,7 +148,7 @@ - (NSDictionary *)constantsToExport RCT_EXPORT_METHOD(voidFuncThrows) { - NSException *myException = [NSException exceptionWithName:@"Excepption" + NSException *myException = [NSException exceptionWithName:@"Exception" reason:@"Intentional exception from ObjC voidFuncThrows" userInfo:nil]; @throw myException; @@ -156,7 +156,7 @@ - (NSDictionary *)constantsToExport RCT_EXPORT_SYNCHRONOUS_TYPED_METHOD(NSDictionary *, getObjectThrows : (NSDictionary *)arg) { - NSException *myException = [NSException exceptionWithName:@"Excepption" + NSException *myException = [NSException exceptionWithName:@"Exception" reason:@"Intentional exception from ObjC getObjectThrows" userInfo:nil]; @throw myException; @@ -167,7 +167,7 @@ - (NSDictionary *)constantsToExport : (RCTPromiseResolveBlock)resolve reject : (RCTPromiseRejectBlock)reject) { - NSException *myException = [NSException exceptionWithName:@"Excepption" + NSException *myException = [NSException exceptionWithName:@"Exception" reason:@"Intentional exception from ObjC promiseThrows" userInfo:nil]; @throw myException; From 25b32b70c580dce1233657d2a1cdb5bb68e2ba6b Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Fri, 11 Aug 2023 21:48:56 +0200 Subject: [PATCH 18/55] Enable trace turbo module rejection in debug iOS builds --- packages/react-native/Libraries/AppDelegate/RCTAppSetupUtils.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-native/Libraries/AppDelegate/RCTAppSetupUtils.mm b/packages/react-native/Libraries/AppDelegate/RCTAppSetupUtils.mm index 895a6a3340ff..4897747c3f72 100644 --- a/packages/react-native/Libraries/AppDelegate/RCTAppSetupUtils.mm +++ b/packages/react-native/Libraries/AppDelegate/RCTAppSetupUtils.mm @@ -56,7 +56,7 @@ void RCTAppSetupPrepareApp(UIApplication *application, BOOL turboModuleEnabled) RCTEnableTurboModule(turboModuleEnabled); #endif -#if RCT_TRACE_PROMISE_REJECTION_ENABLED +#if RCT_TRACE_PROMISE_REJECTION_ENABLED || DEBUG RCTEnableTraceTurboModulePromiseRejection(YES); #endif From be8e02767a28754aa307d998369738529876e62a Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Fri, 11 Aug 2023 21:49:16 +0200 Subject: [PATCH 19/55] Make jsInvocationStack optional --- .../android/ReactCommon/JavaTurboModule.cpp | 65 +++++++++++-------- .../ios/ReactCommon/RCTTurboModule.mm | 25 ++++--- 2 files changed, 52 insertions(+), 38 deletions(-) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp index 8c9ef339ead8..b26309736ec0 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp @@ -27,16 +27,6 @@ namespace facebook::react { -constexpr static auto kReactFeatureFlagsJavaDescriptor = - "com/facebook/react/config/ReactFeatureFlags"; - -static bool getFeatureFlagBoolValue(const char *name) { - static const auto reactFeatureFlagsClass = - jni::findClassStatic(kReactFeatureFlagsJavaDescriptor); - const auto field = reactFeatureFlagsClass->getStaticField(name); - return reactFeatureFlagsClass->getStaticFieldValue(field); -} - namespace TMPL = TurboModulePerfLogger; JavaTurboModule::JavaTurboModule(const InitParams ¶ms) @@ -67,6 +57,23 @@ JavaTurboModule::~JavaTurboModule() { namespace { +constexpr static auto kReactFeatureFlagsJavaDescriptor = "com/facebook/react/config/ReactFeatureFlags"; + +static bool getFeatureFlagBoolValue(const char *name) { + static const auto reactFeatureFlagsClass = facebook::jni::findClassStatic(kReactFeatureFlagsJavaDescriptor); + const auto field = reactFeatureFlagsClass->getStaticField(name); + return reactFeatureFlagsClass->getStaticFieldValue(field); +} + +static bool isSavePromiseJSInvocationStackEnabled() { +#if DEBUG + return true; +#else + static bool savePromiseJSInvocationStack = getFeatureFlagBoolValue("traceTurboModulePromiseRejectionEnabled"); + return savePromiseJSInvocationStack; +#endif +} + struct JNIArgs { JNIArgs(size_t count) : args_(count) {} std::vector args_; @@ -151,8 +158,8 @@ jni::local_ref createPromiseRejectJavaCallbackFromJS jsi::Function &&function, jsi::Runtime &rt, const std::shared_ptr &jsInvoker, - std::shared_ptr jsStack) { - auto executor = [jsStack = std::move(jsStack)]( + std::optional jsInvocationStack) { + auto executor = [jsInvocationStack = std::move(jsInvocationStack)]( const std::shared_ptr& reject, const std::vector& args) { std::string message; @@ -166,7 +173,7 @@ jni::local_ref createPromiseRejectJavaCallbackFromJS jsi::Value error = createJSRuntimeError(rt2, "Exception in HostFunction: " + message); error.asObject(rt2).setProperty(rt2, "cause", *cause); - error.asObject(rt2).setProperty(rt2, "stack", *jsStack); + error.asObject(rt2).setProperty(rt2, "stack", *jsInvocationStack); reject->callback().call(rt2, error); }; return createJavaCallbackFromJSIFunction(std::move(function), rt, jsInvoker, std::move(executor)); @@ -499,20 +506,22 @@ jsi::JSError convertThrowableToJSError( void rejectWithException( std::weak_ptr &rejectWeak, std::exception_ptr &exception, - std::string &jsStack) { + std::optional jsInvocationStack) { auto reject = rejectWeak.lock(); if (reject) { reject->jsInvoker().invokeAsync([ rejectWeak = std::move(rejectWeak), - jsStack, + jsInvocationStack = std::move(jsInvocationStack), exception ]() { auto reject2 = rejectWeak.lock(); if (reject2) { jsi::Runtime &runtime = reject2->runtime(); - auto throwable = jni::getJavaExceptionForCppException(exception); - auto jsError = convertThrowableToJSError(runtime, throwable); - jsError.value().asObject(runtime).setProperty(runtime, "stack", jsStack); + auto throwable = jni::getJavaExceptionForCppException(exception); + auto jsError = convertThrowableToJSError(runtime, throwable); + if (jsInvocationStack.has_value()) { + jsError.value().asObject(runtime).setProperty(runtime, "stack", *jsInvocationStack); + } reject2->callback().call( runtime, jsError.value()); @@ -882,13 +891,13 @@ jsi::Value JavaTurboModule::invokeJavaMethod( throw std::invalid_argument("Promise fn arg count must be 2"); } - bool traceTurboModulePromiseRejectionEnabled = getFeatureFlagBoolValue("traceTurboModulePromiseRejectionEnabled"); - std::shared_ptr jsStack; - if (traceTurboModulePromiseRejectionEnabled) { - jsStack = std::make_shared(createJSRuntimeError(runtime, "") - .asObject(runtime).getProperty(runtime, "stack").asString(runtime).utf8(runtime)); - } else { - jsStack = std::make_shared(""); + std::optional jsInvocationStack; + if (isSavePromiseJSInvocationStackEnabled()) { + jsInvocationStack = createJSRuntimeError(runtime, "") + .asObject(runtime) + .getProperty(runtime, "stack") + .asString(runtime) + .utf8(runtime); } jsi::Function resolveJSIFn = @@ -907,7 +916,7 @@ jsi::Value JavaTurboModule::invokeJavaMethod( std::move(resolveJSIFn), runtime, jsInvoker_) .release(); auto reject = createPromiseRejectJavaCallbackFromJSIFunction( - std::move(rejectJSIFn), runtime, jsInvoker_, jsStack) + std::move(rejectJSIFn), runtime, jsInvoker_, jsInvocationStack) .release(); jclass jPromiseImpl = @@ -936,7 +945,7 @@ jsi::Value JavaTurboModule::invokeJavaMethod( methodName, [jargs, rejectInternalWeakWrapper = rejectInternalWeakWrapper, - jsStack = std::move(jsStack), + jsInvocationStack = std::move(jsInvocationStack), globalRefs, methodID, instance_ = jni::make_weak(instance_), @@ -966,7 +975,7 @@ jsi::Value JavaTurboModule::invokeJavaMethod( TMPL::asyncMethodCallExecutionFail( moduleName, methodName, id); auto exception = std::current_exception(); - rejectWithException(rejectInternalWeakWrapper, exception, *jsStack); + rejectWithException(rejectInternalWeakWrapper, exception, jsInvocationStack); } for (auto globalRef : globalRefs) { diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm index 1f2a3f6f4ebb..5834ac76b0d9 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm @@ -250,7 +250,7 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s /** * Creates JSErrorValue with given stack trace. */ -static jsi::Value createRejectJSErrorValue(jsi::Runtime &runtime, NSDictionary *reason, std::string jsStack) +static jsi::Value createRejectJSErrorValue(jsi::Runtime &runtime, NSDictionary *reason, std::optional jsInvocationStack) { jsi::Object cause = convertNSDictionaryToJSIObject(runtime, reason); jsi::Value error; @@ -260,7 +260,11 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s } else { error = createJSRuntimeError(runtime, "Exception in HostFunction: "); } - error.asObject(runtime).setProperty(runtime, "stack", jsStack); + + if (jsInvocationStack.has_value()) { + error.asObject(runtime).setProperty(runtime, "stack", *jsInvocationStack); + } + error.asObject(runtime).setProperty(runtime, "cause", std::move(cause)); return error; } @@ -274,12 +278,13 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s } jsi::Function Promise = runtime.global().getPropertyAsFunction(runtime, "Promise"); - std::string jsStack; + std::optional jsInvocationStack; if (RCTTraceTurboModulePromiseRejectionEnabled()) { - jsStack = createJSRuntimeError(runtime, "") - .asObject(runtime).getProperty(runtime, "stack").asString(runtime).utf8(runtime); - } else { - jsStack = ""; + jsInvocationStack = createJSRuntimeError(runtime, "") + .asObject(runtime) + .getProperty(runtime, "stack") + .asString(runtime) + .utf8(runtime); } @@ -292,7 +297,7 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s runtime, jsi::PropNameID::forAscii(runtime, "fn"), 2, - [invokeCopy, jsInvoker = jsInvoker_, moduleName, methodName, jsStack = std::move(jsStack)]( + [invokeCopy, jsInvoker = jsInvoker_, moduleName, methodName, jsInvocationStack = std::move(jsInvocationStack)]( jsi::Runtime &rt, const jsi::Value &thisVal, const jsi::Value *args, size_t count) { std::string moduleMethod = moduleName + "." + methodName + "()"; @@ -376,7 +381,7 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s return; } - strongRejectWrapper->jsInvoker().invokeAsync([weakResolveWrapper, weakRejectWrapper, blockGuard, reason, jsStack = std::move(jsStack)]() { + strongRejectWrapper->jsInvoker().invokeAsync([weakResolveWrapper, weakRejectWrapper, blockGuard, reason, jsInvocationStack = std::move(jsInvocationStack)]() { auto strongResolveWrapper2 = weakResolveWrapper.lock(); auto strongRejectWrapper2 = weakRejectWrapper.lock(); if (!strongResolveWrapper2 || !strongRejectWrapper2) { @@ -384,7 +389,7 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s } jsi::Runtime &rt = strongRejectWrapper2->runtime(); - strongRejectWrapper2->callback().call(rt, createRejectJSErrorValue(rt, reason, jsStack)); + strongRejectWrapper2->callback().call(rt, createRejectJSErrorValue(rt, reason, std::move(jsInvocationStack))); strongResolveWrapper2->destroy(); strongRejectWrapper2->destroy(); From 377c328cbdd2e31de9085b2ccb0d7073dc0dbce3 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Fri, 11 Aug 2023 21:51:17 +0200 Subject: [PATCH 20/55] Add jsInvocationStack comment --- .../core/platform/android/ReactCommon/JavaTurboModule.cpp | 1 + .../nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm | 1 + 2 files changed, 2 insertions(+) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp index b26309736ec0..22e4d6a49869 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp @@ -891,6 +891,7 @@ jsi::Value JavaTurboModule::invokeJavaMethod( throw std::invalid_argument("Promise fn arg count must be 2"); } + // JS Stack at the time when the promise is created. std::optional jsInvocationStack; if (isSavePromiseJSInvocationStackEnabled()) { jsInvocationStack = createJSRuntimeError(runtime, "") diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm index 5834ac76b0d9..e8b8d36aa2a2 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm @@ -278,6 +278,7 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s } jsi::Function Promise = runtime.global().getPropertyAsFunction(runtime, "Promise"); + // JS Stack at the time when the promise is created. std::optional jsInvocationStack; if (RCTTraceTurboModulePromiseRejectionEnabled()) { jsInvocationStack = createJSRuntimeError(runtime, "") From 6e4b652849f14a1a1eeb100d0f4e00e4f20a095c Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Fri, 11 Aug 2023 22:17:36 +0200 Subject: [PATCH 21/55] use ref to js stack in createRejectJSErrorValue --- .../core/platform/ios/ReactCommon/RCTTurboModule.mm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm index e8b8d36aa2a2..7c54f42ddcdb 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm @@ -250,7 +250,7 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s /** * Creates JSErrorValue with given stack trace. */ -static jsi::Value createRejectJSErrorValue(jsi::Runtime &runtime, NSDictionary *reason, std::optional jsInvocationStack) +static jsi::Value createRejectJSErrorValue(jsi::Runtime &runtime, NSDictionary *reason, const std::optional &jsInvocationStack) { jsi::Object cause = convertNSDictionaryToJSIObject(runtime, reason); jsi::Value error; @@ -390,7 +390,7 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s } jsi::Runtime &rt = strongRejectWrapper2->runtime(); - strongRejectWrapper2->callback().call(rt, createRejectJSErrorValue(rt, reason, std::move(jsInvocationStack))); + strongRejectWrapper2->callback().call(rt, createRejectJSErrorValue(rt, reason, jsInvocationStack)); strongResolveWrapper2->destroy(); strongRejectWrapper2->destroy(); From abf7e8a4781c41b199bca9d4242ad3b0075eedf6 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Fri, 11 Aug 2023 23:03:19 +0200 Subject: [PATCH 22/55] Use only one rejectJSIFn in java --- .../android/ReactCommon/JavaTurboModule.cpp | 50 +++++++++++++++---- 1 file changed, 40 insertions(+), 10 deletions(-) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp index 22e4d6a49869..926fb6c5708d 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp @@ -155,7 +155,7 @@ jni::local_ref createJavaCallbackFromJSIFunction( jsi::Value createJSRuntimeError(jsi::Runtime &runtime, const std::string &message); jni::local_ref createPromiseRejectJavaCallbackFromJSIFunction( - jsi::Function &&function, + const std::weak_ptr &function, jsi::Runtime &rt, const std::shared_ptr &jsInvoker, std::optional jsInvocationStack) { @@ -176,7 +176,40 @@ jni::local_ref createPromiseRejectJavaCallbackFromJS error.asObject(rt2).setProperty(rt2, "stack", *jsInvocationStack); reject->callback().call(rt2, error); }; - return createJavaCallbackFromJSIFunction(std::move(function), rt, jsInvoker, std::move(executor)); + return JCxxCallbackImpl::newObjectCxxArgs( + [function, + wrapperWasCalled = false, + executor = std::move(executor)](folly::dynamic responses) mutable { + if (wrapperWasCalled) { + LOG(FATAL) << "callback arg cannot be called more than once"; + } + + auto strongWrapper = function.lock(); + if (!strongWrapper) { + return; + } + + strongWrapper->jsInvoker().invokeAsync( + [function, + responses = std::move(responses), + executor = std::move(executor)]() { + auto strongWrapper2 = function.lock(); + if (!strongWrapper2) { + return; + } + + std::vector args; + args.reserve(responses.size()); + for (const auto &val : responses) { + args.emplace_back( + jsi::valueFromDynamic(strongWrapper2->runtime(), val)); + } + + executor(strongWrapper2, args); + }); + + wrapperWasCalled = true; + }); } // This is used for generating short exception strings. @@ -907,17 +940,14 @@ jsi::Value JavaTurboModule::invokeJavaMethod( jsi::Function rejectJSIFn = promiseConstructorArgs[1].getObject(runtime).getFunction( runtime); - jsi::Function rejectInternalJSIFn = - promiseConstructorArgs[1].getObject(runtime).getFunction( - runtime); - auto rejectInternalWeakWrapper = - CallbackWrapper::createWeak(std::move(rejectInternalJSIFn), runtime, jsInvoker_); + auto rejectJSIFunctionWeakWrapper = + CallbackWrapper::createWeak(std::move(rejectJSIFn), runtime, jsInvoker_); auto resolve = createJavaCallbackFromJSIFunction( std::move(resolveJSIFn), runtime, jsInvoker_) .release(); auto reject = createPromiseRejectJavaCallbackFromJSIFunction( - std::move(rejectJSIFn), runtime, jsInvoker_, jsInvocationStack) + rejectJSIFunctionWeakWrapper, runtime, jsInvoker_, jsInvocationStack) .release(); jclass jPromiseImpl = @@ -945,7 +975,7 @@ jsi::Value JavaTurboModule::invokeJavaMethod( nativeMethodCallInvoker_->invokeAsync( methodName, [jargs, - rejectInternalWeakWrapper = rejectInternalWeakWrapper, + rejectJSIFunctionWeakWrapper, jsInvocationStack = std::move(jsInvocationStack), globalRefs, methodID, @@ -976,7 +1006,7 @@ jsi::Value JavaTurboModule::invokeJavaMethod( TMPL::asyncMethodCallExecutionFail( moduleName, methodName, id); auto exception = std::current_exception(); - rejectWithException(rejectInternalWeakWrapper, exception, jsInvocationStack); + rejectWithException(rejectJSIFunctionWeakWrapper, exception, jsInvocationStack); } for (auto globalRef : globalRefs) { From e1634a6ec92b5aa11ce69a67beecef1854d729f6 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Fri, 11 Aug 2023 23:23:36 +0200 Subject: [PATCH 23/55] Rename feature flag --- .../Libraries/AppDelegate/RCTAppSetupUtils.mm | 2 +- packages/react-native/React/Base/RCTBridge.h | 4 ++-- packages/react-native/React/Base/RCTBridge.mm | 10 +++++----- .../com/facebook/react/config/ReactFeatureFlags.java | 2 +- .../platform/android/ReactCommon/JavaTurboModule.cpp | 2 +- .../core/platform/ios/ReactCommon/RCTTurboModule.mm | 2 +- .../com/facebook/react/uiapp/RNTesterApplication.java | 2 +- 7 files changed, 12 insertions(+), 12 deletions(-) diff --git a/packages/react-native/Libraries/AppDelegate/RCTAppSetupUtils.mm b/packages/react-native/Libraries/AppDelegate/RCTAppSetupUtils.mm index 4897747c3f72..5c46b000e75f 100644 --- a/packages/react-native/Libraries/AppDelegate/RCTAppSetupUtils.mm +++ b/packages/react-native/Libraries/AppDelegate/RCTAppSetupUtils.mm @@ -57,7 +57,7 @@ void RCTAppSetupPrepareApp(UIApplication *application, BOOL turboModuleEnabled) #endif #if RCT_TRACE_PROMISE_REJECTION_ENABLED || DEBUG - RCTEnableTraceTurboModulePromiseRejection(YES); + RCTTraceTurboModulePromiseRejections(YES); #endif #if DEBUG diff --git a/packages/react-native/React/Base/RCTBridge.h b/packages/react-native/React/Base/RCTBridge.h index 0299dca2d492..06a6c7da51f7 100644 --- a/packages/react-native/React/Base/RCTBridge.h +++ b/packages/react-native/React/Base/RCTBridge.h @@ -65,8 +65,8 @@ RCT_EXTERN BOOL RCTTurboModuleInteropForAllTurboModulesEnabled(void); RCT_EXTERN void RCTEnableTurboModuleInteropForAllTurboModules(BOOL enabled); // Trace Rejected Promises of Turbo Modules (store callers' js stack) -RCT_EXTERN BOOL RCTTraceTurboModulePromiseRejectionEnabled(void); -RCT_EXTERN void RCTEnableTraceTurboModulePromiseRejection(BOOL enabled); +RCT_EXTERN BOOL RCTTraceTurboModulePromiseRejections(void); +RCT_EXTERN void RCTTraceTurboModulePromiseRejections(BOOL enabled); typedef enum { kRCTGlobalScope, diff --git a/packages/react-native/React/Base/RCTBridge.mm b/packages/react-native/React/Base/RCTBridge.mm index 3da565658ea8..8a06fb6de25d 100644 --- a/packages/react-native/React/Base/RCTBridge.mm +++ b/packages/react-native/React/Base/RCTBridge.mm @@ -103,15 +103,15 @@ BOOL RCTTurboModuleEagerInitEnabled(void) return turboModuleEagerInitEnabled; } -static BOOL traceTurboModulePromiseRejectionEnabled = NO; -BOOL RCTTraceTurboModulePromiseRejectionEnabled(void) +static BOOL traceTurboModulePromiseRejections = NO; +BOOL RCTTraceTurboModulePromiseRejections(void) { - return traceTurboModulePromiseRejectionEnabled; + return traceTurboModulePromiseRejections; } -void RCTEnableTraceTurboModulePromiseRejection(BOOL enabled) +void RCTTraceTurboModulePromiseRejections(BOOL enabled) { - traceTurboModulePromiseRejectionEnabled = enabled; + traceTurboModulePromiseRejections = enabled; } void RCTEnableTurboModuleEagerInit(BOOL enabled) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java index 0edb155d0972..23a4704baa78 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java @@ -165,5 +165,5 @@ public class ReactFeatureFlags { * Enables storing js caller stack when creating promise in native module. * This is useful in case of Promise rejection and tracing the cause. */ - public static boolean traceTurboModulePromiseRejectionEnabled = false; + public static boolean traceTurboModulePromiseRejections = false; } diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp index 926fb6c5708d..f126517dc075 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp @@ -69,7 +69,7 @@ static bool isSavePromiseJSInvocationStackEnabled() { #if DEBUG return true; #else - static bool savePromiseJSInvocationStack = getFeatureFlagBoolValue("traceTurboModulePromiseRejectionEnabled"); + static bool savePromiseJSInvocationStack = getFeatureFlagBoolValue("traceTurboModulePromiseRejections"); return savePromiseJSInvocationStack; #endif } diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm index 7c54f42ddcdb..15461f8d1b67 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm @@ -280,7 +280,7 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s jsi::Function Promise = runtime.global().getPropertyAsFunction(runtime, "Promise"); // JS Stack at the time when the promise is created. std::optional jsInvocationStack; - if (RCTTraceTurboModulePromiseRejectionEnabled()) { + if (RCTTraceTurboModulePromiseRejections()) { jsInvocationStack = createJSRuntimeError(runtime, "") .asObject(runtime) .getProperty(runtime, "stack") diff --git a/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterApplication.java b/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterApplication.java index dd059de93bc3..71aa51f59bfe 100644 --- a/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterApplication.java +++ b/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterApplication.java @@ -140,7 +140,7 @@ protected Boolean isHermesEnabled() { @Override public void onCreate() { - ReactFeatureFlags.traceTurboModulePromiseRejectionEnabled = true; + ReactFeatureFlags.traceTurboModulePromiseRejections = true; ReactFontManager.getInstance().addCustomFont(this, "Rubik", R.font.rubik); super.onCreate(); SoLoader.init(this, /* native exopackage */ false); From 65982e295defd7938c86d726bcc3e88ab82c2969 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Fri, 11 Aug 2023 23:29:59 +0200 Subject: [PATCH 24/55] fix flag func overload --- packages/react-native/Libraries/AppDelegate/RCTAppSetupUtils.mm | 2 +- packages/react-native/React/Base/RCTBridge.h | 2 +- packages/react-native/React/Base/RCTBridge.mm | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/react-native/Libraries/AppDelegate/RCTAppSetupUtils.mm b/packages/react-native/Libraries/AppDelegate/RCTAppSetupUtils.mm index 5c46b000e75f..712138f9fc61 100644 --- a/packages/react-native/Libraries/AppDelegate/RCTAppSetupUtils.mm +++ b/packages/react-native/Libraries/AppDelegate/RCTAppSetupUtils.mm @@ -57,7 +57,7 @@ void RCTAppSetupPrepareApp(UIApplication *application, BOOL turboModuleEnabled) #endif #if RCT_TRACE_PROMISE_REJECTION_ENABLED || DEBUG - RCTTraceTurboModulePromiseRejections(YES); + RCTEnableTraceTurboModulePromiseRejections(YES); #endif #if DEBUG diff --git a/packages/react-native/React/Base/RCTBridge.h b/packages/react-native/React/Base/RCTBridge.h index 06a6c7da51f7..51f7f41e2002 100644 --- a/packages/react-native/React/Base/RCTBridge.h +++ b/packages/react-native/React/Base/RCTBridge.h @@ -66,7 +66,7 @@ RCT_EXTERN void RCTEnableTurboModuleInteropForAllTurboModules(BOOL enabled); // Trace Rejected Promises of Turbo Modules (store callers' js stack) RCT_EXTERN BOOL RCTTraceTurboModulePromiseRejections(void); -RCT_EXTERN void RCTTraceTurboModulePromiseRejections(BOOL enabled); +RCT_EXTERN void RCTEnableTraceTurboModulePromiseRejections(BOOL enabled); typedef enum { kRCTGlobalScope, diff --git a/packages/react-native/React/Base/RCTBridge.mm b/packages/react-native/React/Base/RCTBridge.mm index 8a06fb6de25d..6d654215c33b 100644 --- a/packages/react-native/React/Base/RCTBridge.mm +++ b/packages/react-native/React/Base/RCTBridge.mm @@ -109,7 +109,7 @@ BOOL RCTTraceTurboModulePromiseRejections(void) return traceTurboModulePromiseRejections; } -void RCTTraceTurboModulePromiseRejections(BOOL enabled) +void RCTEnableTraceTurboModulePromiseRejections(BOOL enabled) { traceTurboModulePromiseRejections = enabled; } From ba06a6e3d46e6a96a94c623de0990cb6826b652b Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Fri, 11 Aug 2023 23:55:11 +0200 Subject: [PATCH 25/55] Update feature flag --- .../Libraries/AppDelegate/RCTAppSetupUtils.mm | 8 +++++++- .../core/platform/android/ReactCommon/JavaTurboModule.cpp | 4 ---- .../com/facebook/react/uiapp/RNTesterApplication.java | 4 +++- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/react-native/Libraries/AppDelegate/RCTAppSetupUtils.mm b/packages/react-native/Libraries/AppDelegate/RCTAppSetupUtils.mm index 712138f9fc61..e2a59450cf92 100644 --- a/packages/react-native/Libraries/AppDelegate/RCTAppSetupUtils.mm +++ b/packages/react-native/Libraries/AppDelegate/RCTAppSetupUtils.mm @@ -56,7 +56,13 @@ void RCTAppSetupPrepareApp(UIApplication *application, BOOL turboModuleEnabled) RCTEnableTurboModule(turboModuleEnabled); #endif -#if RCT_TRACE_PROMISE_REJECTION_ENABLED || DEBUG +#ifndef RCT_TRACE_PROMISE_REJECTION_ENABLED +#if DEBUG +#define RCT_TRACE_PROMISE_REJECTION_ENABLED 1 +#endif +#endif + +#if RCT_TRACE_PROMISE_REJECTION_ENABLED RCTEnableTraceTurboModulePromiseRejections(YES); #endif diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp index f126517dc075..5fd3ea26f3b7 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp @@ -66,12 +66,8 @@ static bool getFeatureFlagBoolValue(const char *name) { } static bool isSavePromiseJSInvocationStackEnabled() { -#if DEBUG - return true; -#else static bool savePromiseJSInvocationStack = getFeatureFlagBoolValue("traceTurboModulePromiseRejections"); return savePromiseJSInvocationStack; -#endif } struct JNIArgs { diff --git a/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterApplication.java b/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterApplication.java index 71aa51f59bfe..8ad9d3dc1514 100644 --- a/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterApplication.java +++ b/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterApplication.java @@ -140,7 +140,9 @@ protected Boolean isHermesEnabled() { @Override public void onCreate() { - ReactFeatureFlags.traceTurboModulePromiseRejections = true; + if (BuildConfig.DEBUG) { + ReactFeatureFlags.traceTurboModulePromiseRejections = true; + } ReactFontManager.getInstance().addCustomFont(this, "Rubik", R.font.rubik); super.onCreate(); SoLoader.init(this, /* native exopackage */ false); From b01e866dafa824fd09493756eb65bc3323544ee3 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Mon, 9 Oct 2023 17:30:17 +0200 Subject: [PATCH 26/55] clean up rct bridge --- packages/react-native/React/Base/RCTBridge.mm | 23 ------------------- 1 file changed, 23 deletions(-) diff --git a/packages/react-native/React/Base/RCTBridge.mm b/packages/react-native/React/Base/RCTBridge.mm index 7bdd41109365..4b38569cc4a5 100644 --- a/packages/react-native/React/Base/RCTBridge.mm +++ b/packages/react-native/React/Base/RCTBridge.mm @@ -97,12 +97,6 @@ void RCTEnableTurboModule(BOOL enabled) turboModuleEnabled = enabled; } -static BOOL turboModuleEagerInitEnabled = NO; -BOOL RCTTurboModuleEagerInitEnabled(void) -{ - return turboModuleEagerInitEnabled; -} - static BOOL traceTurboModulePromiseRejections = NO; BOOL RCTTraceTurboModulePromiseRejections(void) { @@ -114,23 +108,6 @@ void RCTEnableTraceTurboModulePromiseRejections(BOOL enabled) traceTurboModulePromiseRejections = enabled; } -void RCTEnableTurboModuleEagerInit(BOOL enabled) -{ - turboModuleEagerInitEnabled = enabled; -} - -// Turn off TurboModule delegate locking -static BOOL turboModuleManagerDelegateLockingDisabled = YES; -BOOL RCTTurboModuleManagerDelegateLockingDisabled(void) -{ - return turboModuleManagerDelegateLockingDisabled; -} - -void RCTDisableTurboModuleManagerDelegateLocking(BOOL disabled) -{ - turboModuleManagerDelegateLockingDisabled = disabled; -} - static BOOL turboModuleInteropEnabled = NO; BOOL RCTTurboModuleInteropEnabled(void) { From 2a3f8ea241e8162337fac56743a7d9921eee564a Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Mon, 9 Oct 2023 19:39:57 +0200 Subject: [PATCH 27/55] enable tracing turbo modules promises rejections in debug --- .../android/ReactCommon/JavaTurboModule.cpp | 17 +++++++++++++---- .../facebook/react/uiapp/RNTesterApplication.kt | 3 --- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp index 08865a8573b8..402edbfbf3b0 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp @@ -56,17 +56,26 @@ JavaTurboModule::~JavaTurboModule() { namespace { -constexpr static auto kReactFeatureFlagsJavaDescriptor = "com/facebook/react/config/ReactFeatureFlags"; +constexpr auto kReactBuildConfigJavaDescriptor = "com/facebook/react/common/build/ReactBuildConfig"; -static bool getFeatureFlagBoolValue(const char *name) { +bool getReactBuildConfigBoolValue(const char *name) { + static const auto reactBuildConfigClass = facebook::jni::findClassStatic(kReactBuildConfigJavaDescriptor); + const auto field = reactBuildConfigClass->getStaticField(name); + return reactBuildConfigClass->getStaticFieldValue(field); +} + +constexpr auto kReactFeatureFlagsJavaDescriptor = "com/facebook/react/config/ReactFeatureFlags"; + +bool getFeatureFlagBoolValue(const char *name) { static const auto reactFeatureFlagsClass = facebook::jni::findClassStatic(kReactFeatureFlagsJavaDescriptor); const auto field = reactFeatureFlagsClass->getStaticField(name); return reactFeatureFlagsClass->getStaticFieldValue(field); } -static bool isSavePromiseJSInvocationStackEnabled() { +bool isSavePromiseJSInvocationStackEnabled() { static bool savePromiseJSInvocationStack = getFeatureFlagBoolValue("traceTurboModulePromiseRejections"); - return savePromiseJSInvocationStack; + static bool isDebugMode = getReactBuildConfigBoolValue("DEBUG"); + return isDebugMode || savePromiseJSInvocationStack; } struct JNIArgs { diff --git a/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterApplication.kt b/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterApplication.kt index 6364c0986a41..871395ab7449 100644 --- a/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterApplication.kt +++ b/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterApplication.kt @@ -119,9 +119,6 @@ class RNTesterApplication : Application(), ReactApplication { } override fun onCreate() { - if (BuildConfig.DEBUG) { - ReactFeatureFlags.traceTurboModulePromiseRejections = true; - } ReactFontManager.getInstance().addCustomFont(this, "Rubik", R.font.rubik) super.onCreate() SoLoader.init(this, /* native exopackage */ false) From bd9b7f1b1b21b1369318b440a91d6fe23ce0b57d Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Mon, 9 Oct 2023 19:41:13 +0200 Subject: [PATCH 28/55] unify naming --- .../core/platform/android/ReactCommon/JavaTurboModule.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp index 402edbfbf3b0..67b445bdba18 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp @@ -72,7 +72,7 @@ bool getFeatureFlagBoolValue(const char *name) { return reactFeatureFlagsClass->getStaticFieldValue(field); } -bool isSavePromiseJSInvocationStackEnabled() { +bool traceTurboModulePromiseRejections() { static bool savePromiseJSInvocationStack = getFeatureFlagBoolValue("traceTurboModulePromiseRejections"); static bool isDebugMode = getReactBuildConfigBoolValue("DEBUG"); return isDebugMode || savePromiseJSInvocationStack; @@ -930,7 +930,7 @@ jsi::Value JavaTurboModule::invokeJavaMethod( // JS Stack at the time when the promise is created. std::optional jsInvocationStack; - if (isSavePromiseJSInvocationStackEnabled()) { + if (traceTurboModulePromiseRejections()) { jsInvocationStack = createJSRuntimeError(runtime, "") .asObject(runtime) .getProperty(runtime, "stack") From 3f81d5ab660bb9264087b606b9dd712630784555 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Tue, 10 Oct 2023 10:26:25 +0200 Subject: [PATCH 29/55] Use jsi types for callback executor --- .../android/ReactCommon/JavaTurboModule.cpp | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp index 67b445bdba18..b04acfa7cbc2 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp @@ -84,13 +84,13 @@ struct JNIArgs { std::vector globalRefs_; }; -using CallbackWrapperExecutor = std::function&, const std::vector&)>; +using CallbackExecutor = std::function&)>; jni::local_ref createJavaCallbackFromJSIFunction( jsi::Function &&function, jsi::Runtime &rt, const std::shared_ptr &jsInvoker, - CallbackWrapperExecutor&& callbackWrapperExecutor) { + CallbackExecutor&& callbackExecutor) { auto weakWrapper = CallbackWrapper::createWeak(std::move(function), rt, jsInvoker); @@ -107,7 +107,7 @@ jni::local_ref createJavaCallbackFromJSIFunction( [weakWrapper = std::move(weakWrapper), callbackWrapperOwner = std::move(callbackWrapperOwner), wrapperWasCalled = false, - callbackWrapperExecutor = std::move(callbackWrapperExecutor)](folly::dynamic responses) mutable { + callbackExecutor = std::move(callbackExecutor)](folly::dynamic responses) mutable { if (wrapperWasCalled) { LOG(FATAL) << "callback arg cannot be called more than once"; } @@ -121,7 +121,7 @@ jni::local_ref createJavaCallbackFromJSIFunction( [weakWrapper = std::move(weakWrapper), callbackWrapperOwner = std::move(callbackWrapperOwner), responses = std::move(responses), - callbackWrapperExecutor = std::move(callbackWrapperExecutor)]() { + callbackExecutor = std::move(callbackExecutor)]() { auto strongWrapper2 = weakWrapper.lock(); if (!strongWrapper2) { return; @@ -134,7 +134,7 @@ jni::local_ref createJavaCallbackFromJSIFunction( jsi::valueFromDynamic(strongWrapper2->runtime(), val)); } - callbackWrapperExecutor(strongWrapper2, args); + callbackExecutor(strongWrapper2->runtime(), strongWrapper2->callback(), args); }); wrapperWasCalled = true; @@ -145,11 +145,12 @@ jni::local_ref createJavaCallbackFromJSIFunction( jsi::Function &&function, jsi::Runtime &rt, const std::shared_ptr &jsInvoker) { - auto executor = []( - const std::shared_ptr& wrapper, + CallbackExecutor executor = []( + jsi::Runtime& runtime, + jsi::Function& callback, const std::vector& args) { - wrapper->callback().call( - wrapper->runtime(), + callback.call( + runtime, (const jsi::Value *)args.data(), args.size()); }; From 193617476b8b640435e988044a984969992ab249 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Tue, 10 Oct 2023 11:22:57 +0200 Subject: [PATCH 30/55] access cause safe --- .../android/ReactCommon/JavaTurboModule.cpp | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp index b04acfa7cbc2..337f613a7dbf 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp @@ -167,17 +167,20 @@ jni::local_ref createPromiseRejectJavaCallbackFromJS auto executor = [jsInvocationStack = std::move(jsInvocationStack)]( const std::shared_ptr& reject, const std::vector& args) { - std::string message; + std::optional message = std::nullopt; jsi::Runtime &rt2 = reject->runtime(); - const jsi::Value* cause = args.data(); - if (cause->isObject() && cause->asObject(rt2).getProperty(rt2, "message").isString()) { - message = cause->asObject(rt2).getProperty(rt2, "message").asString(rt2).utf8(rt2); - } else { - message = ""; + + if (!args.empty()) { + const jsi::Value &cause = args[0]; + if (cause.isObject() && cause.asObject(rt2).getProperty(rt2, "message").isString()) { + message = cause.asObject(rt2).getProperty(rt2, "message").asString(rt2).utf8(rt2); + } } - jsi::Value error = createJSRuntimeError(rt2, "Exception in HostFunction: " + message); - error.asObject(rt2).setProperty(rt2, "cause", *cause); + jsi::Value error = createJSRuntimeError(rt2, "Exception in HostFunction: " + (message.has_value() ? message.value() : "")); + if (!args.empty()) { + error.asObject(rt2).setProperty(rt2, "cause", args[0]); + } error.asObject(rt2).setProperty(rt2, "stack", *jsInvocationStack); reject->callback().call(rt2, error); }; From 975a511baccb889e084c5b3f69c9f2f5638dd071 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Tue, 10 Oct 2023 12:35:08 +0200 Subject: [PATCH 31/55] keep js invocation stack as property --- .../android/ReactCommon/JavaTurboModule.cpp | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp index 337f613a7dbf..115ba3c8afe2 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp @@ -161,10 +161,8 @@ jsi::Value createJSRuntimeError(jsi::Runtime &runtime, const std::string &messag jni::local_ref createPromiseRejectJavaCallbackFromJSIFunction( const std::weak_ptr &function, - jsi::Runtime &rt, - const std::shared_ptr &jsInvoker, - std::optional jsInvocationStack) { - auto executor = [jsInvocationStack = std::move(jsInvocationStack)]( + std::optional> jsInvocationStack) { + auto executor = [jsInvocationStack]( const std::shared_ptr& reject, const std::vector& args) { std::optional message = std::nullopt; @@ -181,7 +179,9 @@ jni::local_ref createPromiseRejectJavaCallbackFromJS if (!args.empty()) { error.asObject(rt2).setProperty(rt2, "cause", args[0]); } - error.asObject(rt2).setProperty(rt2, "stack", *jsInvocationStack); + if (jsInvocationStack.has_value()) { + error.asObject(rt2).setProperty(rt2, "stack", jsInvocationStack.value().get()); + } reject->callback().call(rt2, error); }; return JCxxCallbackImpl::newObjectCxxArgs( @@ -547,7 +547,7 @@ jsi::JSError convertThrowableToJSError( void rejectWithException( std::weak_ptr &rejectWeak, std::exception_ptr &exception, - std::optional jsInvocationStack) { + std::optional> jsInvocationStack) { auto reject = rejectWeak.lock(); if (reject) { reject->jsInvoker().invokeAsync([ @@ -561,7 +561,7 @@ void rejectWithException( auto throwable = jni::getJavaExceptionForCppException(exception); auto jsError = convertThrowableToJSError(runtime, throwable); if (jsInvocationStack.has_value()) { - jsError.value().asObject(runtime).setProperty(runtime, "stack", *jsInvocationStack); + jsError.value().asObject(runtime).setProperty(runtime, "stack", jsInvocationStack.value().get()); } reject2->callback().call( runtime, @@ -933,13 +933,12 @@ jsi::Value JavaTurboModule::invokeJavaMethod( } // JS Stack at the time when the promise is created. - std::optional jsInvocationStack; + std::optional> jsInvocationStack = std::nullopt; if (traceTurboModulePromiseRejections()) { - jsInvocationStack = createJSRuntimeError(runtime, "") + auto stack = createJSRuntimeError(runtime, "") .asObject(runtime) - .getProperty(runtime, "stack") - .asString(runtime) - .utf8(runtime); + .getProperty(runtime, "stack"); + jsInvocationStack = std::ref(stack); } jsi::Function resolveJSIFn = @@ -955,7 +954,7 @@ jsi::Value JavaTurboModule::invokeJavaMethod( std::move(resolveJSIFn), runtime, jsInvoker_) .release(); auto reject = createPromiseRejectJavaCallbackFromJSIFunction( - rejectJSIFunctionWeakWrapper, runtime, jsInvoker_, jsInvocationStack) + rejectJSIFunctionWeakWrapper, jsInvocationStack) .release(); jclass jPromiseImpl = From bf584da35048a14a71351c41c5b6fdc415f1e177 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Tue, 10 Oct 2023 13:55:41 +0200 Subject: [PATCH 32/55] Hold reference to stack to avoid parsing string --- .../android/ReactCommon/JavaTurboModule.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp index 115ba3c8afe2..886b71c6ef76 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp @@ -161,7 +161,7 @@ jsi::Value createJSRuntimeError(jsi::Runtime &runtime, const std::string &messag jni::local_ref createPromiseRejectJavaCallbackFromJSIFunction( const std::weak_ptr &function, - std::optional> jsInvocationStack) { + std::optional> jsInvocationStack) { auto executor = [jsInvocationStack]( const std::shared_ptr& reject, const std::vector& args) { @@ -180,7 +180,7 @@ jni::local_ref createPromiseRejectJavaCallbackFromJS error.asObject(rt2).setProperty(rt2, "cause", args[0]); } if (jsInvocationStack.has_value()) { - error.asObject(rt2).setProperty(rt2, "stack", jsInvocationStack.value().get()); + error.asObject(rt2).setProperty(rt2, "stack", *jsInvocationStack.value()); } reject->callback().call(rt2, error); }; @@ -547,7 +547,7 @@ jsi::JSError convertThrowableToJSError( void rejectWithException( std::weak_ptr &rejectWeak, std::exception_ptr &exception, - std::optional> jsInvocationStack) { + std::optional> jsInvocationStack) { auto reject = rejectWeak.lock(); if (reject) { reject->jsInvoker().invokeAsync([ @@ -561,7 +561,7 @@ void rejectWithException( auto throwable = jni::getJavaExceptionForCppException(exception); auto jsError = convertThrowableToJSError(runtime, throwable); if (jsInvocationStack.has_value()) { - jsError.value().asObject(runtime).setProperty(runtime, "stack", jsInvocationStack.value().get()); + jsError.value().asObject(runtime).setProperty(runtime, "stack", *jsInvocationStack.value()); } reject2->callback().call( runtime, @@ -933,12 +933,12 @@ jsi::Value JavaTurboModule::invokeJavaMethod( } // JS Stack at the time when the promise is created. - std::optional> jsInvocationStack = std::nullopt; + std::optional> jsInvocationStack = std::nullopt; if (traceTurboModulePromiseRejections()) { auto stack = createJSRuntimeError(runtime, "") - .asObject(runtime) - .getProperty(runtime, "stack"); - jsInvocationStack = std::ref(stack); + .asObject(runtime) + .getProperty(runtime, "stack"); + jsInvocationStack = std::make_shared(std::move(stack)); } jsi::Function resolveJSIFn = From 3937f61f31eac47cbb133aaf7de1f230f4efaa92 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Tue, 10 Oct 2023 13:57:15 +0200 Subject: [PATCH 33/55] Add explanation comment --- .../core/platform/android/ReactCommon/JavaTurboModule.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp index 886b71c6ef76..cc1a18f58b73 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp @@ -933,6 +933,7 @@ jsi::Value JavaTurboModule::invokeJavaMethod( } // JS Stack at the time when the promise is created. + // This has to be shared pointer to hold jsi::Value std::optional> jsInvocationStack = std::nullopt; if (traceTurboModulePromiseRejections()) { auto stack = createJSRuntimeError(runtime, "") From c735ba9e4ac0e77f94b6323b0dfbcfb57aa05792 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Tue, 10 Oct 2023 14:25:44 +0200 Subject: [PATCH 34/55] Add test errors logs for easier verification --- .../TurboModule/SampleTurboModuleExample.js | 27 ++++++++++++------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/packages/rn-tester/js/examples/TurboModule/SampleTurboModuleExample.js b/packages/rn-tester/js/examples/TurboModule/SampleTurboModuleExample.js index b79c96eba751..5331195cf0a1 100644 --- a/packages/rn-tester/js/examples/TurboModule/SampleTurboModuleExample.js +++ b/packages/rn-tester/js/examples/TurboModule/SampleTurboModuleExample.js @@ -54,7 +54,10 @@ class SampleTurboModuleExample extends React.Component<{||}, State> { rejectPromise: () => NativeSampleTurboModule.getValueWithPromise(true) .then(() => {}) - .catch(e => this._setResult('rejectPromise', e.message)), + .catch(e => { + this._setResult('rejectPromise', e.message); + console.error(e, e.stack, e.cause); + }), getConstants: () => NativeSampleTurboModule.getConstants(), voidFunc: () => NativeSampleTurboModule.voidFunc(), getBool: () => NativeSampleTurboModule.getBool(true), @@ -81,6 +84,7 @@ class SampleTurboModuleExample extends React.Component<{||}, State> { try { NativeSampleTurboModule.voidFuncThrows?.(); } catch (e) { + console.error(e, e.stack, e.cause); return e.message; } }, @@ -88,18 +92,23 @@ class SampleTurboModuleExample extends React.Component<{||}, State> { try { NativeSampleTurboModule.getObjectThrows?.({a: 1, b: 'foo', c: null}); } catch (e) { + console.error(e, e.stack, e.cause); return e.message; } }, promiseThrows: () => { NativeSampleTurboModule.promiseThrows?.() .then(() => {}) - .catch(e => this._setResult('promiseThrows', e.message)); + .catch(e => { + this._setResult('promiseThrows', e.message); + console.error(e, e.stack, e.cause); + }); }, voidFuncAssert: () => { try { NativeSampleTurboModule.voidFuncAssert?.(); } catch (e) { + console.error(e, e.stack, e.cause); return e.message; } }, @@ -107,17 +116,17 @@ class SampleTurboModuleExample extends React.Component<{||}, State> { try { NativeSampleTurboModule.getObjectAssert?.({a: 1, b: 'foo', c: null}); } catch (e) { + console.error(e, e.stack, e.cause); return e.message; } }, promiseAssert: () => { - try { - NativeSampleTurboModule.promiseAssert?.() - .then(() => {}) - .catch(e => this._setResult('promiseAssert', e.message)); - } catch (e) { - return e.message; - } + NativeSampleTurboModule.promiseAssert?.() + .then(() => {}) + .catch(e => { + this._setResult('promiseAssert', e.message); + console.error(e, e.stack, e.cause); + }); }, }; From 7e84c08ef6cc7624f922f2558dd40bec1aa5da8f Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Tue, 10 Oct 2023 14:25:59 +0200 Subject: [PATCH 35/55] Add rejectTurboModulePromiseOnNativeError flag --- .../react/config/ReactFeatureFlags.java | 7 +++++++ .../android/ReactCommon/JavaTurboModule.cpp | 17 +++++++++++++---- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java index 4180a045bae7..da8643c03dde 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java @@ -171,4 +171,11 @@ public class ReactFeatureFlags { * This is useful in case of Promise rejection and tracing the cause. */ public static boolean traceTurboModulePromiseRejections = false; + + /** + * Enables auto rejecting promises from Turbo Modules + * method calls. If native error occurs Promise in JS + * will be rejected (The JS error will include native stack) + */ + public static boolean rejectTurboModulePromiseOnNativeError = true; } diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp index cc1a18f58b73..a8942343183b 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp @@ -73,9 +73,14 @@ bool getFeatureFlagBoolValue(const char *name) { } bool traceTurboModulePromiseRejections() { - static bool savePromiseJSInvocationStack = getFeatureFlagBoolValue("traceTurboModulePromiseRejections"); + static bool traceRejections = getFeatureFlagBoolValue("traceTurboModulePromiseRejections"); static bool isDebugMode = getReactBuildConfigBoolValue("DEBUG"); - return isDebugMode || savePromiseJSInvocationStack; + return isDebugMode || traceRejections; +} + +bool rejectTurboModulePromiseOnNativeError() { + static bool rejectOnError = getFeatureFlagBoolValue("rejectTurboModulePromiseOnNativeError"); + return rejectOnError; } struct JNIArgs { @@ -1013,8 +1018,12 @@ jsi::Value JavaTurboModule::invokeJavaMethod( } catch (...) { TMPL::asyncMethodCallExecutionFail( moduleName, methodName, id); - auto exception = std::current_exception(); - rejectWithException(rejectJSIFunctionWeakWrapper, exception, jsInvocationStack); + if (rejectTurboModulePromiseOnNativeError()) { + auto exception = std::current_exception(); + rejectWithException(rejectJSIFunctionWeakWrapper, exception, jsInvocationStack); + } else { + throw; + } } for (auto globalRef : globalRefs) { From c9c099be056c759af122a57daa30fac47423e96d Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Tue, 10 Oct 2023 14:43:32 +0200 Subject: [PATCH 36/55] Drop java changes --- .../react/config/ReactFeatureFlags.java | 13 - .../android/ReactCommon/JavaTurboModule.cpp | 320 +++++------------- 2 files changed, 84 insertions(+), 249 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java index da8643c03dde..3a514917cc7c 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java @@ -165,17 +165,4 @@ public class ReactFeatureFlags { /** Enables Stable API for TurboModule (removal of ReactModule, ReactModuleInfoProvider). */ public static boolean enableTurboModuleStableAPI = false; - - /** - * Enables storing js caller stack when creating promise in native module. - * This is useful in case of Promise rejection and tracing the cause. - */ - public static boolean traceTurboModulePromiseRejections = false; - - /** - * Enables auto rejecting promises from Turbo Modules - * method calls. If native error occurs Promise in JS - * will be rejected (The JS error will include native stack) - */ - public static boolean rejectTurboModulePromiseOnNativeError = true; } diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp index a8942343183b..477d144773b8 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp @@ -10,12 +10,12 @@ #include #include -#include #include #include #include #include +#include #include #include #include @@ -28,10 +28,10 @@ namespace facebook::react { namespace TMPL = TurboModulePerfLogger; -JavaTurboModule::JavaTurboModule(const InitParams& params) +JavaTurboModule::JavaTurboModule(const InitParams ¶ms) : TurboModule(params.moduleName, params.jsInvoker), instance_(jni::make_global(params.instance)), - nativeMethodCallInvoker_(params.nativeMethodCallInvoker) {} + nativeInvoker_(params.nativeInvoker) {} JavaTurboModule::~JavaTurboModule() { /** @@ -42,60 +42,29 @@ JavaTurboModule::~JavaTurboModule() { return; } - nativeMethodCallInvoker_->invokeAsync( - "~" + name_, [instance = std::move(instance_)]() mutable { - /** - * Reset the global NativeModule ref on the NativeModules thread. Why: - * - ~JavaTurboModule() can be called on a non-JVM thread. If we reset - * the global ref in ~JavaTurboModule(), we might access the JVM from a - * non-JVM thread, which will crash the app. - */ - instance.reset(); - }); + nativeInvoker_->invokeAsync([instance = std::move(instance_)]() mutable { + /** + * Reset the global NativeModule ref on the NativeModules thread. Why: + * - ~JavaTurboModule() can be called on a non-JVM thread. If we reset the + * global ref in ~JavaTurboModule(), we might access the JVM from a + * non-JVM thread, which will crash the app. + */ + instance.reset(); + }); } namespace { -constexpr auto kReactBuildConfigJavaDescriptor = "com/facebook/react/common/build/ReactBuildConfig"; - -bool getReactBuildConfigBoolValue(const char *name) { - static const auto reactBuildConfigClass = facebook::jni::findClassStatic(kReactBuildConfigJavaDescriptor); - const auto field = reactBuildConfigClass->getStaticField(name); - return reactBuildConfigClass->getStaticFieldValue(field); -} - -constexpr auto kReactFeatureFlagsJavaDescriptor = "com/facebook/react/config/ReactFeatureFlags"; - -bool getFeatureFlagBoolValue(const char *name) { - static const auto reactFeatureFlagsClass = facebook::jni::findClassStatic(kReactFeatureFlagsJavaDescriptor); - const auto field = reactFeatureFlagsClass->getStaticField(name); - return reactFeatureFlagsClass->getStaticFieldValue(field); -} - -bool traceTurboModulePromiseRejections() { - static bool traceRejections = getFeatureFlagBoolValue("traceTurboModulePromiseRejections"); - static bool isDebugMode = getReactBuildConfigBoolValue("DEBUG"); - return isDebugMode || traceRejections; -} - -bool rejectTurboModulePromiseOnNativeError() { - static bool rejectOnError = getFeatureFlagBoolValue("rejectTurboModulePromiseOnNativeError"); - return rejectOnError; -} - struct JNIArgs { JNIArgs(size_t count) : args_(count) {} std::vector args_; std::vector globalRefs_; }; -using CallbackExecutor = std::function&)>; - jni::local_ref createJavaCallbackFromJSIFunction( jsi::Function &&function, jsi::Runtime &rt, - const std::shared_ptr &jsInvoker, - CallbackExecutor&& callbackExecutor) { + const std::shared_ptr &jsInvoker) { auto weakWrapper = CallbackWrapper::createWeak(std::move(function), rt, jsInvoker); @@ -111,10 +80,10 @@ jni::local_ref createJavaCallbackFromJSIFunction( return JCxxCallbackImpl::newObjectCxxArgs( [weakWrapper = std::move(weakWrapper), callbackWrapperOwner = std::move(callbackWrapperOwner), - wrapperWasCalled = false, - callbackExecutor = std::move(callbackExecutor)](folly::dynamic responses) mutable { + wrapperWasCalled = false](folly::dynamic responses) mutable { if (wrapperWasCalled) { - LOG(FATAL) << "callback arg cannot be called more than once"; + throw std::runtime_error( + "Callback arg cannot be called more than once"); } auto strongWrapper = weakWrapper.lock(); @@ -125,8 +94,7 @@ jni::local_ref createJavaCallbackFromJSIFunction( strongWrapper->jsInvoker().invokeAsync( [weakWrapper = std::move(weakWrapper), callbackWrapperOwner = std::move(callbackWrapperOwner), - responses = std::move(responses), - callbackExecutor = std::move(callbackExecutor)]() { + responses = std::move(responses)]() { auto strongWrapper2 = weakWrapper.lock(); if (!strongWrapper2) { return; @@ -134,99 +102,23 @@ jni::local_ref createJavaCallbackFromJSIFunction( std::vector args; args.reserve(responses.size()); - for (const auto& val : responses) { + for (const auto &val : responses) { args.emplace_back( jsi::valueFromDynamic(strongWrapper2->runtime(), val)); } - callbackExecutor(strongWrapper2->runtime(), strongWrapper2->callback(), args); + strongWrapper2->callback().call( + strongWrapper2->runtime(), + (const jsi::Value *)args.data(), + args.size()); }); wrapperWasCalled = true; }); } -jni::local_ref createJavaCallbackFromJSIFunction( - jsi::Function &&function, - jsi::Runtime &rt, - const std::shared_ptr &jsInvoker) { - CallbackExecutor executor = []( - jsi::Runtime& runtime, - jsi::Function& callback, - const std::vector& args) { - callback.call( - runtime, - (const jsi::Value *)args.data(), - args.size()); - }; - return createJavaCallbackFromJSIFunction(std::move(function), rt, jsInvoker, std::move(executor)); -} - -jsi::Value createJSRuntimeError(jsi::Runtime &runtime, const std::string &message); - -jni::local_ref createPromiseRejectJavaCallbackFromJSIFunction( - const std::weak_ptr &function, - std::optional> jsInvocationStack) { - auto executor = [jsInvocationStack]( - const std::shared_ptr& reject, - const std::vector& args) { - std::optional message = std::nullopt; - jsi::Runtime &rt2 = reject->runtime(); - - if (!args.empty()) { - const jsi::Value &cause = args[0]; - if (cause.isObject() && cause.asObject(rt2).getProperty(rt2, "message").isString()) { - message = cause.asObject(rt2).getProperty(rt2, "message").asString(rt2).utf8(rt2); - } - } - - jsi::Value error = createJSRuntimeError(rt2, "Exception in HostFunction: " + (message.has_value() ? message.value() : "")); - if (!args.empty()) { - error.asObject(rt2).setProperty(rt2, "cause", args[0]); - } - if (jsInvocationStack.has_value()) { - error.asObject(rt2).setProperty(rt2, "stack", *jsInvocationStack.value()); - } - reject->callback().call(rt2, error); - }; - return JCxxCallbackImpl::newObjectCxxArgs( - [function, - wrapperWasCalled = false, - executor = std::move(executor)](folly::dynamic responses) mutable { - if (wrapperWasCalled) { - LOG(FATAL) << "callback arg cannot be called more than once"; - } - - auto strongWrapper = function.lock(); - if (!strongWrapper) { - return; - } - - strongWrapper->jsInvoker().invokeAsync( - [function, - responses = std::move(responses), - executor = std::move(executor)]() { - auto strongWrapper2 = function.lock(); - if (!strongWrapper2) { - return; - } - - std::vector args; - args.reserve(responses.size()); - for (const auto &val : responses) { - args.emplace_back( - jsi::valueFromDynamic(strongWrapper2->runtime(), val)); - } - - executor(strongWrapper2, args); - }); - - wrapperWasCalled = true; - }); -} - // This is used for generating short exception strings. -std::string stringifyJSIValue(const jsi::Value& v, jsi::Runtime* rt = nullptr) { +std::string stringifyJSIValue(const jsi::Value &v, jsi::Runtime *rt = nullptr) { if (v.isUndefined()) { return "undefined"; } @@ -255,11 +147,11 @@ std::string stringifyJSIValue(const jsi::Value& v, jsi::Runtime* rt = nullptr) { class JavaTurboModuleArgumentConversionException : public std::runtime_error { public: JavaTurboModuleArgumentConversionException( - const std::string& expectedType, + const std::string &expectedType, int index, - const std::string& methodName, - const jsi::Value* arg, - jsi::Runtime* rt) + const std::string &methodName, + const jsi::Value *arg, + jsi::Runtime *rt) : std::runtime_error( "Expected argument " + std::to_string(index) + " of method \"" + methodName + "\" to be a " + expectedType + ", but got " + @@ -269,9 +161,9 @@ class JavaTurboModuleArgumentConversionException : public std::runtime_error { class JavaTurboModuleInvalidArgumentTypeException : public std::runtime_error { public: JavaTurboModuleInvalidArgumentTypeException( - const std::string& actualType, + const std::string &actualType, int argIndex, - const std::string& methodName) + const std::string &methodName) : std::runtime_error( "Called method \"" + methodName + "\" with unsupported type " + actualType + " at argument " + std::to_string(argIndex)) {} @@ -280,7 +172,7 @@ class JavaTurboModuleInvalidArgumentTypeException : public std::runtime_error { class JavaTurboModuleInvalidArgumentCountException : public std::runtime_error { public: JavaTurboModuleInvalidArgumentCountException( - const std::string& methodName, + const std::string &methodName, int actualArgCount, int expectedArgCount) : std::runtime_error( @@ -296,7 +188,7 @@ class JavaTurboModuleInvalidArgumentCountException : public std::runtime_error { * for a description of Java method signature structure. */ std::vector getMethodArgTypesFromSignature( - const std::string& methodSignature) { + const std::string &methodSignature) { std::vector methodArgs; for (auto it = methodSignature.begin(); it != methodSignature.end(); @@ -344,13 +236,13 @@ int32_t getUniqueId() { // TODO (axe) Reuse existing implementation as needed - the exist in // MethodInvoker.cpp JNIArgs convertJSIArgsToJNIArgs( - JNIEnv* env, - jsi::Runtime& rt, - const std::string& methodName, - const std::vector& methodArgTypes, - const jsi::Value* args, + JNIEnv *env, + jsi::Runtime &rt, + const std::string &methodName, + const std::vector &methodArgTypes, + const jsi::Value *args, size_t count, - const std::shared_ptr& jsInvoker, + const std::shared_ptr &jsInvoker, TurboModuleMethodValueKind valueKind) { unsigned int expectedArgumentCount = valueKind == PromiseKind ? methodArgTypes.size() - 1 @@ -362,8 +254,8 @@ JNIArgs convertJSIArgsToJNIArgs( } JNIArgs jniArgs(valueKind == PromiseKind ? count + 1 : count); - auto& jargs = jniArgs.args_; - auto& globalRefs = jniArgs.globalRefs_; + auto &jargs = jniArgs.args_; + auto &globalRefs = jniArgs.globalRefs_; auto makeGlobalIfNecessary = [&globalRefs, env, valueKind](jobject obj) -> jobject { @@ -378,10 +270,10 @@ JNIArgs convertJSIArgsToJNIArgs( }; for (unsigned int argIndex = 0; argIndex < count; argIndex += 1) { - const std::string& type = methodArgTypes.at(argIndex); + const std::string &type = methodArgTypes.at(argIndex); - const jsi::Value* arg = &args[argIndex]; - jvalue* jarg = &jargs[argIndex]; + const jsi::Value *arg = &args[argIndex]; + jvalue *jarg = &jargs[argIndex]; if (type == "D") { if (!arg->isNumber()) { @@ -492,7 +384,7 @@ JNIArgs convertJSIArgsToJNIArgs( return jniArgs; } -jsi::Value convertFromJMapToValue(JNIEnv* env, jsi::Runtime& rt, jobject arg) { +jsi::Value convertFromJMapToValue(JNIEnv *env, jsi::Runtime &rt, jobject arg) { // We currently use Java Argument.makeNativeMap() method to do this conversion // This could also be done purely in C++, but iterative over map methods // but those may end up calling reflection methods anyway @@ -509,8 +401,8 @@ jsi::Value convertFromJMapToValue(JNIEnv* env, jsi::Runtime& rt, jobject arg) { } jsi::Value createJSRuntimeError( - jsi::Runtime& runtime, - const std::string& message) { + jsi::Runtime &runtime, + const std::string &message) { return runtime.global() .getPropertyAsFunction(runtime, "Error") .call(runtime, message); @@ -520,7 +412,7 @@ jsi::Value createJSRuntimeError( * Creates JSError with current JS runtime stack and Throwable stack trace. */ jsi::JSError convertThrowableToJSError( - jsi::Runtime& runtime, + jsi::Runtime &runtime, jni::local_ref throwable) { auto stackTrace = throwable->getStackTrace(); @@ -537,9 +429,15 @@ jsi::JSError convertThrowableToJSError( } jsi::Object cause(runtime); - auto name = throwable->getClass()->getCanonicalName()->toStdString(); - auto message = throwable->getMessage()->toStdString(); - cause.setProperty(runtime, "name", name); + auto getName = throwable->getClass() + ->getClass() + ->getMethod()>("getName"); + auto getMessage = + throwable->getClass()->getMethod()>( + "getMessage"); + auto message = getMessage(throwable)->toStdString(); + cause.setProperty( + runtime, "name", getName(throwable->getClass())->toStdString()); cause.setProperty(runtime, "message", message); cause.setProperty(runtime, "stackElements", std::move(stackElements)); @@ -549,47 +447,18 @@ jsi::JSError convertThrowableToJSError( return {runtime, std::move(error)}; } -void rejectWithException( - std::weak_ptr &rejectWeak, - std::exception_ptr &exception, - std::optional> jsInvocationStack) { - auto reject = rejectWeak.lock(); - if (reject) { - reject->jsInvoker().invokeAsync([ - rejectWeak = std::move(rejectWeak), - jsInvocationStack = std::move(jsInvocationStack), - exception - ]() { - auto reject2 = rejectWeak.lock(); - if (reject2) { - jsi::Runtime &runtime = reject2->runtime(); - auto throwable = jni::getJavaExceptionForCppException(exception); - auto jsError = convertThrowableToJSError(runtime, throwable); - if (jsInvocationStack.has_value()) { - jsError.value().asObject(runtime).setProperty(runtime, "stack", *jsInvocationStack.value()); - } - reject2->callback().call( - runtime, - jsError.value()); - } - }); - } else { - throw; - } -} - } // namespace jsi::Value JavaTurboModule::invokeJavaMethod( - jsi::Runtime& runtime, + jsi::Runtime &runtime, TurboModuleMethodValueKind valueKind, - const std::string& methodNameStr, - const std::string& methodSignature, - const jsi::Value* args, + const std::string &methodNameStr, + const std::string &methodSignature, + const jsi::Value *args, size_t argCount, - jmethodID& methodID) { - const char* methodName = methodNameStr.c_str(); - const char* moduleName = name_.c_str(); + jmethodID &methodID) { + const char *methodName = methodNameStr.c_str(); + const char *moduleName = name_.c_str(); bool isMethodSync = !(valueKind == VoidKind || valueKind == PromiseKind); @@ -601,7 +470,7 @@ jsi::Value JavaTurboModule::invokeJavaMethod( TMPL::asyncMethodCallArgConversionStart(moduleName, methodName); } - JNIEnv* env = jni::Environment::current(); + JNIEnv *env = jni::Environment::current(); auto instance = instance_.get(); /** @@ -698,8 +567,8 @@ jsi::Value JavaTurboModule::invokeJavaMethod( TMPL::syncMethodCallExecutionStart(moduleName, methodName); } - auto& jargs = jniArgs.args_; - auto& globalRefs = jniArgs.globalRefs_; + auto &jargs = jniArgs.args_; + auto &globalRefs = jniArgs.globalRefs_; switch (valueKind) { case BooleanKind: { @@ -818,7 +687,7 @@ jsi::Value JavaTurboModule::invokeJavaMethod( jsi::Value returnValue = jsi::Value::null(); if (returnString != nullptr) { - const char* js = env->GetStringUTFChars(returnString, nullptr); + const char *js = env->GetStringUTFChars(returnString, nullptr); std::string result = js; env->ReleaseStringUTFChars(returnString, js); returnValue = @@ -873,8 +742,7 @@ jsi::Value JavaTurboModule::invokeJavaMethod( TMPL::asyncMethodCallArgConversionEnd(moduleName, methodName); TMPL::asyncMethodCallDispatch(moduleName, methodName); - nativeMethodCallInvoker_->invokeAsync( - methodName, + nativeInvoker_->invokeAsync( [jargs, globalRefs, methodID, @@ -891,9 +759,9 @@ jsi::Value JavaTurboModule::invokeJavaMethod( * again? Why does JNI crash when we use the env from the upper * scope? */ - JNIEnv* env = jni::Environment::current(); - const char* moduleName = moduleNameStr.c_str(); - const char* methodName = methodNameStr.c_str(); + JNIEnv *env = jni::Environment::current(); + const char *moduleName = moduleNameStr.c_str(); + const char *methodName = methodNameStr.c_str(); TMPL::asyncMethodCallExecutionStart(moduleName, methodName, id); env->CallVoidMethodA(instance.get(), methodID, jargs.data()); @@ -929,38 +797,26 @@ jsi::Value JavaTurboModule::invokeJavaMethod( moduleNameStr = name_, methodNameStr, env]( - jsi::Runtime& runtime, - const jsi::Value& thisVal, - const jsi::Value* promiseConstructorArgs, + jsi::Runtime &runtime, + const jsi::Value &thisVal, + const jsi::Value *promiseConstructorArgs, size_t promiseConstructorArgCount) { if (promiseConstructorArgCount != 2) { throw std::invalid_argument("Promise fn arg count must be 2"); } - // JS Stack at the time when the promise is created. - // This has to be shared pointer to hold jsi::Value - std::optional> jsInvocationStack = std::nullopt; - if (traceTurboModulePromiseRejections()) { - auto stack = createJSRuntimeError(runtime, "") - .asObject(runtime) - .getProperty(runtime, "stack"); - jsInvocationStack = std::make_shared(std::move(stack)); - } - jsi::Function resolveJSIFn = promiseConstructorArgs[0].getObject(runtime).getFunction( runtime); jsi::Function rejectJSIFn = promiseConstructorArgs[1].getObject(runtime).getFunction( runtime); - auto rejectJSIFunctionWeakWrapper = - CallbackWrapper::createWeak(std::move(rejectJSIFn), runtime, jsInvoker_); auto resolve = createJavaCallbackFromJSIFunction( - std::move(resolveJSIFn), runtime, jsInvoker_) - .release(); - auto reject = createPromiseRejectJavaCallbackFromJSIFunction( - rejectJSIFunctionWeakWrapper, jsInvocationStack) + std::move(resolveJSIFn), runtime, jsInvoker_) + .release(); + auto reject = createJavaCallbackFromJSIFunction( + std::move(rejectJSIFn), runtime, jsInvoker_) .release(); jclass jPromiseImpl = @@ -973,8 +829,8 @@ jsi::Value JavaTurboModule::invokeJavaMethod( jobject promise = env->NewObject( jPromiseImpl, jPromiseImplConstructor, resolve, reject); - const char* moduleName = moduleNameStr.c_str(); - const char* methodName = methodNameStr.c_str(); + const char *moduleName = moduleNameStr.c_str(); + const char *methodName = methodNameStr.c_str(); jobject globalPromise = env->NewGlobalRef(promise); @@ -985,11 +841,8 @@ jsi::Value JavaTurboModule::invokeJavaMethod( TMPL::asyncMethodCallArgConversionEnd(moduleName, methodName); TMPL::asyncMethodCallDispatch(moduleName, methodName); - nativeMethodCallInvoker_->invokeAsync( - methodName, + nativeInvoker_->invokeAsync( [jargs, - rejectJSIFunctionWeakWrapper, - jsInvocationStack = std::move(jsInvocationStack), globalRefs, methodID, instance_ = jni::make_weak(instance_), @@ -1006,9 +859,9 @@ jsi::Value JavaTurboModule::invokeJavaMethod( * environment again? Why does JNI crash when we use the env * from the upper scope? */ - JNIEnv* env = jni::Environment::current(); - const char* moduleName = moduleNameStr.c_str(); - const char* methodName = methodNameStr.c_str(); + JNIEnv *env = jni::Environment::current(); + const char *moduleName = moduleNameStr.c_str(); + const char *methodName = methodNameStr.c_str(); TMPL::asyncMethodCallExecutionStart( moduleName, methodName, id); @@ -1018,12 +871,7 @@ jsi::Value JavaTurboModule::invokeJavaMethod( } catch (...) { TMPL::asyncMethodCallExecutionFail( moduleName, methodName, id); - if (rejectTurboModulePromiseOnNativeError()) { - auto exception = std::current_exception(); - rejectWithException(rejectJSIFunctionWeakWrapper, exception, jsInvocationStack); - } else { - throw; - } + throw; } for (auto globalRef : globalRefs) { From b5471d2da33f0e4cb7ee6d64d60e363c65632ac9 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Tue, 10 Oct 2023 18:14:47 +0200 Subject: [PATCH 37/55] fix perform invocation --- .../core/platform/ios/ReactCommon/RCTTurboModule.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm index a1b57e1631f4..40482878a063 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm @@ -920,7 +920,7 @@ static void handleCause( case ObjectKind: case ArrayKind: case FunctionKind: { - id result = performMethodInvocation(runtime, true, methodName, inv, retainedObjectsForInvocation); + id result = performMethodInvocation(runtime, true, methodName, inv, retainedObjectsForInvocation, nil); TurboModulePerfLogger::syncMethodCallReturnConversionStart(moduleName, methodName); returnValue = convertReturnIdToJSIValue(runtime, methodName, returnType, result); TurboModulePerfLogger::syncMethodCallReturnConversionEnd(moduleName, methodName); From b3cfec3f442c543d1a024639a38e8ad4b040229d Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Tue, 10 Oct 2023 18:14:56 +0200 Subject: [PATCH 38/55] use double try to catch errors --- .../ios/ReactCommon/RCTTurboModule.mm | 112 +++++++++++------- 1 file changed, 71 insertions(+), 41 deletions(-) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm index 40482878a063..714344b50383 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm @@ -416,20 +416,19 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s return Promise.callAsConstructor(runtime, fn); } -static void handleCause( +static std::optional handleCause( jsi::Runtime &runtime, bool isSync, NSDictionary *cause, NSMutableArray *retainedObjectsForInvocation, RCTNSDictionaryPromiseRejectBlock optionalInternalRejectBlock) { - [retainedObjectsForInvocation removeAllObjects]; if (isSync) { NSString *message = cause[@"message"]; if (message) { - throw convertNSDictionaryToJSError(runtime, std::string([message UTF8String]), cause); + return convertNSDictionaryToJSError(runtime, std::string([message UTF8String]), cause); } else { - throw convertNSDictionaryToJSError(runtime, "", cause); + return convertNSDictionaryToJSError(runtime, "", cause); } } else { if (optionalInternalRejectBlock != nil) { @@ -440,6 +439,7 @@ static void handleCause( throw; } } + return std::nullopt; } /** @@ -477,47 +477,77 @@ static void handleCause( TurboModulePerfLogger::asyncMethodCallExecutionStart(moduleName, methodNameStr.c_str(), asyncCallCounter); } + std::optional handledCatch = std::nullopt; try { - [inv invokeWithTarget:strongModule]; - } catch (NSException *exception) { - NSDictionary *cause = @{ - @"name": exception.name, - @"message": exception.reason, - @"stackSymbols": exception.callStackSymbols, - @"stackReturnAddresses": exception.callStackReturnAddresses, - }; - handleCause(runtime, isSync, cause, retainedObjectsForInvocation, optionalInternalRejectBlock); - } catch (NSError *error) { - NSDictionary *cause = @{ - @"userInfo": error.userInfo, - @"domain": error.domain, - }; - handleCause(runtime, isSync, cause, retainedObjectsForInvocation, optionalInternalRejectBlock); - } catch (NSString *errorMessage) { - NSDictionary *cause = @{ - @"message": errorMessage, - }; - handleCause(runtime, isSync, cause, retainedObjectsForInvocation, optionalInternalRejectBlock); - } catch (id e) { - NSDictionary *cause = @{ - @"message": @"Unknown Objective-C Object thrown.", - }; - handleCause(runtime, isSync, cause, retainedObjectsForInvocation, optionalInternalRejectBlock); + @try { + [inv invokeWithTarget:strongModule]; + } @catch (NSException *exception) { + handledCatch = handleCause(runtime, + isSync, + @{ + @"name": exception.name, + @"message": exception.reason, + @"stackSymbols": exception.callStackSymbols, + @"stackReturnAddresses": exception.callStackReturnAddresses, + }, + retainedObjectsForInvocation, + optionalInternalRejectBlock); + } @catch (NSError *error) { + handledCatch = handleCause(runtime, + isSync, + @{ + @"userInfo": error.userInfo, + @"domain": error.domain, + }, + retainedObjectsForInvocation, + optionalInternalRejectBlock); + } @catch (NSString *errorMessage) { + handledCatch = handleCause(runtime, + isSync, + @{ + @"message": errorMessage, + }, + retainedObjectsForInvocation, + optionalInternalRejectBlock); + } @catch (id e) { + handledCatch = handleCause(runtime, + isSync, + @{ + @"message": @"Unknown Objective-C Object thrown.", + }, + retainedObjectsForInvocation, + optionalInternalRejectBlock); + } @finally { + [retainedObjectsForInvocation removeAllObjects]; + } } catch (const std::exception &exception) { - NSDictionary *cause = @{ - @"message": [NSString stringWithUTF8String:exception.what()], - }; - handleCause(runtime, isSync, cause, retainedObjectsForInvocation, optionalInternalRejectBlock); + handledCatch = handleCause(runtime, + isSync, + @{ + @"message": [NSString stringWithUTF8String:exception.what()], + }, + retainedObjectsForInvocation, + optionalInternalRejectBlock); } catch (const std::string &errorMessage) { - NSDictionary *cause = @{ - @"message": [NSString stringWithUTF8String:errorMessage.c_str()], - }; - handleCause(runtime, isSync, cause, retainedObjectsForInvocation, optionalInternalRejectBlock); + handledCatch = handleCause(runtime, + isSync, + @{ + @"message": [NSString stringWithUTF8String:errorMessage.c_str()], + }, + retainedObjectsForInvocation, + optionalInternalRejectBlock); } catch (...) { - NSDictionary *cause = @{ - @"message": @"Unknown C++ exception thrown.", - }; - handleCause(runtime, isSync, cause, retainedObjectsForInvocation, optionalInternalRejectBlock); + handledCatch = handleCause(runtime, + isSync, + @{ + @"message": @"Unknown C++ exception thrown.", + }, + retainedObjectsForInvocation, + optionalInternalRejectBlock); + } + + if (handledCatch.has_value()) { + throw handledCatch.value(); } if (!isSync) { From f19abd3bbcc5242c05a20bff4619838ad2f2967b Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Tue, 10 Oct 2023 18:15:43 +0200 Subject: [PATCH 39/55] Add example cpp error to the ios turbo modules --- .../samples/NativeSampleTurboModule.js | 1 + .../ReactCommon/SampleTurboModuleSpec.cpp | 3 +++ .../RCTNativeSampleTurboModuleSpec.h | 1 + .../RCTNativeSampleTurboModuleSpec.mm | 11 ++++++++ .../ios/ReactCommon/RCTSampleLegacyModule.mm | 5 ++++ .../ios/ReactCommon/RCTSampleTurboModule.mm | 5 ++++ .../TurboModule/SampleTurboModuleExample.js | 25 +++++++++++++------ 7 files changed, 43 insertions(+), 8 deletions(-) diff --git a/packages/react-native/Libraries/TurboModule/samples/NativeSampleTurboModule.js b/packages/react-native/Libraries/TurboModule/samples/NativeSampleTurboModule.js index a46dd55ed9d7..68acb7763388 100644 --- a/packages/react-native/Libraries/TurboModule/samples/NativeSampleTurboModule.js +++ b/packages/react-native/Libraries/TurboModule/samples/NativeSampleTurboModule.js @@ -43,6 +43,7 @@ export interface Spec extends TurboModule { +voidFuncAssert?: () => void; +getObjectAssert?: (arg: Object) => Object; +promiseAssert?: () => Promise; + +getObjectCppThrows?: (arg: Object) => Object; } export default (TurboModuleRegistry.getEnforcing( diff --git a/packages/react-native/ReactCommon/react/nativemodule/samples/platform/android/ReactCommon/SampleTurboModuleSpec.cpp b/packages/react-native/ReactCommon/react/nativemodule/samples/platform/android/ReactCommon/SampleTurboModuleSpec.cpp index e0212c1a21bd..568a858c39fa 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/samples/platform/android/ReactCommon/SampleTurboModuleSpec.cpp +++ b/packages/react-native/ReactCommon/react/nativemodule/samples/platform/android/ReactCommon/SampleTurboModuleSpec.cpp @@ -347,6 +347,9 @@ NativeSampleTurboModuleSpecJSI::NativeSampleTurboModuleSpecJSI( methodMap_["promiseAssert"] = MethodMetadata{ 1, __hostFunction_NativeSampleTurboModuleSpecJSI_promiseAssert}; + methodMap_["getObjectCppThrows"] = MethodMetadata{ + 1, __hostFunction_NativeSampleTurboModuleSpecJSI_getObjectCppThrows}; + methodMap_["getConstants"] = MethodMetadata{ 0, __hostFunction_NativeSampleTurboModuleSpecJSI_getConstants}; } diff --git a/packages/react-native/ReactCommon/react/nativemodule/samples/platform/ios/ReactCommon/RCTNativeSampleTurboModuleSpec.h b/packages/react-native/ReactCommon/react/nativemodule/samples/platform/ios/ReactCommon/RCTNativeSampleTurboModuleSpec.h index be0835f7d42c..32c53c793329 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/samples/platform/ios/ReactCommon/RCTNativeSampleTurboModuleSpec.h +++ b/packages/react-native/ReactCommon/react/nativemodule/samples/platform/ios/ReactCommon/RCTNativeSampleTurboModuleSpec.h @@ -38,6 +38,7 @@ - (void)voidFuncAssert; - (NSDictionary *)getObjectAssert:(NSDictionary *)arg; - (void)promiseAssert:(BOOL)error resolve:(RCTPromiseResolveBlock)resolve reject:(RCTPromiseRejectBlock)reject; +- (NSDictionary *)getObjectCppThrows:(NSDictionary *)arg; - (NSDictionary *)constantsToExport; - (NSDictionary *)getConstants; diff --git a/packages/react-native/ReactCommon/react/nativemodule/samples/platform/ios/ReactCommon/RCTNativeSampleTurboModuleSpec.mm b/packages/react-native/ReactCommon/react/nativemodule/samples/platform/ios/ReactCommon/RCTNativeSampleTurboModuleSpec.mm index de9e39504c34..7fcc27ad9cc1 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/samples/platform/ios/ReactCommon/RCTNativeSampleTurboModuleSpec.mm +++ b/packages/react-native/ReactCommon/react/nativemodule/samples/platform/ios/ReactCommon/RCTNativeSampleTurboModuleSpec.mm @@ -200,6 +200,16 @@ .invokeObjCMethod(rt, PromiseKind, "promiseAssert", @selector(promiseAssert:resolve:reject:), args, count); } +static facebook::jsi::Value __hostFunction_NativeSampleTurboModuleSpecJSI_getObjectCppThrows( + facebook::jsi::Runtime &rt, + TurboModule &turboModule, + const facebook::jsi::Value *args, + size_t count) +{ + return static_cast(turboModule) + .invokeObjCMethod(rt, ObjectKind, "getObjectCppThrows", @selector(getObjectCppThrows:), args, count); +} + NativeSampleTurboModuleSpecJSI::NativeSampleTurboModuleSpecJSI(const ObjCTurboModule::InitParams ¶ms) : ObjCTurboModule(params) { @@ -223,6 +233,7 @@ methodMap_["voidFuncAssert"] = MethodMetadata{0, __hostFunction_NativeSampleTurboModuleSpecJSI_voidFuncAssert}; methodMap_["getObjectAssert"] = MethodMetadata{1, __hostFunction_NativeSampleTurboModuleSpecJSI_getObjectAssert}; methodMap_["promiseAssert"] = MethodMetadata{1, __hostFunction_NativeSampleTurboModuleSpecJSI_promiseAssert}; + methodMap_["getObjectCppThrows"] = MethodMetadata{1, __hostFunction_NativeSampleTurboModuleSpecJSI_getObjectCppThrows}; methodMap_["getConstants"] = MethodMetadata{0, __hostFunction_NativeSampleTurboModuleSpecJSI_getConstants}; } diff --git a/packages/react-native/ReactCommon/react/nativemodule/samples/platform/ios/ReactCommon/RCTSampleLegacyModule.mm b/packages/react-native/ReactCommon/react/nativemodule/samples/platform/ios/ReactCommon/RCTSampleLegacyModule.mm index 5613f858cf4d..8549faa5a8f8 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/samples/platform/ios/ReactCommon/RCTSampleLegacyModule.mm +++ b/packages/react-native/ReactCommon/react/nativemodule/samples/platform/ios/ReactCommon/RCTSampleLegacyModule.mm @@ -200,6 +200,11 @@ - (NSDictionary *)constantsToExport @throw myException; } +RCT_EXPORT_SYNCHRONOUS_TYPED_METHOD(NSDictionary *, getObjectCppThrows : (NSDictionary *)arg) +{ + throw std::runtime_error("Intentional error from Cpp getObjectCppThrows"); +} + RCT_EXPORT_METHOD(voidFuncAssert) { RCTAssert(false, @"Intentional assert from ObjC voidFuncAssert"); diff --git a/packages/react-native/ReactCommon/react/nativemodule/samples/platform/ios/ReactCommon/RCTSampleTurboModule.mm b/packages/react-native/ReactCommon/react/nativemodule/samples/platform/ios/ReactCommon/RCTSampleTurboModule.mm index a5f92e0ee4a6..5fff7c8fc607 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/samples/platform/ios/ReactCommon/RCTSampleTurboModule.mm +++ b/packages/react-native/ReactCommon/react/nativemodule/samples/platform/ios/ReactCommon/RCTSampleTurboModule.mm @@ -173,6 +173,11 @@ - (NSDictionary *)constantsToExport @throw myException; } +RCT_EXPORT_SYNCHRONOUS_TYPED_METHOD(NSDictionary *, getObjectCppThrows : (NSDictionary *)arg) +{ + throw std::runtime_error("Intentional error from Cpp getObjectCppThrows"); +} + RCT_EXPORT_METHOD(voidFuncAssert) { RCTAssert(false, @"Intentional assert from ObjC voidFuncAssert"); diff --git a/packages/rn-tester/js/examples/TurboModule/SampleTurboModuleExample.js b/packages/rn-tester/js/examples/TurboModule/SampleTurboModuleExample.js index 5331195cf0a1..fe1c08e9a08c 100644 --- a/packages/rn-tester/js/examples/TurboModule/SampleTurboModuleExample.js +++ b/packages/rn-tester/js/examples/TurboModule/SampleTurboModuleExample.js @@ -56,7 +56,7 @@ class SampleTurboModuleExample extends React.Component<{||}, State> { .then(() => {}) .catch(e => { this._setResult('rejectPromise', e.message); - console.error(e, e.stack, e.cause); + console.error('Error:', e, 'Stack: ', e.stack, 'Cause: ', e.cause); }), getConstants: () => NativeSampleTurboModule.getConstants(), voidFunc: () => NativeSampleTurboModule.voidFunc(), @@ -84,7 +84,7 @@ class SampleTurboModuleExample extends React.Component<{||}, State> { try { NativeSampleTurboModule.voidFuncThrows?.(); } catch (e) { - console.error(e, e.stack, e.cause); + console.error('Error:', e, 'Stack: ', e.stack, 'Cause: ', e.cause); return e.message; } }, @@ -92,7 +92,7 @@ class SampleTurboModuleExample extends React.Component<{||}, State> { try { NativeSampleTurboModule.getObjectThrows?.({a: 1, b: 'foo', c: null}); } catch (e) { - console.error(e, e.stack, e.cause); + console.error('Error:', e, 'Stack: ', e.stack, 'Cause: ', e.cause); return e.message; } }, @@ -101,14 +101,14 @@ class SampleTurboModuleExample extends React.Component<{||}, State> { .then(() => {}) .catch(e => { this._setResult('promiseThrows', e.message); - console.error(e, e.stack, e.cause); + console.error('Error:', e, 'Stack: ', e.stack, 'Cause: ', e.cause); }); }, voidFuncAssert: () => { try { NativeSampleTurboModule.voidFuncAssert?.(); } catch (e) { - console.error(e, e.stack, e.cause); + console.error('Error:', e, 'Stack: ', e.stack, 'Cause: ', e.cause); return e.message; } }, @@ -116,7 +116,7 @@ class SampleTurboModuleExample extends React.Component<{||}, State> { try { NativeSampleTurboModule.getObjectAssert?.({a: 1, b: 'foo', c: null}); } catch (e) { - console.error(e, e.stack, e.cause); + console.error('Error:', e, 'Stack: ', e.stack, 'Cause: ', e.cause); return e.message; } }, @@ -125,9 +125,17 @@ class SampleTurboModuleExample extends React.Component<{||}, State> { .then(() => {}) .catch(e => { this._setResult('promiseAssert', e.message); - console.error(e, e.stack, e.cause); + console.error('Error:', e, 'Stack: ', e.stack, 'Cause: ', e.cause); }); }, + getObjectCppThrows: () => { + try { + NativeSampleTurboModule.getObjectCppThrows?.({a: 1, b: 'foo', c: null}); + } catch (e) { + console.error('Error:', e, 'Stack: ', e.stack, 'Cause: ', e.cause); + return e.message; + } + }, }; _setResult( @@ -152,7 +160,8 @@ class SampleTurboModuleExample extends React.Component<{||}, State> { | 'promiseThrows' | 'voidFuncAssert' | 'getObjectAssert' - | 'promiseAssert', + | 'promiseAssert' + | 'getObjectCppThrows', result: | $FlowFixMe | void From 95480e1051788a0be01bc58986c0514b3258ffb6 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Wed, 11 Oct 2023 11:29:42 +0200 Subject: [PATCH 40/55] Remove unwanted Android changes --- .../android/ReactCommon/JavaTurboModule.cpp | 165 ++++++++++-------- .../ReactCommon/SampleTurboModuleSpec.cpp | 3 - 2 files changed, 88 insertions(+), 80 deletions(-) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp index 477d144773b8..521dcbaed5d9 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp @@ -10,12 +10,12 @@ #include #include +#include #include #include #include #include -#include #include #include #include @@ -28,10 +28,11 @@ namespace facebook::react { namespace TMPL = TurboModulePerfLogger; -JavaTurboModule::JavaTurboModule(const InitParams ¶ms) +JavaTurboModule::JavaTurboModule(const InitParams& params) : TurboModule(params.moduleName, params.jsInvoker), instance_(jni::make_global(params.instance)), - nativeInvoker_(params.nativeInvoker) {} + nativeMethodCallInvoker_(params.nativeMethodCallInvoker), + shouldVoidMethodsExecuteSync_(params.shouldVoidMethodsExecuteSync) {} JavaTurboModule::~JavaTurboModule() { /** @@ -42,15 +43,16 @@ JavaTurboModule::~JavaTurboModule() { return; } - nativeInvoker_->invokeAsync([instance = std::move(instance_)]() mutable { - /** - * Reset the global NativeModule ref on the NativeModules thread. Why: - * - ~JavaTurboModule() can be called on a non-JVM thread. If we reset the - * global ref in ~JavaTurboModule(), we might access the JVM from a - * non-JVM thread, which will crash the app. - */ - instance.reset(); - }); + nativeMethodCallInvoker_->invokeAsync( + "~" + name_, [instance = std::move(instance_)]() mutable { + /** + * Reset the global NativeModule ref on the NativeModules thread. Why: + * - ~JavaTurboModule() can be called on a non-JVM thread. If we reset + * the global ref in ~JavaTurboModule(), we might access the JVM from a + * non-JVM thread, which will crash the app. + */ + instance.reset(); + }); } namespace { @@ -62,9 +64,9 @@ struct JNIArgs { }; jni::local_ref createJavaCallbackFromJSIFunction( - jsi::Function &&function, - jsi::Runtime &rt, - const std::shared_ptr &jsInvoker) { + jsi::Function&& function, + jsi::Runtime& rt, + const std::shared_ptr& jsInvoker) { auto weakWrapper = CallbackWrapper::createWeak(std::move(function), rt, jsInvoker); @@ -82,8 +84,7 @@ jni::local_ref createJavaCallbackFromJSIFunction( callbackWrapperOwner = std::move(callbackWrapperOwner), wrapperWasCalled = false](folly::dynamic responses) mutable { if (wrapperWasCalled) { - throw std::runtime_error( - "Callback arg cannot be called more than once"); + LOG(FATAL) << "callback arg cannot be called more than once"; } auto strongWrapper = weakWrapper.lock(); @@ -102,14 +103,14 @@ jni::local_ref createJavaCallbackFromJSIFunction( std::vector args; args.reserve(responses.size()); - for (const auto &val : responses) { + for (const auto& val : responses) { args.emplace_back( jsi::valueFromDynamic(strongWrapper2->runtime(), val)); } strongWrapper2->callback().call( strongWrapper2->runtime(), - (const jsi::Value *)args.data(), + (const jsi::Value*)args.data(), args.size()); }); @@ -118,7 +119,7 @@ jni::local_ref createJavaCallbackFromJSIFunction( } // This is used for generating short exception strings. -std::string stringifyJSIValue(const jsi::Value &v, jsi::Runtime *rt = nullptr) { +std::string stringifyJSIValue(const jsi::Value& v, jsi::Runtime* rt = nullptr) { if (v.isUndefined()) { return "undefined"; } @@ -147,11 +148,11 @@ std::string stringifyJSIValue(const jsi::Value &v, jsi::Runtime *rt = nullptr) { class JavaTurboModuleArgumentConversionException : public std::runtime_error { public: JavaTurboModuleArgumentConversionException( - const std::string &expectedType, + const std::string& expectedType, int index, - const std::string &methodName, - const jsi::Value *arg, - jsi::Runtime *rt) + const std::string& methodName, + const jsi::Value* arg, + jsi::Runtime* rt) : std::runtime_error( "Expected argument " + std::to_string(index) + " of method \"" + methodName + "\" to be a " + expectedType + ", but got " + @@ -161,9 +162,9 @@ class JavaTurboModuleArgumentConversionException : public std::runtime_error { class JavaTurboModuleInvalidArgumentTypeException : public std::runtime_error { public: JavaTurboModuleInvalidArgumentTypeException( - const std::string &actualType, + const std::string& actualType, int argIndex, - const std::string &methodName) + const std::string& methodName) : std::runtime_error( "Called method \"" + methodName + "\" with unsupported type " + actualType + " at argument " + std::to_string(argIndex)) {} @@ -172,7 +173,7 @@ class JavaTurboModuleInvalidArgumentTypeException : public std::runtime_error { class JavaTurboModuleInvalidArgumentCountException : public std::runtime_error { public: JavaTurboModuleInvalidArgumentCountException( - const std::string &methodName, + const std::string& methodName, int actualArgCount, int expectedArgCount) : std::runtime_error( @@ -188,7 +189,7 @@ class JavaTurboModuleInvalidArgumentCountException : public std::runtime_error { * for a description of Java method signature structure. */ std::vector getMethodArgTypesFromSignature( - const std::string &methodSignature) { + const std::string& methodSignature) { std::vector methodArgs; for (auto it = methodSignature.begin(); it != methodSignature.end(); @@ -236,13 +237,13 @@ int32_t getUniqueId() { // TODO (axe) Reuse existing implementation as needed - the exist in // MethodInvoker.cpp JNIArgs convertJSIArgsToJNIArgs( - JNIEnv *env, - jsi::Runtime &rt, - const std::string &methodName, - const std::vector &methodArgTypes, - const jsi::Value *args, + JNIEnv* env, + jsi::Runtime& rt, + const std::string& methodName, + const std::vector& methodArgTypes, + const jsi::Value* args, size_t count, - const std::shared_ptr &jsInvoker, + const std::shared_ptr& jsInvoker, TurboModuleMethodValueKind valueKind) { unsigned int expectedArgumentCount = valueKind == PromiseKind ? methodArgTypes.size() - 1 @@ -254,8 +255,8 @@ JNIArgs convertJSIArgsToJNIArgs( } JNIArgs jniArgs(valueKind == PromiseKind ? count + 1 : count); - auto &jargs = jniArgs.args_; - auto &globalRefs = jniArgs.globalRefs_; + auto& jargs = jniArgs.args_; + auto& globalRefs = jniArgs.globalRefs_; auto makeGlobalIfNecessary = [&globalRefs, env, valueKind](jobject obj) -> jobject { @@ -270,10 +271,10 @@ JNIArgs convertJSIArgsToJNIArgs( }; for (unsigned int argIndex = 0; argIndex < count; argIndex += 1) { - const std::string &type = methodArgTypes.at(argIndex); + const std::string& type = methodArgTypes.at(argIndex); - const jsi::Value *arg = &args[argIndex]; - jvalue *jarg = &jargs[argIndex]; + const jsi::Value* arg = &args[argIndex]; + jvalue* jarg = &jargs[argIndex]; if (type == "D") { if (!arg->isNumber()) { @@ -384,7 +385,7 @@ JNIArgs convertJSIArgsToJNIArgs( return jniArgs; } -jsi::Value convertFromJMapToValue(JNIEnv *env, jsi::Runtime &rt, jobject arg) { +jsi::Value convertFromJMapToValue(JNIEnv* env, jsi::Runtime& rt, jobject arg) { // We currently use Java Argument.makeNativeMap() method to do this conversion // This could also be done purely in C++, but iterative over map methods // but those may end up calling reflection methods anyway @@ -401,8 +402,8 @@ jsi::Value convertFromJMapToValue(JNIEnv *env, jsi::Runtime &rt, jobject arg) { } jsi::Value createJSRuntimeError( - jsi::Runtime &runtime, - const std::string &message) { + jsi::Runtime& runtime, + const std::string& message) { return runtime.global() .getPropertyAsFunction(runtime, "Error") .call(runtime, message); @@ -412,7 +413,7 @@ jsi::Value createJSRuntimeError( * Creates JSError with current JS runtime stack and Throwable stack trace. */ jsi::JSError convertThrowableToJSError( - jsi::Runtime &runtime, + jsi::Runtime& runtime, jni::local_ref throwable) { auto stackTrace = throwable->getStackTrace(); @@ -429,15 +430,9 @@ jsi::JSError convertThrowableToJSError( } jsi::Object cause(runtime); - auto getName = throwable->getClass() - ->getClass() - ->getMethod()>("getName"); - auto getMessage = - throwable->getClass()->getMethod()>( - "getMessage"); - auto message = getMessage(throwable)->toStdString(); - cause.setProperty( - runtime, "name", getName(throwable->getClass())->toStdString()); + auto name = throwable->getClass()->getCanonicalName()->toStdString(); + auto message = throwable->getMessage()->toStdString(); + cause.setProperty(runtime, "name", name); cause.setProperty(runtime, "message", message); cause.setProperty(runtime, "stackElements", std::move(stackElements)); @@ -450,17 +445,19 @@ jsi::JSError convertThrowableToJSError( } // namespace jsi::Value JavaTurboModule::invokeJavaMethod( - jsi::Runtime &runtime, + jsi::Runtime& runtime, TurboModuleMethodValueKind valueKind, - const std::string &methodNameStr, - const std::string &methodSignature, - const jsi::Value *args, + const std::string& methodNameStr, + const std::string& methodSignature, + const jsi::Value* args, size_t argCount, - jmethodID &methodID) { - const char *methodName = methodNameStr.c_str(); - const char *moduleName = name_.c_str(); + jmethodID& methodID) { + const char* methodName = methodNameStr.c_str(); + const char* moduleName = name_.c_str(); - bool isMethodSync = !(valueKind == VoidKind || valueKind == PromiseKind); + bool isMethodSync = + (valueKind == VoidKind && shouldVoidMethodsExecuteSync_) || + !(valueKind == VoidKind || valueKind == PromiseKind); if (isMethodSync) { TMPL::syncMethodCallStart(moduleName, methodName); @@ -470,7 +467,7 @@ jsi::Value JavaTurboModule::invokeJavaMethod( TMPL::asyncMethodCallArgConversionStart(moduleName, methodName); } - JNIEnv *env = jni::Environment::current(); + JNIEnv* env = jni::Environment::current(); auto instance = instance_.get(); /** @@ -567,8 +564,8 @@ jsi::Value JavaTurboModule::invokeJavaMethod( TMPL::syncMethodCallExecutionStart(moduleName, methodName); } - auto &jargs = jniArgs.args_; - auto &globalRefs = jniArgs.globalRefs_; + auto& jargs = jniArgs.args_; + auto& globalRefs = jniArgs.globalRefs_; switch (valueKind) { case BooleanKind: { @@ -687,7 +684,7 @@ jsi::Value JavaTurboModule::invokeJavaMethod( jsi::Value returnValue = jsi::Value::null(); if (returnString != nullptr) { - const char *js = env->GetStringUTFChars(returnString, nullptr); + const char* js = env->GetStringUTFChars(returnString, nullptr); std::string result = js; env->ReleaseStringUTFChars(returnString, js); returnValue = @@ -739,10 +736,23 @@ jsi::Value JavaTurboModule::invokeJavaMethod( return returnValue; } case VoidKind: { + if (shouldVoidMethodsExecuteSync_) { + env->CallVoidMethodA(instance, methodID, jargs.data()); + TMPL::syncMethodCallExecutionEnd(moduleName, methodName); + TMPL::syncMethodCallEnd(moduleName, methodName); + try { + FACEBOOK_JNI_THROW_PENDING_EXCEPTION(); + } catch (...) { + throw; + } + return jsi::Value::undefined(); + } + TMPL::asyncMethodCallArgConversionEnd(moduleName, methodName); TMPL::asyncMethodCallDispatch(moduleName, methodName); - nativeInvoker_->invokeAsync( + nativeMethodCallInvoker_->invokeAsync( + methodName, [jargs, globalRefs, methodID, @@ -759,9 +769,9 @@ jsi::Value JavaTurboModule::invokeJavaMethod( * again? Why does JNI crash when we use the env from the upper * scope? */ - JNIEnv *env = jni::Environment::current(); - const char *moduleName = moduleNameStr.c_str(); - const char *methodName = methodNameStr.c_str(); + JNIEnv* env = jni::Environment::current(); + const char* moduleName = moduleNameStr.c_str(); + const char* methodName = methodNameStr.c_str(); TMPL::asyncMethodCallExecutionStart(moduleName, methodName, id); env->CallVoidMethodA(instance.get(), methodID, jargs.data()); @@ -797,9 +807,9 @@ jsi::Value JavaTurboModule::invokeJavaMethod( moduleNameStr = name_, methodNameStr, env]( - jsi::Runtime &runtime, - const jsi::Value &thisVal, - const jsi::Value *promiseConstructorArgs, + jsi::Runtime& runtime, + const jsi::Value& thisVal, + const jsi::Value* promiseConstructorArgs, size_t promiseConstructorArgCount) { if (promiseConstructorArgCount != 2) { throw std::invalid_argument("Promise fn arg count must be 2"); @@ -829,8 +839,8 @@ jsi::Value JavaTurboModule::invokeJavaMethod( jobject promise = env->NewObject( jPromiseImpl, jPromiseImplConstructor, resolve, reject); - const char *moduleName = moduleNameStr.c_str(); - const char *methodName = methodNameStr.c_str(); + const char* moduleName = moduleNameStr.c_str(); + const char* methodName = methodNameStr.c_str(); jobject globalPromise = env->NewGlobalRef(promise); @@ -841,7 +851,8 @@ jsi::Value JavaTurboModule::invokeJavaMethod( TMPL::asyncMethodCallArgConversionEnd(moduleName, methodName); TMPL::asyncMethodCallDispatch(moduleName, methodName); - nativeInvoker_->invokeAsync( + nativeMethodCallInvoker_->invokeAsync( + methodName, [jargs, globalRefs, methodID, @@ -859,9 +870,9 @@ jsi::Value JavaTurboModule::invokeJavaMethod( * environment again? Why does JNI crash when we use the env * from the upper scope? */ - JNIEnv *env = jni::Environment::current(); - const char *moduleName = moduleNameStr.c_str(); - const char *methodName = methodNameStr.c_str(); + JNIEnv* env = jni::Environment::current(); + const char* moduleName = moduleNameStr.c_str(); + const char* methodName = methodNameStr.c_str(); TMPL::asyncMethodCallExecutionStart( moduleName, methodName, id); diff --git a/packages/react-native/ReactCommon/react/nativemodule/samples/platform/android/ReactCommon/SampleTurboModuleSpec.cpp b/packages/react-native/ReactCommon/react/nativemodule/samples/platform/android/ReactCommon/SampleTurboModuleSpec.cpp index 568a858c39fa..e0212c1a21bd 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/samples/platform/android/ReactCommon/SampleTurboModuleSpec.cpp +++ b/packages/react-native/ReactCommon/react/nativemodule/samples/platform/android/ReactCommon/SampleTurboModuleSpec.cpp @@ -347,9 +347,6 @@ NativeSampleTurboModuleSpecJSI::NativeSampleTurboModuleSpecJSI( methodMap_["promiseAssert"] = MethodMetadata{ 1, __hostFunction_NativeSampleTurboModuleSpecJSI_promiseAssert}; - methodMap_["getObjectCppThrows"] = MethodMetadata{ - 1, __hostFunction_NativeSampleTurboModuleSpecJSI_getObjectCppThrows}; - methodMap_["getConstants"] = MethodMetadata{ 0, __hostFunction_NativeSampleTurboModuleSpecJSI_getConstants}; } From 707f5aa25ba3b81788e1f4ec54c51f2623b28bdb Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Wed, 11 Oct 2023 11:33:16 +0200 Subject: [PATCH 41/55] Once more remove Android changes --- .../android/ReactCommon/JavaTurboModule.cpp | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp index 521dcbaed5d9..8d5a6f94d51a 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp @@ -31,8 +31,7 @@ namespace TMPL = TurboModulePerfLogger; JavaTurboModule::JavaTurboModule(const InitParams& params) : TurboModule(params.moduleName, params.jsInvoker), instance_(jni::make_global(params.instance)), - nativeMethodCallInvoker_(params.nativeMethodCallInvoker), - shouldVoidMethodsExecuteSync_(params.shouldVoidMethodsExecuteSync) {} + nativeMethodCallInvoker_(params.nativeMethodCallInvoker) {} JavaTurboModule::~JavaTurboModule() { /** @@ -455,9 +454,7 @@ jsi::Value JavaTurboModule::invokeJavaMethod( const char* methodName = methodNameStr.c_str(); const char* moduleName = name_.c_str(); - bool isMethodSync = - (valueKind == VoidKind && shouldVoidMethodsExecuteSync_) || - !(valueKind == VoidKind || valueKind == PromiseKind); + bool isMethodSync = !(valueKind == VoidKind || valueKind == PromiseKind); if (isMethodSync) { TMPL::syncMethodCallStart(moduleName, methodName); @@ -736,18 +733,6 @@ jsi::Value JavaTurboModule::invokeJavaMethod( return returnValue; } case VoidKind: { - if (shouldVoidMethodsExecuteSync_) { - env->CallVoidMethodA(instance, methodID, jargs.data()); - TMPL::syncMethodCallExecutionEnd(moduleName, methodName); - TMPL::syncMethodCallEnd(moduleName, methodName); - try { - FACEBOOK_JNI_THROW_PENDING_EXCEPTION(); - } catch (...) { - throw; - } - return jsi::Value::undefined(); - } - TMPL::asyncMethodCallArgConversionEnd(moduleName, methodName); TMPL::asyncMethodCallDispatch(moduleName, methodName); From a48a1f2740399e4bf1dfba83aa213fcd29647bd1 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Wed, 11 Oct 2023 12:42:30 +0200 Subject: [PATCH 42/55] Gate promise rejection on native error --- packages/react-native/React/Base/RCTBridge.h | 4 ++++ packages/react-native/React/Base/RCTBridge.mm | 11 +++++++++++ .../platform/ios/ReactCommon/RCTTurboModule.mm | 15 +++++++++++---- 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/packages/react-native/React/Base/RCTBridge.h b/packages/react-native/React/Base/RCTBridge.h index 2d12895bc5cb..c7b15d018c71 100644 --- a/packages/react-native/React/Base/RCTBridge.h +++ b/packages/react-native/React/Base/RCTBridge.h @@ -74,6 +74,10 @@ void RCTEnableTurboModuleInteropForAllTurboModules(BOOL enabled); RCT_EXTERN BOOL RCTTraceTurboModulePromiseRejections(void); RCT_EXTERN void RCTEnableTraceTurboModulePromiseRejections(BOOL enabled); +// Reject JS Promise on native throw in TM method call +RCT_EXTERN BOOL RCTRejectTurboModulePromiseOnNativeError(void); +RCT_EXTERN void RCTEnableRejectTurboModulePromiseOnNativeError(BOOL enabled); + typedef enum { kRCTGlobalScope, kRCTGlobalScopeUsingRetainJSCallback, diff --git a/packages/react-native/React/Base/RCTBridge.mm b/packages/react-native/React/Base/RCTBridge.mm index 4b38569cc4a5..b79a650c5de8 100644 --- a/packages/react-native/React/Base/RCTBridge.mm +++ b/packages/react-native/React/Base/RCTBridge.mm @@ -108,6 +108,17 @@ void RCTEnableTraceTurboModulePromiseRejections(BOOL enabled) traceTurboModulePromiseRejections = enabled; } +static BOOL rejectTurboModulePromiseOnNativeError = YES; +BOOL RCTRejectTurboModulePromiseOnNativeError(void) +{ + return rejectTurboModulePromiseOnNativeError; +} + +void RCTEnableRejectTurboModulePromiseOnNativeError(BOOL enabled) +{ + rejectTurboModulePromiseOnNativeError = enabled; +} + static BOOL turboModuleInteropEnabled = NO; BOOL RCTTurboModuleInteropEnabled(void) { diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm index 714344b50383..551a4521b924 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm @@ -405,9 +405,13 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s handlePromiseRejection(reason); }; - RCTNSDictionaryPromiseRejectBlock internalRejectBlock = ^(NSDictionary *dict) { - handlePromiseRejection(dict); - }; + RCTNSDictionaryPromiseRejectBlock internalRejectBlock = nil; + + if (RCTRejectTurboModulePromiseOnNativeError()) { + internalRejectBlock = ^(NSDictionary *dict) { + handlePromiseRejection(dict); + }; + } invokeCopy(resolveBlock, rejectBlock, internalRejectBlock); return jsi::Value::undefined(); @@ -922,7 +926,10 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s runtime, methodNameStr, ^(RCTPromiseResolveBlock resolveBlock, RCTPromiseRejectBlock rejectBlock, RCTNSDictionaryPromiseRejectBlock internalRejectBlock) { RCTPromiseResolveBlock resolveCopy = [resolveBlock copy]; RCTPromiseRejectBlock rejectCopy = [rejectBlock copy]; - RCTNSDictionaryPromiseRejectBlock internalRejectCopy = [internalRejectBlock copy]; + RCTNSDictionaryPromiseRejectBlock internalRejectCopy = nil; + if (internalRejectBlock != nil) { + internalRejectCopy = [internalRejectBlock copy]; + } [inv setArgument:(void *)&resolveCopy atIndex:count + 2]; [inv setArgument:(void *)&rejectCopy atIndex:count + 3]; From 2fdd778ff2f30227b6ba41ee03e3d2b11d2b997f Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Mon, 23 Oct 2023 17:07:20 +0200 Subject: [PATCH 43/55] rename promise reject block to RCTInvocationErrorHandler --- .../core/platform/ios/ReactCommon/RCTTurboModule.h | 6 +++--- .../core/platform/ios/ReactCommon/RCTTurboModule.mm | 10 +++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.h b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.h index fa8023059524..3a9b716cd70e 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.h +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.h @@ -27,7 +27,7 @@ * Block used to reject the JS promise waiting for a result * in cases when the native method throws an exception. */ -typedef void (^RCTNSDictionaryPromiseRejectBlock)(NSDictionary *exception); +typedef void (^RCTInvocationErrorHandler)(NSDictionary *exception); namespace facebook::react { @@ -148,14 +148,14 @@ class JSI_EXPORT ObjCTurboModule : public TurboModule { const char *methodName, NSInvocation *inv, NSMutableArray *retainedObjectsForInvocation, - RCTNSDictionaryPromiseRejectBlock optionalInternalRejectBlock); + RCTInvocationErrorHandler optionalInternalRejectBlock); void performVoidMethodInvocation( jsi::Runtime &runtime, const char *methodName, NSInvocation *inv, NSMutableArray *retainedObjectsForInvocation); - using PromiseInvocationBlock = void (^)(RCTPromiseResolveBlock resolveWrapper, RCTPromiseRejectBlock rejectWrapper, RCTNSDictionaryPromiseRejectBlock internalRejectWrapper); + using PromiseInvocationBlock = void (^)(RCTPromiseResolveBlock resolveWrapper, RCTPromiseRejectBlock rejectWrapper, RCTInvocationErrorHandler internalRejectWrapper); jsi::Value createPromise(jsi::Runtime &runtime, std::string methodName, PromiseInvocationBlock invoke); }; diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm index 551a4521b924..70687c5febe5 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm @@ -405,7 +405,7 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s handlePromiseRejection(reason); }; - RCTNSDictionaryPromiseRejectBlock internalRejectBlock = nil; + RCTInvocationErrorHandler internalRejectBlock = nil; if (RCTRejectTurboModulePromiseOnNativeError()) { internalRejectBlock = ^(NSDictionary *dict) { @@ -425,7 +425,7 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s bool isSync, NSDictionary *cause, NSMutableArray *retainedObjectsForInvocation, - RCTNSDictionaryPromiseRejectBlock optionalInternalRejectBlock) + RCTInvocationErrorHandler optionalInternalRejectBlock) { if (isSync) { NSString *message = cause[@"message"]; @@ -461,7 +461,7 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s const char *methodName, NSInvocation *inv, NSMutableArray *retainedObjectsForInvocation, - RCTNSDictionaryPromiseRejectBlock optionalInternalRejectBlock) + RCTInvocationErrorHandler optionalInternalRejectBlock) { __block id result; __weak id weakModule = instance_; @@ -923,10 +923,10 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s switch (returnType) { case PromiseKind: { returnValue = createPromise( - runtime, methodNameStr, ^(RCTPromiseResolveBlock resolveBlock, RCTPromiseRejectBlock rejectBlock, RCTNSDictionaryPromiseRejectBlock internalRejectBlock) { + runtime, methodNameStr, ^(RCTPromiseResolveBlock resolveBlock, RCTPromiseRejectBlock rejectBlock, RCTInvocationErrorHandler internalRejectBlock) { RCTPromiseResolveBlock resolveCopy = [resolveBlock copy]; RCTPromiseRejectBlock rejectCopy = [rejectBlock copy]; - RCTNSDictionaryPromiseRejectBlock internalRejectCopy = nil; + RCTInvocationErrorHandler internalRejectCopy = nil; if (internalRejectBlock != nil) { internalRejectCopy = [internalRejectBlock copy]; } From 11e9b9b9f21ec19c7cfb2948401178fa534c4864 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Mon, 23 Oct 2023 17:16:02 +0200 Subject: [PATCH 44/55] Use explicit JSError constructor --- .../core/platform/ios/ReactCommon/RCTTurboModule.mm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm index 70687c5febe5..77026587cf05 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm @@ -234,7 +234,7 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s jsi::Value error = createJSRuntimeError(runtime, "Exception in HostFunction: " + reason); error.asObject(runtime).setProperty(runtime, "cause", std::move(cause)); - return {runtime, std::move(error)}; + return jsi::JSError(runtime, std::move(error)); } /** @@ -244,7 +244,7 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s { jsi::Value error = createJSRuntimeError(runtime, "Exception in HostFunction: " + message); error.asObject(runtime).setProperty(runtime, "cause", convertNSDictionaryToJSIObject(runtime, cause)); - return {runtime, std::move(error)}; + return jsi::JSError(runtime, std::move(error)); } /** From b1ea76e72f9bef4e4e8324b46d0c2a5784b46a76 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Mon, 23 Oct 2023 17:21:29 +0200 Subject: [PATCH 45/55] clear handleCause return statements --- .../core/platform/ios/ReactCommon/RCTTurboModule.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm index 77026587cf05..78de4fef534f 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm @@ -437,13 +437,13 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s } else { if (optionalInternalRejectBlock != nil) { optionalInternalRejectBlock(cause); + return std::nullopt; } else { // Crash on native layer there is no promise in JS to reject. // We executed JSFunction returning void asynchrounously. throw; } } - return std::nullopt; } /** From af39197917ceefd4c8f5f3a1e90913c2b2acd36c Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Mon, 23 Oct 2023 18:01:57 +0200 Subject: [PATCH 46/55] refactor try catch use maybecatch --- .../ios/ReactCommon/RCTTurboModule.mm | 108 +++++++----------- 1 file changed, 40 insertions(+), 68 deletions(-) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm index 78de4fef534f..ef108079e9b2 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm @@ -240,9 +240,9 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s /** * Creates JSError with current JS runtime and NSDictionary data as cause. */ -static jsi::JSError convertNSDictionaryToJSError(jsi::Runtime &runtime, const std::string &message, NSDictionary *cause) +static jsi::JSError convertNSDictionaryToJSError(jsi::Runtime &runtime, NSDictionary *cause) { - jsi::Value error = createJSRuntimeError(runtime, "Exception in HostFunction: " + message); + jsi::Value error = createJSRuntimeError(runtime, "Exception in HostFunction: " + (cause[@"message"] ? std::string([cause[@"message"] UTF8String]) : "")); error.asObject(runtime).setProperty(runtime, "cause", convertNSDictionaryToJSIObject(runtime, cause)); return jsi::JSError(runtime, std::move(error)); } @@ -253,13 +253,7 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s static jsi::Value createRejectJSErrorValue(jsi::Runtime &runtime, NSDictionary *reason, const std::optional &jsInvocationStack) { jsi::Object cause = convertNSDictionaryToJSIObject(runtime, reason); - jsi::Value error; - NSString *message = reason[@"message"]; - if (message) { - error = createJSRuntimeError(runtime, "Exception in HostFunction: " + std::string([message UTF8String])); - } else { - error = createJSRuntimeError(runtime, "Exception in HostFunction: "); - } + jsi::Value error = createJSRuntimeError(runtime, "Exception in HostFunction: " + (reason[@"message"] ? std::string([reason[@"message"] UTF8String]) : "")); if (jsInvocationStack.has_value()) { error.asObject(runtime).setProperty(runtime, "stack", *jsInvocationStack); @@ -420,29 +414,16 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s return Promise.callAsConstructor(runtime, fn); } -static std::optional handleCause( - jsi::Runtime &runtime, - bool isSync, - NSDictionary *cause, - NSMutableArray *retainedObjectsForInvocation, - RCTInvocationErrorHandler optionalInternalRejectBlock) +static NSDictionary * maybeCatchException( + BOOL shoudCatch, + NSDictionary *cause) { - if (isSync) { - NSString *message = cause[@"message"]; - if (message) { - return convertNSDictionaryToJSError(runtime, std::string([message UTF8String]), cause); - } else { - return convertNSDictionaryToJSError(runtime, "", cause); - } + if (shoudCatch) { + return cause; } else { - if (optionalInternalRejectBlock != nil) { - optionalInternalRejectBlock(cause); - return std::nullopt; - } else { - // Crash on native layer there is no promise in JS to reject. - // We executed JSFunction returning void asynchrounously. - throw; - } + // Crash on native layer there is no promise in JS to reject. + // We executed JSFunction returning void asynchrounously. + throw; } } @@ -481,77 +462,68 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s TurboModulePerfLogger::asyncMethodCallExecutionStart(moduleName, methodNameStr.c_str(), asyncCallCounter); } - std::optional handledCatch = std::nullopt; + NSDictionary *caughtException = nil; + BOOL shouldCatchException = isSync || optionalInternalRejectBlock; try { @try { [inv invokeWithTarget:strongModule]; } @catch (NSException *exception) { - handledCatch = handleCause(runtime, - isSync, + caughtException = maybeCatchException( + shouldCatchException, @{ @"name": exception.name, @"message": exception.reason, @"stackSymbols": exception.callStackSymbols, @"stackReturnAddresses": exception.callStackReturnAddresses, - }, - retainedObjectsForInvocation, - optionalInternalRejectBlock); + }); } @catch (NSError *error) { - handledCatch = handleCause(runtime, - isSync, + caughtException = maybeCatchException( + shouldCatchException, @{ @"userInfo": error.userInfo, @"domain": error.domain, - }, - retainedObjectsForInvocation, - optionalInternalRejectBlock); + }); } @catch (NSString *errorMessage) { - handledCatch = handleCause(runtime, - isSync, + caughtException = maybeCatchException( + shouldCatchException, @{ @"message": errorMessage, - }, - retainedObjectsForInvocation, - optionalInternalRejectBlock); + }); } @catch (id e) { - handledCatch = handleCause(runtime, - isSync, + caughtException = maybeCatchException( + shouldCatchException, @{ @"message": @"Unknown Objective-C Object thrown.", - }, - retainedObjectsForInvocation, - optionalInternalRejectBlock); + }); } @finally { [retainedObjectsForInvocation removeAllObjects]; } } catch (const std::exception &exception) { - handledCatch = handleCause(runtime, - isSync, + caughtException = maybeCatchException( + shouldCatchException, @{ @"message": [NSString stringWithUTF8String:exception.what()], - }, - retainedObjectsForInvocation, - optionalInternalRejectBlock); + }); } catch (const std::string &errorMessage) { - handledCatch = handleCause(runtime, - isSync, + caughtException = maybeCatchException( + shouldCatchException, @{ @"message": [NSString stringWithUTF8String:errorMessage.c_str()], - }, - retainedObjectsForInvocation, - optionalInternalRejectBlock); + }); } catch (...) { - handledCatch = handleCause(runtime, - isSync, + caughtException = maybeCatchException( + shouldCatchException, @{ @"message": @"Unknown C++ exception thrown.", - }, - retainedObjectsForInvocation, - optionalInternalRejectBlock); + }); } - if (handledCatch.has_value()) { - throw handledCatch.value(); + if (caughtException) { + if (isSync) { + throw convertNSDictionaryToJSError(runtime, caughtException); + } else { + optionalInternalRejectBlock(caughtException); + } } if (!isSync) { From 464f6bc24ccf5e5abf73045bbe70be68654dca5d Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Mon, 23 Oct 2023 18:03:13 +0200 Subject: [PATCH 47/55] remove unnecessary copy --- .../core/platform/ios/ReactCommon/RCTTurboModule.mm | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm index ef108079e9b2..ca2bbc394124 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm @@ -898,10 +898,7 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s runtime, methodNameStr, ^(RCTPromiseResolveBlock resolveBlock, RCTPromiseRejectBlock rejectBlock, RCTInvocationErrorHandler internalRejectBlock) { RCTPromiseResolveBlock resolveCopy = [resolveBlock copy]; RCTPromiseRejectBlock rejectCopy = [rejectBlock copy]; - RCTInvocationErrorHandler internalRejectCopy = nil; - if (internalRejectBlock != nil) { - internalRejectCopy = [internalRejectBlock copy]; - } + RCTInvocationErrorHandler internalRejectCopy = [internalRejectBlock copy]; [inv setArgument:(void *)&resolveCopy atIndex:count + 2]; [inv setArgument:(void *)&rejectCopy atIndex:count + 3]; From efa52b6b875c04fe24fbd6435be6bba73be75835 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Mon, 23 Oct 2023 20:20:39 +0200 Subject: [PATCH 48/55] remove extra internal reject wrapper --- packages/react-native/React/Base/RCTUtils.h | 2 +- packages/react-native/React/Base/RCTUtils.m | 2 +- .../platform/ios/ReactCommon/RCTTurboModule.h | 4 +- .../ios/ReactCommon/RCTTurboModule.mm | 101 ++++++++---------- 4 files changed, 50 insertions(+), 59 deletions(-) diff --git a/packages/react-native/React/Base/RCTUtils.h b/packages/react-native/React/Base/RCTUtils.h index dc218c47f199..ecc270e77f2b 100644 --- a/packages/react-native/React/Base/RCTUtils.h +++ b/packages/react-native/React/Base/RCTUtils.h @@ -73,7 +73,7 @@ RCT_EXTERN NSDictionary * RCTMakeAndLogError(NSString *message, id __nullable toStringify, NSDictionary *__nullable extraData); RCT_EXTERN NSDictionary *RCTJSErrorFromNSError(NSError *error); RCT_EXTERN NSDictionary - *RCTJSErrorFromCodeMessageAndNSError(NSString *code, NSString *message, NSError *__nullable error); + *RCTJSErrorFromCodeMessageAndNSError(NSString *__nullable code, NSString *__nullable message, NSError *__nullable error); // The default error code to use as the `code` property for callback error objects RCT_EXTERN NSString *const RCTErrorUnspecified; diff --git a/packages/react-native/React/Base/RCTUtils.m b/packages/react-native/React/Base/RCTUtils.m index bebbd61c8db3..028b593af65d 100644 --- a/packages/react-native/React/Base/RCTUtils.m +++ b/packages/react-native/React/Base/RCTUtils.m @@ -483,7 +483,7 @@ BOOL RCTClassOverridesInstanceMethod(Class cls, SEL selector) // TODO: Can we just replace RCTMakeError with this function instead? NSDictionary - *RCTJSErrorFromCodeMessageAndNSError(NSString *code, NSString *message, NSError *__nullable error) + *RCTJSErrorFromCodeMessageAndNSError(NSString *__nullable code, NSString *__nullable message, NSError *__nullable error) { NSString *errorMessage; NSArray *stackTrace = [NSThread callStackSymbols]; diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.h b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.h index 3a9b716cd70e..d19fcfdc3a9f 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.h +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.h @@ -148,14 +148,14 @@ class JSI_EXPORT ObjCTurboModule : public TurboModule { const char *methodName, NSInvocation *inv, NSMutableArray *retainedObjectsForInvocation, - RCTInvocationErrorHandler optionalInternalRejectBlock); + RCTPromiseRejectBlock reject); void performVoidMethodInvocation( jsi::Runtime &runtime, const char *methodName, NSInvocation *inv, NSMutableArray *retainedObjectsForInvocation); - using PromiseInvocationBlock = void (^)(RCTPromiseResolveBlock resolveWrapper, RCTPromiseRejectBlock rejectWrapper, RCTInvocationErrorHandler internalRejectWrapper); + using PromiseInvocationBlock = void (^)(RCTPromiseResolveBlock resolveWrapper, RCTPromiseRejectBlock rejectWrapper); jsi::Value createPromise(jsi::Runtime &runtime, std::string methodName, PromiseInvocationBlock invoke); }; diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm index ca2bbc394124..64227ebdd648 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm @@ -7,6 +7,7 @@ #import "RCTTurboModule.h" #import "RCTBlockGuard.h" +#import "RCTUtils.h" #include #import @@ -399,27 +400,24 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s handlePromiseRejection(reason); }; - RCTInvocationErrorHandler internalRejectBlock = nil; - - if (RCTRejectTurboModulePromiseOnNativeError()) { - internalRejectBlock = ^(NSDictionary *dict) { - handlePromiseRejection(dict); - }; - } - - invokeCopy(resolveBlock, rejectBlock, internalRejectBlock); + invokeCopy(resolveBlock, rejectBlock); return jsi::Value::undefined(); }); return Promise.callAsConstructor(runtime, fn); } -static NSDictionary * maybeCatchException( +static NSError * maybeCatchException( BOOL shoudCatch, - NSDictionary *cause) + NSDictionary *causeUserInfo, + NSError *overrideWithError) { if (shoudCatch) { - return cause; + if (overrideWithError) { + return overrideWithError; + } else { + return [[NSError alloc] initWithDomain:RCTErrorDomain code:-1 userInfo:causeUserInfo]; + } } else { // Crash on native layer there is no promise in JS to reject. // We executed JSFunction returning void asynchrounously. @@ -442,7 +440,7 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s const char *methodName, NSInvocation *inv, NSMutableArray *retainedObjectsForInvocation, - RCTInvocationErrorHandler optionalInternalRejectBlock) + RCTPromiseRejectBlock reject) { __block id result; __weak id weakModule = instance_; @@ -462,67 +460,51 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s TurboModulePerfLogger::asyncMethodCallExecutionStart(moduleName, methodNameStr.c_str(), asyncCallCounter); } - NSDictionary *caughtException = nil; - BOOL shouldCatchException = isSync || optionalInternalRejectBlock; + NSError *caughtException = nil; + BOOL shouldCatchException = isSync || reject; try { @try { [inv invokeWithTarget:strongModule]; } @catch (NSException *exception) { caughtException = maybeCatchException( - shouldCatchException, - @{ + shouldCatchException, @{ @"name": exception.name, - @"message": exception.reason, + NSLocalizedDescriptionKey: exception.reason, @"stackSymbols": exception.callStackSymbols, @"stackReturnAddresses": exception.callStackReturnAddresses, - }); + }, nil); } @catch (NSError *error) { - caughtException = maybeCatchException( - shouldCatchException, - @{ - @"userInfo": error.userInfo, - @"domain": error.domain, - }); + caughtException = maybeCatchException(shouldCatchException, nil, error); } @catch (NSString *errorMessage) { - caughtException = maybeCatchException( - shouldCatchException, - @{ - @"message": errorMessage, - }); + caughtException = maybeCatchException(shouldCatchException, @{ + NSLocalizedDescriptionKey: errorMessage, + }, nil); } @catch (id e) { - caughtException = maybeCatchException( - shouldCatchException, - @{ - @"message": @"Unknown Objective-C Object thrown.", - }); + caughtException = maybeCatchException(shouldCatchException, @{ + NSLocalizedDescriptionKey: @"Unknown Objective-C Object thrown.", + }, nil); } @finally { [retainedObjectsForInvocation removeAllObjects]; } } catch (const std::exception &exception) { - caughtException = maybeCatchException( - shouldCatchException, - @{ - @"message": [NSString stringWithUTF8String:exception.what()], - }); + caughtException = maybeCatchException(shouldCatchException, @{ + NSLocalizedDescriptionKey: [NSString stringWithUTF8String:exception.what()], + }, nil); } catch (const std::string &errorMessage) { - caughtException = maybeCatchException( - shouldCatchException, - @{ - @"message": [NSString stringWithUTF8String:errorMessage.c_str()], - }); + caughtException = maybeCatchException(shouldCatchException, @{ + NSLocalizedDescriptionKey: [NSString stringWithUTF8String:errorMessage.c_str()], + }, nil); } catch (...) { - caughtException = maybeCatchException( - shouldCatchException, - @{ - @"message": @"Unknown C++ exception thrown.", - }); + caughtException = maybeCatchException(shouldCatchException, @{ + NSLocalizedDescriptionKey: @"Unknown C++ exception thrown.", + }, nil); } if (caughtException) { if (isSync) { - throw convertNSDictionaryToJSError(runtime, caughtException); + throw convertNSDictionaryToJSError(runtime, RCTJSErrorFromCodeMessageAndNSError(nil, nil, caughtException)); } else { - optionalInternalRejectBlock(caughtException); + reject(nil, nil, caughtException); } } @@ -895,17 +877,26 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s switch (returnType) { case PromiseKind: { returnValue = createPromise( - runtime, methodNameStr, ^(RCTPromiseResolveBlock resolveBlock, RCTPromiseRejectBlock rejectBlock, RCTInvocationErrorHandler internalRejectBlock) { + runtime, methodNameStr, ^(RCTPromiseResolveBlock resolveBlock, RCTPromiseRejectBlock rejectBlock) { RCTPromiseResolveBlock resolveCopy = [resolveBlock copy]; RCTPromiseRejectBlock rejectCopy = [rejectBlock copy]; - RCTInvocationErrorHandler internalRejectCopy = [internalRejectBlock copy]; [inv setArgument:(void *)&resolveCopy atIndex:count + 2]; [inv setArgument:(void *)&rejectCopy atIndex:count + 3]; [retainedObjectsForInvocation addObject:resolveCopy]; [retainedObjectsForInvocation addObject:rejectCopy]; + + RCTPromiseRejectBlock internalReject = nil; + if (RCTRejectTurboModulePromiseOnNativeError()) { + internalReject = rejectCopy; + }; // The return type becomes void in the ObjC side. - performMethodInvocation(runtime, isMethodSync(VoidKind), methodName, inv, retainedObjectsForInvocation, internalRejectCopy); + performMethodInvocation(runtime, + isMethodSync(VoidKind), + methodName, + inv, + retainedObjectsForInvocation, + internalReject); }); break; } From ae80dc72f9ace9ed9e156412e7f9077e51a613aa Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Wed, 25 Oct 2023 11:57:10 +0200 Subject: [PATCH 49/55] Cleanup --- .../platform/ios/ReactCommon/RCTTurboModule.h | 6 --- .../ios/ReactCommon/RCTTurboModule.mm | 45 +++++++++---------- 2 files changed, 21 insertions(+), 30 deletions(-) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.h b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.h index d19fcfdc3a9f..e682194da046 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.h +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.h @@ -23,12 +23,6 @@ ((RCTTurboModuleEnabled() && [(klass) conformsToProtocol:@protocol(RCTTurboModule)])) #define RCT_IS_TURBO_MODULE_INSTANCE(module) RCT_IS_TURBO_MODULE_CLASS([(module) class]) -/** - * Block used to reject the JS promise waiting for a result - * in cases when the native method throws an exception. - */ -typedef void (^RCTInvocationErrorHandler)(NSDictionary *exception); - namespace facebook::react { class CallbackWrapper; diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm index 64227ebdd648..574572eb9ce0 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm @@ -253,14 +253,13 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s */ static jsi::Value createRejectJSErrorValue(jsi::Runtime &runtime, NSDictionary *reason, const std::optional &jsInvocationStack) { - jsi::Object cause = convertNSDictionaryToJSIObject(runtime, reason); jsi::Value error = createJSRuntimeError(runtime, "Exception in HostFunction: " + (reason[@"message"] ? std::string([reason[@"message"] UTF8String]) : "")); + error.asObject(runtime).setProperty(runtime, "cause", convertNSDictionaryToJSIObject(runtime, reason)); if (jsInvocationStack.has_value()) { error.asObject(runtime).setProperty(runtime, "stack", *jsInvocationStack); } - error.asObject(runtime).setProperty(runtime, "cause", std::move(cause)); return error; } @@ -273,6 +272,7 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s } jsi::Function Promise = runtime.global().getPropertyAsFunction(runtime, "Promise"); + // JS Stack at the time when the promise is created. std::optional jsInvocationStack; if (RCTTraceTurboModulePromiseRejections()) { @@ -283,7 +283,6 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s .utf8(runtime); } - std::string moduleName = name_; // Note: the passed invoke() block is not retained by default, so let's retain it here to help keep it longer. @@ -877,27 +876,25 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s switch (returnType) { case PromiseKind: { returnValue = createPromise( - runtime, methodNameStr, ^(RCTPromiseResolveBlock resolveBlock, RCTPromiseRejectBlock rejectBlock) { - RCTPromiseResolveBlock resolveCopy = [resolveBlock copy]; - RCTPromiseRejectBlock rejectCopy = [rejectBlock copy]; - - [inv setArgument:(void *)&resolveCopy atIndex:count + 2]; - [inv setArgument:(void *)&rejectCopy atIndex:count + 3]; - [retainedObjectsForInvocation addObject:resolveCopy]; - [retainedObjectsForInvocation addObject:rejectCopy]; - - RCTPromiseRejectBlock internalReject = nil; - if (RCTRejectTurboModulePromiseOnNativeError()) { - internalReject = rejectCopy; - }; - // The return type becomes void in the ObjC side. - performMethodInvocation(runtime, - isMethodSync(VoidKind), - methodName, - inv, - retainedObjectsForInvocation, - internalReject); - }); + runtime, methodNameStr, ^(RCTPromiseResolveBlock resolveBlock, RCTPromiseRejectBlock rejectBlock) { + RCTPromiseResolveBlock resolveCopy = [resolveBlock copy]; + RCTPromiseRejectBlock rejectCopy = [rejectBlock copy]; + [inv setArgument:(void *)&resolveCopy atIndex:count + 2]; + [inv setArgument:(void *)&rejectCopy atIndex:count + 3]; + [retainedObjectsForInvocation addObject:resolveCopy]; + [retainedObjectsForInvocation addObject:rejectCopy]; + RCTPromiseRejectBlock internalReject = nil; + if (RCTRejectTurboModulePromiseOnNativeError()) { + internalReject = rejectCopy; + }; + // The return type becomes void in the ObjC side. + performMethodInvocation(runtime, + isMethodSync(VoidKind), + methodName, + inv, + retainedObjectsForInvocation, + internalReject); + }); break; } case VoidKind: { From d5aa06ba084248e5dd9647bc14fcf352d6942542 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Wed, 25 Oct 2023 11:59:26 +0200 Subject: [PATCH 50/55] Fix lint --- .../core/platform/ios/ReactCommon/RCTTurboModule.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm index 574572eb9ce0..889f125d50dd 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm @@ -894,7 +894,7 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s inv, retainedObjectsForInvocation, internalReject); - }); + }); break; } case VoidKind: { From be13239ece8a174c1458105868ab377c93a3cb1f Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Sun, 10 Mar 2024 17:50:01 +0100 Subject: [PATCH 51/55] Fix merge --- .../ios/ReactCommon/RCTTurboModule.mm | 34 ++++++------------- 1 file changed, 10 insertions(+), 24 deletions(-) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm index f06d03350bb3..09c6585a41e3 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm @@ -6,7 +6,6 @@ */ #import "RCTTurboModule.h" -#import "RCTBlockGuard.h" #import "RCTUtils.h" #include @@ -231,30 +230,19 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s return jsi::JSError(runtime, std::move(error)); } -/** - * Creates JSErrorValue with given stack trace. - */ -static jsi::Value createRejectJSErrorValue(jsi::Runtime &runtime, NSDictionary *reason, const std::optional &jsInvocationStack) -{ - jsi::Value error = createJSRuntimeError(runtime, "Exception in HostFunction: " + (reason[@"message"] ? std::string([reason[@"message"] UTF8String]) : "")); - error.asObject(runtime).setProperty(runtime, "cause", convertNSDictionaryToJSIObject(runtime, reason)); - - if (jsInvocationStack.has_value()) { - error.asObject(runtime).setProperty(runtime, "stack", *jsInvocationStack); - } - - return error; -} - /** * Creates JS error value with current JS runtime and error details. */ -static jsi::Value convertJSErrorDetailsToJSRuntimeError(jsi::Runtime &runtime, NSDictionary *jsErrorDetails) +static jsi::Value convertJSErrorDetailsToJSRuntimeError(jsi::Runtime &runtime, NSDictionary *jsErrorDetails, const std::optional &jsInvocationStack) { NSString *message = jsErrorDetails[@"message"]; auto jsError = createJSRuntimeError(runtime, [message UTF8String]); jsError.asObject(runtime).setProperty(runtime, "cause", convertObjCObjectToJSIValue(runtime, jsErrorDetails)); + + if (jsInvocationStack.has_value()) { + jsError.asObject(runtime).setProperty(runtime, "stack", *jsInvocationStack); + } return jsError; } @@ -279,8 +267,6 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s .utf8(runtime); } - std::string moduleName = name_; - // Note: the passed invoke() block is not retained by default, so let's retain it here to help keep it longer. // Otherwise, there's a risk of it getting released before the promise function below executes. PromiseInvocationBlock invokeCopy = [invoke copy]; @@ -340,8 +326,8 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s } NSDictionary *jsErrorDetails = RCTJSErrorFromCodeMessageAndNSError(code, message, error); - reject->call([jsErrorDetails](jsi::Runtime &rt, jsi::Function &jsFunction) { - jsFunction.call(rt, convertJSErrorDetailsToJSRuntimeError(rt, jsErrorDetails)); + reject->call([jsErrorDetails, jsInvocationStack](jsi::Runtime &rt, jsi::Function &jsFunction) { + jsFunction.call(rt, convertJSErrorDetailsToJSRuntimeError(rt, jsErrorDetails, jsInvocationStack)); }); resolveWasCalled = NO; resolve = std::nullopt; @@ -844,9 +830,9 @@ SystraceSection s( [inv setArgument:(void *)&rejectCopy atIndex:count + 3]; [retainedObjectsForInvocation addObject:resolveCopy]; [retainedObjectsForInvocation addObject:rejectCopy]; - RCTPromiseRejectBlock internalReject = nil; + RCTPromiseRejectBlock rejectOnNativeError = nil; if (RCTRejectTurboModulePromiseOnNativeError()) { - internalReject = rejectCopy; + rejectOnNativeError = rejectCopy; }; // The return type becomes void in the ObjC side. performMethodInvocation(runtime, @@ -854,7 +840,7 @@ SystraceSection s( methodName, inv, retainedObjectsForInvocation, - internalReject); + rejectOnNativeError); }); break; } From f2e6d32c86ec3b4546bc748a1d2a1768523f37e0 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Sun, 10 Mar 2024 18:36:31 +0100 Subject: [PATCH 52/55] Fix review comments --- .../platform/ios/ReactCommon/RCTTurboModule.h | 2 +- .../ios/ReactCommon/RCTTurboModule.mm | 47 ++++++++++--------- 2 files changed, 26 insertions(+), 23 deletions(-) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.h b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.h index ffb3f0e7aa3e..b578e07ea7a9 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.h +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.h @@ -142,7 +142,7 @@ class JSI_EXPORT ObjCTurboModule : public TurboModule { const char *methodName, NSInvocation *inv, NSMutableArray *retainedObjectsForInvocation, - RCTPromiseRejectBlock reject); + _Nullable RCTPromiseRejectBlock reject); void performVoidMethodInvocation( jsi::Runtime &runtime, const char *methodName, diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm index 09c6585a41e3..62427eb99441 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm @@ -341,20 +341,24 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s static NSError * maybeCatchException( BOOL shoudCatch, - NSDictionary *causeUserInfo, - NSError *overrideWithError) + id causeOrError) { - if (shoudCatch) { - if (overrideWithError) { - return overrideWithError; - } else { - return [[NSError alloc] initWithDomain:RCTErrorDomain code:-1 userInfo:causeUserInfo]; + if (!shoudCatch) { + // Crash on native layer there is no promise in JS to reject. + // We executed JSFunction returning void asynchrounously. + throw; } - } else { - // Crash on native layer there is no promise in JS to reject. - // We executed JSFunction returning void asynchrounously. - throw; - } + + if ([causeOrError isKindOfClass:[NSError class]]) { + return causeOrError; + } + + if ([causeOrError isKindOfClass:[NSDictionary class]]) { + return [[NSError alloc] initWithDomain:RCTErrorDomain code:-1 userInfo:causeOrError]; + } + + // This should never happen, to avoid consequent errors, we wrapp the unknown value in NSError. + return [[NSError alloc] initWithDomain:RCTErrorDomain code:-1 userInfo:@{ @"unknown": causeOrError }]; } /** @@ -372,7 +376,7 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s const char *methodName, NSInvocation *inv, NSMutableArray *retainedObjectsForInvocation, - RCTPromiseRejectBlock reject) + _Nullable RCTPromiseRejectBlock reject) { __block id result; __weak id weakModule = instance_; @@ -404,33 +408,32 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s NSLocalizedDescriptionKey: exception.reason, @"stackSymbols": exception.callStackSymbols, @"stackReturnAddresses": exception.callStackReturnAddresses, - }, nil); + }); } @catch (NSError *error) { - caughtException = maybeCatchException(shouldCatchException, nil, error); + caughtException = maybeCatchException(shouldCatchException, error); } @catch (NSString *errorMessage) { caughtException = maybeCatchException(shouldCatchException, @{ NSLocalizedDescriptionKey: errorMessage, - }, nil); + }); } @catch (id e) { caughtException = maybeCatchException(shouldCatchException, @{ NSLocalizedDescriptionKey: @"Unknown Objective-C Object thrown.", - }, nil); - } @finally { - [retainedObjectsForInvocation removeAllObjects]; + }); } } catch (const std::exception &exception) { caughtException = maybeCatchException(shouldCatchException, @{ NSLocalizedDescriptionKey: [NSString stringWithUTF8String:exception.what()], - }, nil); + }); } catch (const std::string &errorMessage) { caughtException = maybeCatchException(shouldCatchException, @{ NSLocalizedDescriptionKey: [NSString stringWithUTF8String:errorMessage.c_str()], - }, nil); + }); } catch (...) { caughtException = maybeCatchException(shouldCatchException, @{ NSLocalizedDescriptionKey: @"Unknown C++ exception thrown.", - }, nil); + }); } + [retainedObjectsForInvocation removeAllObjects]; if (caughtException) { if (isSync) { From 229ec73e659f1f9ae43db5c03d1763d00d261bba Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Sun, 10 Mar 2024 18:55:52 +0100 Subject: [PATCH 53/55] remove duplicate imports --- .../core/platform/ios/ReactCommon/RCTTurboModule.mm | 9 --------- 1 file changed, 9 deletions(-) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm index 62427eb99441..d1abf303d39a 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm @@ -6,15 +6,6 @@ */ #import "RCTTurboModule.h" -#import "RCTUtils.h" - -#include -#import -#import -#import -#import -#import -#import #import #import From bfb410c30409917d2bb320b740d172cf288df486 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Sun, 14 Jul 2024 23:02:52 +0200 Subject: [PATCH 54/55] fixing review comments --- .../core/platform/ios/ReactCommon/RCTTurboModule.mm | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm index c708cfe92d67..22094a28dd53 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm @@ -216,7 +216,7 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s */ static jsi::JSError convertNSDictionaryToJSError(jsi::Runtime &runtime, NSDictionary *cause) { - jsi::Value error = createJSRuntimeError(runtime, "Exception in HostFunction: " + (cause[@"message"] ? std::string([cause[@"message"] UTF8String]) : "")); + jsi::Value error = createJSRuntimeError(runtime, std::string("Exception in HostFunction: ") + (cause[@"message"] ? [cause[@"message"] UTF8String] : "")); error.asObject(runtime).setProperty(runtime, "cause", convertNSDictionaryToJSIObject(runtime, cause)); return jsi::JSError(runtime, std::move(error)); } @@ -256,7 +256,7 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s // JS Stack at the time when the promise is created. std::optional jsInvocationStack; if (RCTTraceTurboModulePromiseRejections()) { - jsInvocationStack = createJSRuntimeError(runtime, "") + jsInvocationStack = createJSRuntimeError(runtime, jsi::Value::undefined()) .asObject(runtime) .getProperty(runtime, "stack") .asString(runtime) @@ -415,6 +415,8 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s caughtException = maybeCatchException(shouldCatchException, @{ NSLocalizedDescriptionKey: @"Unknown Objective-C Object thrown.", }); + } @finally { + [retainedObjectsForInvocation removeAllObjects]; } } catch (const std::exception &exception) { caughtException = maybeCatchException(shouldCatchException, @{ @@ -429,7 +431,6 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s NSLocalizedDescriptionKey: @"Unknown C++ exception thrown.", }); } - [retainedObjectsForInvocation removeAllObjects]; if (caughtException) { if (isSync) { From 04fb46ea7029a5b09b803311921d33c0a3661689 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Mon, 15 Jul 2024 06:07:15 +0200 Subject: [PATCH 55/55] fix createJSRuntimeError and NSException parsing --- .../ios/ReactCommon/RCTTurboModule.mm | 33 ++++++++++++------- 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm index 22094a28dd53..411da3d22b36 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm @@ -187,9 +187,14 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s throw std::runtime_error("Unsupported jsi::Value kind"); } -static jsi::Value createJSRuntimeError(jsi::Runtime &runtime, const std::string &message) -{ - return runtime.global().getPropertyAsFunction(runtime, "Error").call(runtime, message); +jsi::Value createJSRuntimeError(jsi::Runtime& runtime, jsi::Value&& message) { + return runtime.global() + .getPropertyAsFunction(runtime, "Error") + .call(runtime, std::move(message)); +} + +jsi::Value createJSRuntimeError(jsi::Runtime& runtime, const std::string& message) { + return createJSRuntimeError(runtime, jsi::String::createFromUtf8(runtime, message)); } /** @@ -228,7 +233,7 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s { NSString *message = jsErrorDetails[@"message"]; - auto jsError = createJSRuntimeError(runtime, [message UTF8String]); + auto jsError = createJSRuntimeError(runtime, std::string([message UTF8String])); jsError.asObject(runtime).setProperty(runtime, "cause", convertObjCObjectToJSIValue(runtime, jsErrorDetails)); if (jsInvocationStack.has_value()) { @@ -352,7 +357,17 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s if ([causeOrError isKindOfClass:[NSDictionary class]]) { return [[NSError alloc] initWithDomain:RCTErrorDomain code:-1 userInfo:causeOrError]; } - + + if ([causeOrError isKindOfClass:[NSException class]]) { + NSException *exception = (NSException *)causeOrError; + return [[NSError alloc] initWithDomain:RCTErrorDomain code:-1 userInfo:@{ + @"name": exception.name, + NSLocalizedDescriptionKey: exception.reason, + @"stackSymbols": exception.callStackSymbols, + @"stackReturnAddresses": exception.callStackReturnAddresses, + }]; + } + // This should never happen, to avoid consequent errors, we wrapp the unknown value in NSError. return [[NSError alloc] initWithDomain:RCTErrorDomain code:-1 userInfo:@{ @"unknown": causeOrError }]; } @@ -398,13 +413,7 @@ id convertJSIValueToObjCObject(jsi::Runtime &runtime, const jsi::Value &value, s @try { [inv invokeWithTarget:strongModule]; } @catch (NSException *exception) { - caughtException = maybeCatchException( - shouldCatchException, @{ - @"name": exception.name, - NSLocalizedDescriptionKey: exception.reason, - @"stackSymbols": exception.callStackSymbols, - @"stackReturnAddresses": exception.callStackReturnAddresses, - }); + caughtException = maybeCatchException(shouldCatchException, exception); } @catch (NSError *error) { caughtException = maybeCatchException(shouldCatchException, error); } @catch (NSString *errorMessage) {