Attach TurboModule identity to exceptions rethrown from async and void calls (#58264) - #58264
Attach TurboModule identity to exceptions rethrown from async and void calls (#58264)#58264christophpurrer wants to merge 1 commit into
Conversation
|
@christophpurrer has exported this pull request. If you are a Meta employee, you can view the originating Diff in D118144605. |
…d calls (react#58264) 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 `<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 Differential Revision: D118144605
fd3c00a to
76eead6
Compare
…d calls (react#58264) 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 `<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 Differential Revision: D118144605
76eead6 to
8d9e681
Compare
|
/review |
🤖 AI code reviewDecision: Ready for human review Overall PR risk: Low. Change modifies existing behavior in the iOS ObjC TurboModule async and void NSException rethrow path to include module and method identity in the rethrown exception. Blast radius is limited to iOS diagnostics with no JS/Android or public API change, and rollback is a single-file revert. Most plausible breakage is crash grouping or log parsers that expect the prior exact reason string or do not expect the new userInfo key; no warning or critical findings were reported. No findings. This review is advisory — it never blocks a merge and never auto-approves. |
8d9e681 to
e75ff79
Compare
…d calls (react#58264) 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 `<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
…d calls (react#58264) 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 `<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
e75ff79 to
91b1af0
Compare
|
This pull request has been merged in 253a84c. |
Summary:
When an ObjC TurboModule method raises an
NSException, what happens next depends on how it wascalled. 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 — theyrun on the module's method queue with no JS runtime to attach the error to — so they rethrow.
Both rethrow sites discarded
moduleNameandmethodNameStr, even though both are captured in theenclosing 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_rethrowfollowed by alibdispatch 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
addModuleIdentityToExceptionhelper next toconvertNSExceptionToJSErrorand appliesit at both rethrow sites. It preserves the exception's
nameand its existinguserInfoentries soany predicate-based handling is unaffected, and prefixes
reasonwith<module>.<method>to matchthe sync path's wording. A freshly constructed
NSExceptioncaptures its call stack atthrowrather than at the original raise, so the raise-site return addresses are carried across in
userInfoand 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