Skip to content

fix(sdk): respect the 30-minute session expiry in getSessionId - #695

Open
r69shabh wants to merge 1 commit into
databuddy-analytics:stagingfrom
r69shabh:fix/sdk-session-id-expiry
Open

fix(sdk): respect the 30-minute session expiry in getSessionId#695
r69shabh wants to merge 1 commit into
databuddy-analytics:stagingfrom
r69shabh:fix/sdk-session-id-expiry

Conversation

@r69shabh

@r69shabh r69shabh commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Fixes #694

What

getSessionId() reads did_session from sessionStorage without checking did_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() and getTrackingParams() 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 returns null if it's missing or older than 30 minutes — the same rule the tracker uses (sessionAge < 30 * 60 * 1000 in getOrCreateSessionId()). The window is a named constant with a comment pointing at the tracker logic. URL-param priority is untouched, and getTrackingIds/getTrackingParams are unchanged — they get the behavior for free.

Tests: the existing did_session setups 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

109 passed (sdk e2e suite, chromium)
7 getSessionId-specific tests passing
tsc --noEmit: 0 errors
bun run test (monorepo): 27/27 tasks passed

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 returns null when the stored session has no did_session_timestamp or when the timestamp is older than 30 minutes, matching the tracker's expiry rule. getTrackingIds() and getTrackingParams() inherit the fix since they delegate to getSessionId(), and URL params still take priority.

Written for commit 5e753a2. Summary will update on new commits.

Review in cubic

@r69shabh
r69shabh requested a review from izadoesdev as a code owner August 31, 2026 19:49
@coderabbitai

coderabbitai Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: e2a14d69-52e9-48e2-b2c1-ff340628141e

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@vercel

vercel Bot commented Aug 31, 2026

Copy link
Copy Markdown

@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-apps

greptile-apps Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR updates the browser SDK’s session helpers to honor the tracker’s 30-minute inactivity window.

  • Validates the stored session timestamp before returning a session ID.
  • Preserves URL-parameter priority and propagates the corrected behavior through tracking ID and query-parameter helpers.
  • Adds coverage for fresh, missing, expired, and exact-boundary timestamps.
  • Adds a patch changeset documenting the behavior change.

Confidence Score: 5/5

The 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

Filename Overview
packages/sdk/src/core/tracker.ts Adds timestamp-aware session expiration matching the tracker’s strict 30-minute inactivity boundary.
packages/sdk/tests/sdk-functions.spec.ts Updates fixtures with fresh timestamps and tests missing, expired, and boundary timestamp behavior.
.changeset/sdk-session-id-expiry.md Accurately documents the patch-level SDK behavior change.

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]
Loading

Reviews (1): Last reviewed commit: "fix(sdk): respect the 30-minute session ..." | Re-trigger Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant