-
Notifications
You must be signed in to change notification settings - Fork 58
fix(session-replay-react-native): prevent interacting with the view hierarchy from a background thread #1425
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
@aliaksandr-kazarez are you available to look at this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes a critical production crash on iOS (NSInternalInconsistencyException) that occurs when the session replay SDK attempts to interact with the view hierarchy from a background thread. The solution wraps UI-related operations in DispatchQueue.main.async to ensure they execute on the main thread.
Key Changes:
- Added main thread dispatch for
sessionReplay.start()whenautoStartis enabled in thesetup()method - Wrapped
sessionReplay.start()execution in thestart()method withDispatchQueue.main.async - Wrapped
sessionReplay.stop()execution in both thestop()andteardown()methods withDispatchQueue.main.async
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| DispatchQueue.main.async { | ||
| self.sessionReplay.start() | ||
| resolve(nil) | ||
| } |
Copilot
AI
Dec 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent promise resolution pattern: In this method, resolve(nil) is called inside the DispatchQueue.main.async block (line 72), but in the setup() method when autoStart is true (lines 41-43), the resolve(nil) is called outside the async block (line 45).
For consistency, either:
- Move
resolve(nil)outside the async block here to matchsetup()(resolves immediately, fire-and-forget pattern) - Or move the
resolve(nil)insetup()inside its async block (waits for completion)
The choice depends on whether JavaScript callers need to wait for the operation to complete on the main thread before the promise resolves.
| DispatchQueue.main.async { | ||
| self.sessionReplay.stop() | ||
| resolve(nil) | ||
| } |
Copilot
AI
Dec 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent promise resolution pattern: In this method, resolve(nil) is called inside the DispatchQueue.main.async block (line 81), but in the setup() method when autoStart is true (lines 41-43), the resolve(nil) is called outside the async block (line 45).
For consistency, consider using the same pattern as in setup() where resolve(nil) is called immediately after dispatching the async work, unless you specifically need to wait for the stop operation to complete on the main thread before resolving the promise.
| resolve(nil) | ||
| } |
Copilot
AI
Dec 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent promise resolution pattern: In this method, resolve(nil) is called inside the DispatchQueue.main.async block (line 97), but in the setup() method when autoStart is true (lines 41-43), the resolve(nil) is called outside the async block (line 45).
For consistency, consider using the same pattern as in setup() where resolve(nil) is called immediately after dispatching the async work, unless you specifically need to wait for the stop operation to complete on the main thread before resolving the promise.
| resolve(nil) | |
| } | |
| } | |
| resolve(nil) |
Summary
This PR aims to resolve a production crash on iOS (
NSInternalInconsistencyException). The crash occurs when the SDK attempts to interact with the view hierarchy from a background thread.We observed intermittent but fatal crashes on iOS with the following stack trace:
Root Cause
The crash is an
NSInternalInconsistencyExceptionoriginating from[NSISEngine optimize], which is part of the Auto Layout engine. The stack trace shows thatAmplitudeSessionReplayis the caller. This exception is triggered because the Amplitude SDK attempts to access or modify UI layout information (to record the session) from a background worker thread (_dispatch_workloop_worker_thread).Accessing the main thread is strictly forbidden and unsafe in that kinda way, leading to race conditions and crashes.
In my config, I used the
autoStart: trueoption. Here is my config:Solution
Changes in PluginSessionReplayReactNative.swift:
The native
start(),stop(), andteardown()methods were modified to wrap their execution inDispatchQueue.main.async. This forces the execution of the SDK's recording logic to jump to the main thread before touching any UI or layout components.Checklist