Repository navigation
feat(ios): Report JS thread hangs as errors (iOS only) - #6964
Merged
Merged
Conversation
Contributor
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog.
🤖 This preview updates automatically when you update the PR. |
Contributor
Instructions and example for changelogPlease add an entry to Example: ## Unreleased
### Features
- Report JS thread hangs as errors (iOS only) ([#6964](https://github.com/getsentry/sentry-react-native/pull/6964))If none of the above apply, you can opt out of this check by adding |
alwx
force-pushed
the
alwx/feature/3460
branch
from
November 3, 2026 13:14
0f9e51b to
49a9e7b
Compare
alwx
marked this pull request as ready for review
November 4, 2026 07:55
alwx
requested review from
0xadam-brown,
antonis,
buenaflor,
itaybre,
philprime and
runningcode
and removed request for
a team
November 4, 2026 07:55
alwx
removed request for
0xadam-brown,
buenaflor,
itaybre,
lucas-zimerman,
philprime and
runningcode
November 4, 2026 08:00
alwx
force-pushed
the
alwx/feature/3460
branch
from
November 4, 2026 11:06
b65bad6 to
f8f9d1c
Compare
📲 Install BuildsAndroid
|
antonis
reviewed
Nov 4, 2026
antonis
reviewed
Nov 4, 2026
RNSentry runs on the main queue, so stop() blocked the UI thread while it waited for a running Hermes sample. The sample slept for its whole 300 ms window. When the wait timed out, a worker that still waited for the sampler lock could also turn the sampler on after stop() returned. The sample now waits on the tracker's wake-up semaphore instead of sleeping. stop() signals that semaphore, so a running sample ends at once and turns the sampler off. A sample that starts later sees the stored signal and ends at once too. The 1 s limit stays only as protection against a deadlock. invalidate now stops the tracker before it calls super, so the sampler is off before the module tears down.
Contributor
Author
|
About the new Warden findings:
|
antonis
reviewed
Nov 4, 2026
antonis
left a comment
Contributor
There was a problem hiding this comment.
Thank you for iterating on this @alwx 🙇
Did a few checks (example 1, example 2) and added a couple of comments. Overall the feature looks good.
Let's also add some links in the description
Collaborator
|
The PR description might be a bit outdated, it mentions the changelog will be updated on the next step but it was added on this PR |
A very short jsHangTimeoutInterval sent an event and took a 300 ms Hermes sample on every short stall. The integration now uses at least 0.25 seconds. A value that is not a finite number, like NaN, uses the default of 2 seconds. Both cases log a warning. The native check rejected only values <= 0, so NaN reached the uint64_t cast, which is undefined behavior. The check is now negated, so it rejects NaN too.
When a Hermes profile ran, the hang sample returned nothing, so the hang event had no stack. The sample app profiles every transaction, so this was the usual case. The tracker skipped the sample on purpose: Hermes clears its samples on every dump, so a dump would remove the samples that the profile collected before the hang. During a profile the sampler is already on and its newest samples show the hang. The tracker now dumps them at once and takes the stack from the newest JS sample. The sampler keeps the dumped traces and merges them into the profile in stopHermesProfiling. Every Hermes trace numbers its stack frames from 1, so the merge moves the frame ids and parents of each trace past the ids before it. Samples stay in time order, so the JS profile conversion does not change. A new profile drops the traces of the profile before it.
startJsHangTracking casts the timeout to uint64_t. A value below 1 ms became 0, so the tracker reported a hang for every ping that was still pending at the next poll. Infinity passed the check too, and casting it is undefined behavior, like NaN. The method now rejects values that are not finite or are below 1 ms. The JS integration already sends at least 250 ms, so this only guards the bridge.
Contributor
Author
|
About the Warden finding JS hang timeout accepts non-finite and sub-millisecond values: Fixed in the latest commits |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 2dd6b4d. Configure here.
A JS debugger that pauses at a breakpoint blocks the JS thread, so the tracker reported every pause longer than the timeout as a hang. The native check for an attached debugger sees only native tracers like lldb, not the Hermes inspector, and native code has no flag for a paused JS debugger. The integration now does not start the tracker when __DEV__ is true. sentry-android does the same for ANRs, which it does not report in debug builds by default. The option docs and the CHANGELOG entry say so.
stop() waited at most 1 s for the tracker thread and then returned, even when the thread still dumped the Hermes sampler. invalidate could then tear down the JS runtime while the sampler was in use, and a restart could start a new thread next to the old one. stop() now waits until the thread ends. This cannot deadlock: the thread never waits for the main thread or for the tracker lock, and the only lock it takes is the sampler mutex, which is held only for a dump. To keep the wait short, the thread now only takes the sample. Parsing the trace, building the event, and the capture run on a background queue. In a test with a stub Hermes, stop() during a sample window took 0.3 ms, and the event was still captured with its stack.
Contributor
Author
|
About the
|
antonis
approved these changes
Nov 5, 2026
antonis
left a comment
Contributor
There was a problem hiding this comment.
Thank you for iterating on this @alwx 🙇
LGTM 👍 Did a couple of tests locally and it worked as expected:
- 5 s block with profiling on: https://sentry-sdks.sentry.io/discover/sentry-react-native:61303b76973c48bbb209b1f371bd629d/
- infinite loop: https://sentry-sdks.sentry.io/discover/sentry-react-native:b71966e9db104539b952247be9033c56/
🔮 Next steps
Android is left out because reload crashes in the Hermes sampler thread (meta/hermes#1853), and the follow-up is #6985. Still to do for iOS: native unit tests, a symbolication check on sentry.io, and the sentry-docs page.
- Let's also update the PR description and add a link.
- I'm fine with iterating on a next PR with the missing unit tests.
7 of 11 tasks
3 tasks done
4 of 8 tasks
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
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.
📢 Type of change
📜 Description
Adds the experimental, iOS-only
enableJsHangTrackingandjsHangTimeoutIntervaloptions. When the JS thread does not run a watchdog task within the timeout, the SDK captures aJS Thread Hangerror event with the Hermes JS stack and the bundle Debug ID. The tracker does not run in__DEV__builds, and during a profile it reads the profile samples and puts them back into the profile.💡 Motivation and Context
App Hang tracking watches only the UI thread, so an infinite loop in JS sent no event. Closes #3460.
💚 How did you test it?
Before the review fixes, on the iOS 26.1 simulator (New Architecture), a 5 s block and an infinite loop each produced one event with the blocking function as the top frame, and the Release build resolved it to the right source line. The review fixes are covered by Jest tests and by a local test with a stub Hermes for the stack choice, the profile merge, and a
stopcall during a sample.After the review fixes, @antonis tested the latest commit:
📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps
Android is left out because reload crashes in the Hermes sampler thread (meta/hermes#1853), and the follow-up is #6985. Still to do for iOS: native unit tests in a follow-up PR, and the sentry-docs page.