-
Notifications
You must be signed in to change notification settings - Fork 38
fix: Enforce a 24-hour maximum session duration #494
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
Open
turnipdabeets
wants to merge
15
commits into
main
Choose a base branch
from
fix/use-date-provider-in-session-manager
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
c653034
fix: use PostHogDateProvider in PostHogSessionManager instead of Systβ¦
marandaneto f6f117b
fix: skip session rotation for React Native in lifecycle observer
marandaneto 09e64fc
chore: update PR description and changeset for 24h session limit
marandaneto 7de0557
chore: changeset to patch
marandaneto 30480c1
fix
marandaneto edf88a3
fix
marandaneto 54296e2
ref
marandaneto 879275e
fix: use setDateProvider() after dateProvider was made private
marandaneto a8c4635
fix: restart session replay after 24h rotation in background
turnipdabeets 28bc682
fix: rotate session in PostHogSessionManager getter after 24h
turnipdabeets 165e5f2
fix: prefer caller-provided $session_id over getter in buildProperties
turnipdabeets 516034c
test: cover caller-provided session_id; clean up session listener on β¦
turnipdabeets 5c777b5
fix: stamp sessionStartedAt in setSessionId; restart replay on rotation
turnipdabeets 7888e8a
fix: post replay restart to main thread; add test for rotation listener
turnipdabeets 68e3e9a
fix: rotate or clear session after 30 minutes of inactivity
turnipdabeets File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| --- | ||
| 'posthog': patch | ||
| 'posthog-android': patch | ||
| --- | ||
|
|
||
| Enforce 24-hour maximum session duration with automatic session rotation |
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -276,6 +276,8 @@ public class PostHogReplayIntegration( | |
| private val onTouchEventListener = | ||
| TouchEventInterceptor { motionEvent, dispatch -> | ||
| val timestamp = config.dateProvider.currentTimeMillis() | ||
| // User touch counts as activity (closest equivalent to iOS UIEvent swizzling). | ||
| PostHogSessionManager.touchSession() | ||
|
|
||
| try { | ||
| val state = dispatch(motionEvent) | ||
|
|
@@ -1662,7 +1664,8 @@ public class PostHogReplayIntegration( | |
|
|
||
| /** | ||
| * Called when the session ID changes. Stops recording if event triggers are configured | ||
| * and the new session hasn't been activated yet. | ||
| * and the new session hasn't been activated yet, or re-initializes recording so the | ||
| * new session gets fresh meta + full wireframe events. | ||
| */ | ||
| override fun onSessionIdChanged() { | ||
| val postHog = this.postHog ?: return | ||
|
|
@@ -1678,6 +1681,20 @@ public class PostHogReplayIntegration( | |
| config.logger.log("[Session Replay] Session changed. Stopping until trigger is matched.") | ||
| stop() | ||
| } | ||
| } else if (isSessionReplayActive) { | ||
| // Session rotated/cleared silently (e.g., 24h max duration via getter). | ||
| // Posting to main: getter can be invoked from any thread that calls capture(), | ||
| // and start(resumeCurrent = false) iterates a non-thread-safe WeakHashMap. | ||
| if (currentSessionId == null) { | ||
| config.logger.log("[Session Replay] Session cleared. Stopping recording.") | ||
| mainHandler.handler.post { stop() } | ||
| } else { | ||
| config.logger.log("[Session Replay] Session changed. Re-initializing recording for new session.") | ||
| mainHandler.handler.post { | ||
| stop() | ||
| start(resumeCurrent = false) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to the comment above, this will skip sampling check and start the session anyway? Maybe we can route through PostHog.startSessionReplay() which checks sampling? |
||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
||
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
Oops, something went wrong.
Oops, something went wrong.
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.
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.
I think we should react even if session replay is not active. Replay may be enabled in config but Session A may not have been sampled and not started. Now that we rotate to session B, sampling may return true and session replay should start?