Skip to content

Guard referral storage access - #126

Merged
ralyodio merged 1 commit into
profullstack:masterfrom
absalonCRC:fix-referral-storage-guard
May 23, 2026
Merged

Guard referral storage access#126
ralyodio merged 1 commit into
profullstack:masterfrom
absalonCRC:fix-referral-storage-guard

Conversation

@absalonCRC

@absalonCRC absalonCRC commented May 21, 2026

Copy link
Copy Markdown
Contributor

Summary

  • guard referral localStorage reads, writes, and cleanup so blocked storage does not break invite signup flows
  • add regression coverage for normal referral storage, unavailable storage, and write-rejecting storage

Fixes #125

Validation

  • pnpm test:run src/components/referral/ReferralTracker.test.tsx
  • pnpm type-check
  • pnpm exec eslint src/components/referral/ReferralTracker.tsx src/components/referral/ReferralTracker.test.tsx
  • git diff --check

Bounty / payment

Submitted for the active uGig affiliate-program testing task. SOL receive address: 27sdMYXofqoM9qR13bZhccRNYeEgYn5EoHXTSJn4QWKP.

Payment fallback: PayPal cultofrozen@gmail.com

@greptile-apps

greptile-apps Bot commented May 23, 2026

Copy link
Copy Markdown

Greptile Summary

This PR guards all localStorage access in ReferralTracker so that blocked or write-rejecting storage (common in private-browsing and strict cookie modes) no longer breaks invite signup flows. A new getLocalStorage() helper centralises the SSR and access-throw guard, and a second try/catch around setItem handles QuotaExceededError-style rejections.

  • ReferralTracker.tsx: Replaces bare localStorage.* calls with a null-safe helper and individual try/catch blocks for each operation (setItem, getItem, removeItem).
  • ReferralTracker.test.tsx: Adds four cases — normal storage, unavailable storage (getter throws), and write-rejecting storage — using a Map-backed Storage mock that replaces window.localStorage per test.

Confidence Score: 4/5

Safe to merge — the production guard is correct and handles all documented failure modes without altering the happy path.

The implementation is sound and the happy path is unchanged. The two open points are minor: the outer try/catch in getStoredReferral is unreachable dead code, and the "does not throw" test cases don't await effects so they may not exercise the guard inside useEffect if effects flush asynchronously in this environment.

src/components/referral/ReferralTracker.test.tsx — the synchronous "does not throw" assertions may not be verifying the useEffect guard.

Important Files Changed

Filename Overview
src/components/referral/ReferralTracker.tsx Adds getLocalStorage() helper with try/catch for blocked storage, wraps setItem in a second try/catch for write-rejecting storage, and mirrors the same guards in getStoredReferral and clearStoredReferral.
src/components/referral/ReferralTracker.test.tsx New test file covering normal storage, unavailable storage, and write-rejecting storage; "does not throw" cases are synchronous and don't await effects, which may not fully exercise the guard inside the useEffect callback.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[useEffect fires / getStoredReferral / clearStoredReferral] --> B[getLocalStorage]
    B --> C{typeof window === 'undefined'?}
    C -- Yes --> D[return null]
    C -- No --> E[try: return window.localStorage]
    E -- throws SecurityError --> F[catch: return null]
    E -- ok --> G[Storage object]
    G --> H{caller operation}
    D --> I[null?.op = no-op]
    F --> I
    H -- setItem --> J[try: storage?.setItem]
    J -- throws QuotaExceededError --> K[catch: silently ignore]
    J -- ok --> L[value stored]
    H -- getItem --> M[storage?.getItem ?? null]
    H -- removeItem --> N[try: storage?.removeItem]
    N -- throws --> O[catch: silently ignore]
Loading

Reviews (1): Last reviewed commit: "Guard referral tracking storage access" | Re-trigger Greptile

Comment on lines +71 to +83
it("does not throw when localStorage is unavailable", () => {
Object.defineProperty(window, "localStorage", {
configurable: true,
get() {
throw new DOMException("Storage is blocked", "SecurityError");
},
});
searchParams = new URLSearchParams("ref=invite-789");

expect(() => render(<ReferralTracker />)).not.toThrow();
expect(getStoredReferral()).toBeNull();
expect(() => clearStoredReferral()).not.toThrow();
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Effect guard not awaited in "does not throw" tests

Both "does not throw" cases are synchronous and don't await the useEffect. RTL's render() wraps in a synchronous act(), but the first test in this suite still uses await waitFor() to observe the effect's side effect — which suggests effects aren't reliably flushed by synchronous render() here. If the effect is scheduled after render() returns, the not.toThrow() assertion passes regardless of whether the guard fires, meaning a future regression that removes the guard could go undetected by these tests. Making the tests async and adding an await act(async () => { render(...) }) (or checking via waitFor) would ensure the effect actually runs before the assertion.

Comment on lines 41 to 47
export function getStoredReferral(): string | null {
if (typeof window === "undefined") return null;
return localStorage.getItem(REFERRAL_KEY);
try {
return getLocalStorage()?.getItem(REFERRAL_KEY) ?? null;
} catch {
return null;
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Outer try/catch in getStoredReferral is unreachable

getLocalStorage() already catches any access-time throw and returns null. The optional-chaining call null?.getItem(...) produces undefined without throwing, and ?? null converts that to null. The outer try/catch block can therefore never be triggered by the getLocalStorage() path. It would only fire if getItem() itself throws, which isn't a documented browser behaviour. Consider removing it to keep the error-handling surface consistent with clearStoredReferral, or add a comment explaining the specific edge case it guards.

@ralyodio
ralyodio merged commit 469cd8b into profullstack:master May 23, 2026
4 checks passed
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.

bug: referral invite flow crashes when localStorage is unavailable

2 participants