diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/iostests/RCTTurboModuleTests.mm b/packages/react-native/ReactCommon/react/nativemodule/core/iostests/RCTTurboModuleTests.mm index a4d41956ea2a..693cddd7d398 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/iostests/RCTTurboModuleTests.mm +++ b/packages/react-native/ReactCommon/react/nativemodule/core/iostests/RCTTurboModuleTests.mm @@ -48,6 +48,27 @@ - (void)logEvent:(NSString *)eventName data:(NSDictionary *)data analyticsModule @end +@interface RCTThrowingTurboModule : NSObject + +@end + +@implementation RCTThrowingTurboModule + +RCT_EXPORT_MODULE() + +// A plain `NSArray *` parameter has no element converter to sanitise it (unlike, say, +// `NSArray *`, which RCTConvert routes through `NSStringArray:` and which drops nulls), +// so `convertJSIArrayToNSArray` substituting `[NSNull null]` for a null element to preserve the +// indices is what this loop actually receives from a JS caller passing `['a', null]`. +RCT_EXPORT_METHOD(testMethodWhichReadsStringsFromArray : (NSArray *)items) +{ + for (NSUInteger i = 0; i < items.count; i++) { + (void)[(NSString *)items[i] length]; + } +} + +@end + class ReactNativeFeatureFlagsNSNullConversionEnabled : public ReactNativeFeatureFlagsDefaults { public: bool enableModuleArgumentNSNullConversionIOS() override @@ -118,6 +139,37 @@ void invokeSync(const std::string &methodName, NativeMethodCallFunc &&func) noex } }; +// `NativeMethodCallInvoker::invokeAsync` is noexcept, so an NSException escaping the async +// invocation terminates the process — which is the production failure mode, but leaves nothing for +// a test to inspect. Catching here stands in for the process-level handler and puts the exception +// exactly where that handler would see it. +class ExceptionCapturingNativeMethodCallInvoker : public NativeMethodCallInvoker { + public: + __strong NSException *caught = nil; + + void invokeAsync(const std::string & /*methodName*/, NativeMethodCallFunc &&func) noexcept override + { + // The outer C++ handler is what makes the `noexcept` honest: `func` is a std::function, and + // invoking an empty one throws a `std::bad_function_call` that `@catch (NSException *)` cannot + // bind. + try { + @try { + func(); + } @catch (NSException *exception) { + caught = exception; + } + } catch (...) { + } + } + void invokeSync(const std::string & /*methodName*/, NativeMethodCallFunc &&func) noexcept override + { + try { + func(); + } catch (...) { + } + } +}; + @interface RCTTurboModuleTests : XCTestCase @end @@ -253,6 +305,55 @@ - (void)testInvokeTurboModuleKeepsNestedNullAsNSNullWhenFlagEnabled OCMVerify(OCMTimes(1), [instance_ testMethodWhichTakesObject:@{@"foo" : (id)kCFNull}]); } +// Void methods are always async, so an NSException raised by the module unwinds past every module +// frame before anything reports it. The rethrow is the last point at which the failing module and +// method are still known, so it has to put them on the exception. +- (void)testVoidMethodExceptionCarriesModuleAndMethodName +{ + auto hermesRuntime = facebook::hermes::makeHermesRuntime(); + facebook::jsi::Runtime *rt = hermesRuntime.get(); + + auto invoker = std::make_shared(); + RCTThrowingTurboModule *instance = [RCTThrowingTurboModule new]; + ObjCTurboModule::InitParams params = { + .moduleName = "ThrowingTestModule", + .instance = instance, + .jsInvoker = nullptr, + .nativeMethodCallInvoker = invoker, + .isSyncModule = false, + }; + ObjCTurboModule module(params); + + auto items = facebook::jsi::Array(*rt, 2); + items.setValueAtIndex(*rt, 0, facebook::jsi::String::createFromAscii(*rt, "a")); + items.setValueAtIndex(*rt, 1, facebook::jsi::Value::null()); + std::array args = {facebook::jsi::Value(*rt, items)}; + + module.invokeObjCMethod( + *rt, + VoidKind, + "testMethodWhichReadsStringsFromArray", + @selector(testMethodWhichReadsStringsFromArray:), + args.data(), + 1); + + NSException *caught = invoker->caught; + XCTAssertNotNil(caught, @"Sending -length to the NSNull standing in for the null element must raise"); + XCTAssertEqualObjects(caught.name, NSInvalidArgumentException); + XCTAssertTrue( + [caught.reason containsString:@"ThrowingTestModule"], @"reason must name the module: %@", caught.reason); + XCTAssertTrue( + [caught.reason containsString:@"testMethodWhichReadsStringsFromArray"], + @"reason must name the method: %@", + caught.reason); + // The original failure has to survive alongside the identity rather than be replaced by it. + XCTAssertTrue([caught.reason containsString:@"unrecognized selector"], @"%@", caught.reason); + NSException *original = caught.userInfo[@"RCTTurboModuleOriginalException"]; + XCTAssertNotNil(original); + XCTAssertNotNil(original.callStackReturnAddresses); + XCTAssertNotNil(original.callStackSymbols); +} + // A native-backed ArrayBuffer is aliased rather than copied, and the RCTArrayBuffer retains // the backing MutableBuffer, so the alias outlives the JS object. - (void)testNativeBackedArrayBufferIsAliasedAndKeepsBackingStoreAlive 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 6efbe9db1f4f..b8b5f2db3cd9 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 @@ -307,6 +307,35 @@ id convertJSIValueToObjCObject( return {runtime, std::move(error)}; } +/** + * userInfo key under which `addModuleIdentityToException` preserves the exception it wraps, so its + * raise-site `callStackReturnAddresses` and `callStackSymbols` stay available to symbolication. + */ +static NSString *const RCTTurboModuleOriginalExceptionKey = @"RCTTurboModuleOriginalException"; + +/** + * Async and void method calls have no JS runtime to attach a JSError to, so an NSException raised + * by the module escapes to the process-level handler instead. The stack it arrives with has + * already unwound past the module, so unless the module and method names travel on the exception + * itself the resulting crash cannot be attributed to an owning module. + */ +static NSException * +addModuleIdentityToException(NSException *exception, const std::string &moduleName, const std::string &methodName) +{ + // A newly constructed NSException captures its call stack at @throw rather than at the original + // raise, so the original exception is carried across whole. + NSMutableDictionary *userInfo = + [NSMutableDictionary dictionaryWithDictionary:exception.userInfo != nil ? exception.userInfo : @{}]; + userInfo[RCTTurboModuleOriginalExceptionKey] = exception; + + return [NSException exceptionWithName:exception.name + reason:[NSString stringWithFormat:@"%s.%s raised an exception: %@", + moduleName.c_str(), + methodName.c_str(), + exception.reason] + userInfo:userInfo]; +} + /** * Creates JS error value with current JS runtime and error details. */ @@ -477,7 +506,7 @@ id convertJSIValueToObjCObject( // See https://github.com/reactwg/react-native-new-architecture/discussions/276#discussioncomment-12567155 throw convertNSExceptionToJSError(runtime, exception, std::string{moduleName}, methodNameStr); } else { - @throw exception; + @throw addModuleIdentityToException(exception, std::string{moduleName}, methodNameStr); } } @finally { [retainedObjectsForInvocation removeAllObjects]; @@ -539,7 +568,7 @@ TraceSection s( } @catch (NSException *exception) { // Void methods are always async, re-throw instead of converting to // JSError, same as the async branch in performMethodInvocation. - @throw exception; + @throw addModuleIdentityToException(exception, std::string{moduleName}, methodNameStr); } @finally { [retainedObjectsForInvocation removeAllObjects]; }