Skip to content

Commit dfde51e

Browse files
cipolleschifacebook-github-bot
authored andcommitted
Convert to JSException only NSException from sync methods (#50193)
Summary: Pull Request resolved: #50193 This fix makes sure that we convert to JSException only NSException thrwn by sync methods. Currently, nothing in the stack will be capable of understanding that js error if it is triggered by an exception raised by an asyc method. See reactwg/react-native-new-architecture#276 for further details We need to cherry pick this in 0.78 and 0.79 ## Changelog: [iOS][Fixed] - Make sure the TM infra does not crash on NSException when triggered by async method Reviewed By: fabriziocucci Differential Revision: D71619229 fbshipit-source-id: b87aef5dd2720a2641c8da0904da651866370dc6
1 parent 7b500b8 commit dfde51e

File tree

1 file changed

+24
-13
lines changed
  • packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon

1 file changed

+24
-13
lines changed

packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm

+24-13
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ static int32_t getUniqueId()
5757

5858
static jsi::String convertNSStringToJSIString(jsi::Runtime &runtime, NSString *value)
5959
{
60-
return jsi::String::createFromUtf8(runtime, [value UTF8String] ?: "");
60+
return jsi::String::createFromUtf8(runtime, [value UTF8String] ? [value UTF8String] : "");
6161
}
6262

6363
static jsi::Object convertNSDictionaryToJSIObject(jsi::Runtime &runtime, NSDictionary *value)
@@ -213,7 +213,11 @@ id convertJSIValueToObjCObject(
213213
/**
214214
* Creates JSError with current JS runtime and NSException stack trace.
215215
*/
216-
static jsi::JSError convertNSExceptionToJSError(jsi::Runtime &runtime, NSException *exception)
216+
static jsi::JSError convertNSExceptionToJSError(
217+
jsi::Runtime &runtime,
218+
NSException *exception,
219+
const std::string &moduleName,
220+
const std::string &methodName)
217221
{
218222
std::string reason = [exception.reason UTF8String];
219223

@@ -224,7 +228,8 @@ id convertJSIValueToObjCObject(
224228
cause.setProperty(
225229
runtime, "stackReturnAddresses", convertNSArrayToJSIArray(runtime, exception.callStackReturnAddresses));
226230

227-
jsi::Value error = createJSRuntimeError(runtime, "Exception in HostFunction: " + reason);
231+
std::string message = moduleName + "." + methodName + " raised an exception: " + reason;
232+
jsi::Value error = createJSRuntimeError(runtime, message);
228233
error.asObject(runtime).setProperty(runtime, "cause", std::move(cause));
229234
return {runtime, std::move(error)};
230235
}
@@ -356,28 +361,34 @@ id convertJSIValueToObjCObject(
356361
}
357362

358363
if (isSync) {
359-
TurboModulePerfLogger::syncMethodCallExecutionStart(moduleName, methodNameStr.c_str());
364+
TurboModulePerfLogger::syncMethodCallExecutionStart(moduleName, methodName);
360365
} else {
361-
TurboModulePerfLogger::asyncMethodCallExecutionStart(moduleName, methodNameStr.c_str(), asyncCallCounter);
366+
TurboModulePerfLogger::asyncMethodCallExecutionStart(moduleName, methodName, asyncCallCounter);
362367
}
363368

364369
@try {
365370
[inv invokeWithTarget:strongModule];
366371
} @catch (NSException *exception) {
367-
throw convertNSExceptionToJSError(runtime, exception);
372+
if (isSync) {
373+
// We can only convert NSException to JSError in sync method calls.
374+
// See https://github.com/reactwg/react-native-new-architecture/discussions/276#discussioncomment-12567155
375+
throw convertNSExceptionToJSError(runtime, exception, std::string{moduleName}, methodNameStr);
376+
} else {
377+
@throw exception;
378+
}
368379
} @finally {
369380
[retainedObjectsForInvocation removeAllObjects];
370381
}
371382

372383
if (!isSync) {
373-
TurboModulePerfLogger::asyncMethodCallExecutionEnd(moduleName, methodNameStr.c_str(), asyncCallCounter);
384+
TurboModulePerfLogger::asyncMethodCallExecutionEnd(moduleName, methodName, asyncCallCounter);
374385
return;
375386
}
376387

377388
void *rawResult;
378389
[inv getReturnValue:&rawResult];
379390
result = (__bridge id)rawResult;
380-
TurboModulePerfLogger::syncMethodCallExecutionEnd(moduleName, methodNameStr.c_str());
391+
TurboModulePerfLogger::syncMethodCallExecutionEnd(moduleName, methodName);
381392
};
382393

383394
if (isSync) {
@@ -419,23 +430,23 @@ TraceSection s(
419430
}
420431

421432
if (shouldVoidMethodsExecuteSync_) {
422-
TurboModulePerfLogger::syncMethodCallExecutionStart(moduleName, methodNameStr.c_str());
433+
TurboModulePerfLogger::syncMethodCallExecutionStart(moduleName, methodName);
423434
} else {
424-
TurboModulePerfLogger::asyncMethodCallExecutionStart(moduleName, methodNameStr.c_str(), asyncCallCounter);
435+
TurboModulePerfLogger::asyncMethodCallExecutionStart(moduleName, methodName, asyncCallCounter);
425436
}
426437

427438
@try {
428439
[inv invokeWithTarget:strongModule];
429440
} @catch (NSException *exception) {
430-
throw convertNSExceptionToJSError(runtime, exception);
441+
throw convertNSExceptionToJSError(runtime, exception, std::string{moduleName}, methodNameStr);
431442
} @finally {
432443
[retainedObjectsForInvocation removeAllObjects];
433444
}
434445

435446
if (shouldVoidMethodsExecuteSync_) {
436-
TurboModulePerfLogger::syncMethodCallExecutionEnd(moduleName, methodNameStr.c_str());
447+
TurboModulePerfLogger::syncMethodCallExecutionEnd(moduleName, methodName);
437448
} else {
438-
TurboModulePerfLogger::asyncMethodCallExecutionEnd(moduleName, methodNameStr.c_str(), asyncCallCounter);
449+
TurboModulePerfLogger::asyncMethodCallExecutionEnd(moduleName, methodName, asyncCallCounter);
439450
}
440451

441452
return;

0 commit comments

Comments
 (0)