From 91b1af05989e00367d8e594c58250e0a8ce7da21 Mon Sep 17 00:00:00 2001 From: Christoph Purrer Date: Tue, 1 Sep 2026 12:49:38 -0700 Subject: [PATCH] Attach TurboModule identity to exceptions rethrown from async and void calls (#58264) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: When an ObjC TurboModule method raises an `NSException`, what happens next depends on how it was called. A sync call converts it into a JSError via `convertNSExceptionToJSError`, which builds `. raised an exception: `. The async and void paths cannot do that — they run on the module's method queue with no JS runtime to attach the error to — so they rethrow. Both rethrow sites discarded `moduleName` and `methodNameStr`, even though both are captured in the enclosing block and in scope at the throw site. Because void and async methods are dispatched onto the method queue, the rethrown exception is uncaught and terminates the process, and by then every module frame has unwound: the reported stack bottoms out in `objc_exception_rethrow` followed by a libdispatch queue drain. Nothing in the resulting crash says which module or method failed. The practical effect is that all such crashes — regardless of which module raised them, and regardless of whether the underlying bug is a null argument, a wrong-typed argument, or anything else — collapse into a single crash bucket with no owner attached, and cannot be split or routed. This adds an `addModuleIdentityToException` helper next to `convertNSExceptionToJSError` and applies it at both rethrow sites. It preserves the exception's `name` and its existing `userInfo` entries so any predicate-based handling is unaffected, and prefixes `reason` with `.` to match the sync path's wording. A freshly constructed `NSException` captures its call stack at `throw` rather than at the original raise, so the raise-site return addresses are carried across in `userInfo` and nothing is lost. Behaviour is otherwise unchanged: the exception is still thrown, on the same thread, at the same point, with the same name. Nothing is caught, swallowed, logged away, or downgraded. Reviewers should expect the crash grouping to change: the existing aggregate bucket will drain and be replaced by per-module buckets. That is the point of the change, but it is worth knowing before it happens. Changelog: [iOS][Fixed] - Include the module and method name in exceptions rethrown from async and void TurboModule calls Reviewed By: javache Differential Revision: D118144605 --- .../core/iostests/RCTTurboModuleTests.mm | 101 ++++++++++++++++++ .../ios/ReactCommon/RCTTurboModule.mm | 33 +++++- 2 files changed, 132 insertions(+), 2 deletions(-) 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]; }