fix(sdk): respect the 30-minute session expiry in getSessionId - #695
fix(sdk): respect the 30-minute session expiry in getSessionId#695r69shabh wants to merge 1 commit into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@r69shabh is attempting to deploy a commit to the Databuddy OSS Team on Vercel. A member of the Team first needs to authorize it. |
Greptile SummaryThe PR updates the browser SDK’s session helpers to honor the tracker’s 30-minute inactivity window.
Confidence Score: 5/5The PR appears safe to merge with no actionable correctness or security issues identified. The SDK now applies the same timestamp key, parsing approach, timeout duration, and strict expiration boundary as the tracker, while preserving URL-parameter priority and covering the relevant boundary cases. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[getSessionId called] --> B{Session ID in URL params?}
B -- Yes --> C[Return URL session ID]
B -- No --> D[Read stored session ID and timestamp]
D --> E{ID exists and timestamp is finite?}
E -- No --> F[Return null]
E -- Yes --> G{Age is under 30 minutes?}
G -- No --> F
G -- Yes --> H[Return stored session ID]
Reviews (1): Last reviewed commit: "fix(sdk): respect the 30-minute session ..." | Re-trigger Greptile |
Fixes #694
What
getSessionId()readsdid_sessionfrom sessionStorage without checkingdid_session_timestamp. The tracker rotates sessions after 30 minutes of inactivity, so between expiry and the tracker's next visit the SDK hands back a session id that is no longer the current one.getTrackingIds()andgetTrackingParams()inherit the problem since they delegate to it.Why it matters
getTrackingParams()exists to carry the ids to another origin for cross-domain tracking. If the session has actually expired, propagating the stale id links events to a session the tracker has already closed — anyone keying server-side joins on these ids gets subtly wrong data.How
getSessionId()now reads the timestamp alongside the id and returnsnullif it's missing or older than 30 minutes — the same rule the tracker uses (sessionAge < 30 * 60 * 1000ingetOrCreateSessionId()). The window is a named constant with a comment pointing at the tracker logic. URL-param priority is untouched, andgetTrackingIds/getTrackingParamsare unchanged — they get the behavior for free.Tests: the existing
did_sessionsetups now write a fresh timestamp, plus new cases for no-timestamp → null, older than 30 minutes → null, and exactly at the boundary → expired (matching the tracker's strict<check).Tests
AI disclosure: I used Claude (via Qoder) for research and writing. I found the mismatch myself while comparing the tracker's expiry logic against the SDK helper, reproduced it in the e2e suite, and sketched the fix — Claude helped complete the implementation and fill in the test cases from that. I reviewed all the generated code and ran type-check and the full e2e suite locally; the pre-push hook ran the monorepo suite clean.
Summary by cubic
Fixes #694:
getSessionId()no longer returns a session id that the tracker has already rotated. It now returnsnullwhen the stored session has nodid_session_timestampor when the timestamp is older than 30 minutes, matching the tracker's expiry rule.getTrackingIds()andgetTrackingParams()inherit the fix since they delegate togetSessionId(), and URL params still take priority.Written for commit 5e753a2. Summary will update on new commits.