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

Summary:
Pull Request resolved: #58264

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
`<module>.<method> raised an exception: <reason>`. 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 `<module>.<method>` 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

fbshipit-source-id: fb51936e73c7705ae4e23f650834d2c66e66a50e

Author: christophpurrer

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