Attach TurboModule identity to exceptions rethrown from async and void calls (#58264)

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 <RCTBridgeModule>
+
+@end
+
+@implementation RCTThrowingTurboModule
+
+RCT_EXPORT_MODULE()
+
+// A plain `NSArray *` parameter has no element converter to sanitise it (unlike, say,
+// `NSArray<NSString *> *`, 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<ExceptionCapturingNativeMethodCallInvoker>();
+  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<facebook::jsi::Value, 1> 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

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];
     }