fix(sdk): catch storage errors in getAnonymousId and getSessionId - #641
fix(sdk): catch storage errors in getAnonymousId and getSessionId#641r69shabh wants to merge 3 commits into
Conversation
|
@r69shabh is attempting to deploy a commit to the Databuddy OSS Team on Vercel. A member of the Team first needs to authorize it. |
|
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 |
Greptile SummaryThe PR makes tracking-ID helpers return null when browser storage access throws while preserving URL-parameter precedence.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
Reviews (2): Last reviewed commit: "chore: add changeset for sdk storage err..." | Re-trigger Greptile |
| Storage.prototype.getItem = function (key: string) { | ||
| if (this === localStorage) { | ||
| throw new DOMException("Access denied", "SecurityError"); | ||
| } | ||
| return original.call(this, key); | ||
| }; |
There was a problem hiding this comment.
Function expressions violate lint rules
The new storage mocks use traditional function expressions here and at the other Storage.prototype.getItem assignments, violating the repository's arrow-function requirement and potentially failing the corresponding lint check.
Context Used: Ultracite Rules - AI-Ready Formatter and Linter (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Fixes #638
What
getAnonymousId()andgetSessionId()both accesslocalStorageandsessionStoragewith no error handling. Web Storage can throw aDOMException(usuallySecurityError) when storage is blocked bybrowser privacy settings, sandboxed iframes, or Safari ITP — and when
that happens the exception propagates up through
getTrackingIds()andgetTrackingParams()to every caller.Reproduced it with the snippet from the issue — overriding
Storage.prototype.getItemto throw and callinggetAnonymousId()confirms it throws instead of returning null.
Why it matters
Two real callsites get hurt by this. The contact form calls
getTrackingIds()before entering its try/catch, so a storage errorprevents the form from submitting entirely — not just drops the tracking
IDs. The Stripe metadata helper has no error handling at all.
How
While reading through the codebase I noticed
getProfileId()alreadyhandles this — it wraps
localStorage.getItem()in a try/catch andreturns null on failure. The fix for
getAnonymousIdandgetSessionIdis the exact same pattern, just wasn't applied consistently.
Also made the URL-param short-circuit an explicit
ifguard so when aparam is present, storage is never touched at all (not just skipped via
JS truthiness). Matches what the issue asked for.
getTrackingIdsandgetTrackingParamsare unchanged — they justdelegate and get the fix for free.
Added 8 Playwright tests covering: each helper returning null when its
storage throws, partial results when only one storage fails, empty string
from
getTrackingParams, and URL param bypassing storage entirely.Tests
AI disclosure: I used Claude to help with research and writing. I
reproduced the issue myself, traced the callsites, and spotted the
getProfileIdpattern in the codebase as the solution. I sketched theplan and rough pseudocode for the fix and tests — Claude helped complete
the implementation and fill in the test cases from that. I reviewed all
the generated code, ran type-check and the full E2E suite locally before
opening this PR.
Summary by cubic
Prevents Web Storage errors from breaking tracking ID lookups in
@databuddy/sdk. Previously,getAnonymousId/getSessionIdthrew when storage was blocked; now they return null, and URL params short-circuit without accessing storage so flows like form submissions keep working.getProfileIdtry/catch pattern;getTrackingIds/getTrackingParamsare unchanged and inherit the safer behavior.Written for commit a713e06. Summary will update on new commits.