Convert to JSException only NSException from sync methods#50193
Closed
cipolleschi wants to merge 1 commit intofacebook:mainfrom
Closed
Convert to JSException only NSException from sync methods#50193cipolleschi wants to merge 1 commit intofacebook:mainfrom
cipolleschi wants to merge 1 commit intofacebook:mainfrom
Conversation
facebook-github-bot
added
CLA Signed
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D71619229 |
cipolleschi
force-pushed
the
export-D71619229
branch
from
eae4ff2 to
20a6167
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D71619229 |
…0193) Summary: 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
cipolleschi
force-pushed
the
export-D71619229
branch
from
20a6167 to
0d7c572
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D71619229 |
Contributor
|
This pull request has been merged in dfde51e. |
Collaborator
|
This pull request was successfully merged by @cipolleschi in dfde51e When will my fix make it into a release? | How to file a pick request? |
react-native-bot
pushed a commit
that referenced
this pull request
Mar 24, 2025
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
Collaborator
|
This pull request was successfully merged by @cipolleschi in 9805a4f When will my fix make it into a release? | How to file a pick request? |
react-native-bot
pushed a commit
that referenced
this pull request
Mar 24, 2025
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
Collaborator
|
This pull request was successfully merged by @cipolleschi in 1e9f118 When will my fix make it into a release? | How to file a pick request? |
fabriziocucci
pushed a commit
that referenced
this pull request
Mar 24, 2025
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
Collaborator
|
This pull request was successfully merged by @cipolleschi in 8eec35f When will my fix make it into a release? | How to file a pick request? |
cipolleschi
added a commit
that referenced
this pull request
Apr 1, 2025
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
Collaborator
|
This pull request was successfully merged by @cipolleschi in ae1841a When will my fix make it into a release? | How to file a pick request? |
This was referenced Apr 1, 2025
This was referenced Apr 11, 2025
ghenry22
added a commit
to ghenry22/substreamer
that referenced
this pull request
Apr 10, 2026
Adds RNTPSafeCall.h with the RNTP_SAFE_CALL macro and wraps all 37 bridge-callable method bodies in TrackPlayer.mm. Any NSException raised by the Swift impl is caught and routed to the JS reject block instead of crashing the process with SIGABRT. Defense-in-depth against U6 (facebook/react-native#50193): the upstream fix only catches NSException for value-returning TurboModule methods on iOS. RNTP exposes void-returning methods, which remain uncaught — and Swift do/catch does not catch NSException, so any as!/!! failure inside the impl bypasses Swift error handling and reaches the bridge. NSLog only — no crash reporter.
ghenry22
added a commit
to ghenry22/substreamer
that referenced
this pull request
Apr 10, 2026
Four HIGH-risk NSException raise sites in the Swift impl:
- TrackPlayer.swift:661 (getQueue) — `as! Track` on every queue item.
Now compactMaps with `as? Track`, dropping non-Track items silently.
- TrackPlayer.swift:736 (updateMetadata) — `as! Track` after index check.
Now `as?` + reject("invalid_track_object", ...) on cast failure.
- MediaURL.swift:24, 33 — two `as! String` casts in the failable init.
Now `as? String` + return nil. Init is already declared failable, so
returning nil for malformed JS input matches its contract instead of
crashing the bridge with an NSException.
Removes the four most dangerous U6 NSException sources at the root, on
top of the RNTP_SAFE_CALL outer-layer catch from the previous commit.
facebook/react-native#50193
ghenry22
added a commit
to ghenry22/substreamer
that referenced
this pull request
Apr 10, 2026
Three Expo modules had stray force-unwraps that could in theory raise NSExceptions on the JS bridge call stack: - expo-ssl-trust SslTrustStore.swift:190 — `data(using: .utf8)!` on a hex ASCII fingerprint. Encoding can't fail in practice but the bang is removed defensively. - expo-ssl-trust SslTrustURLProtocol.swift:38 — `mutableCopy() as! NSMutableURLRequest`. URLProtocol contract guarantees the cast, but a misbehaving bridged subclass could crash the URL loading thread. Now `as?` + didFailWithError fallback. - expo-gzip ExpoGzipModule.swift:62, 99, 104 — three `baseAddress!` unwraps inside withUnsafeBytes closures. Each is guarded by an outer `!data.isEmpty` check, but the unwraps are now replaced with proper guards that surface as Z_ERRNO / GzipError.decompressionFailed. Defense-in-depth pass on the LOW/MEDIUM raise sites identified in the U6 audit. facebook/react-native#50193
ghenry22
added a commit
to ghenry22/substreamer
that referenced
this pull request
Apr 10, 2026
MusicModule.scope now installs a CoroutineExceptionHandler so any throwable that escapes a coroutine block gets logged via Timber instead of crashing the process via the global handler. launchInScope additionally try/catches each block and routes the throwable to the caller's Promise so JS calls don't hang on a never-settled promise. Hygiene fixes for the same crash class: - AudioItem.fromMediaItem returns nullable; QueuedAudioPlayer uses mapNotNull. - BaseAudioPlayer audio focus path drops focusRequest!! in favour of a local. - Cache.initCache replaces double-checked instance!! with a smart-cast pattern. Sibling of facebook/react-native#50193 on the Android side.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
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
Differential Revision: D71619229