From fec0e985a2e118f7da00ed3a20f5d1cbc2c55e62 Mon Sep 17 00:00:00 2001 From: Alberto Leal Date: Tue, 14 Apr 2026 16:08:10 -0400 Subject: [PATCH 1/4] fix(billing): Use localStorageWrapper instead of raw localStorage in navBillingStatus Replace 4 raw localStorage.getItem/setItem calls with localStorageWrapper from sentry/utils/localStorage. On devices where window.localStorage is null (e.g. Android WebView without DOM Storage enabled), raw access throws TypeError. localStorageWrapper safely falls back to a noopStorage that returns null for reads and no-ops for writes. Add unit and integration tests verifying the component renders, auto-opens, and allows dismiss interaction when localStorage is unavailable. Co-Authored-By: Claude Opus 4.6 --- .../components/navBillingStatus.spec.tsx | 140 ++++++++++++++++++ static/gsApp/components/navBillingStatus.tsx | 9 +- 2 files changed, 145 insertions(+), 4 deletions(-) diff --git a/static/gsApp/components/navBillingStatus.spec.tsx b/static/gsApp/components/navBillingStatus.spec.tsx index cb8c858a2265c9..b136e649f3475c 100644 --- a/static/gsApp/components/navBillingStatus.spec.tsx +++ b/static/gsApp/components/navBillingStatus.spec.tsx @@ -8,6 +8,29 @@ import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary'; import {resetMockDate, setMockDate} from 'sentry-test/utils'; import {DataCategory} from 'sentry/types/core'; +import {createStorage} from 'sentry/utils/createStorage'; + +jest.mock('sentry/utils/localStorage', () => { + const actual = jest.requireActual( + 'sentry/utils/localStorage' + ); + return { + ...actual, + localStorageWrapper: { + ...actual.localStorageWrapper, + getItem: jest.fn((...args: Parameters) => + actual.localStorageWrapper.getItem(...args) + ), + setItem: jest.fn((...args: Parameters) => + actual.localStorageWrapper.setItem(...args) + ), + }, + }; +}); + +const {localStorageWrapper} = jest.requireMock< + typeof import('sentry/utils/localStorage') +>('sentry/utils/localStorage'); import {PrimaryNavigationQuotaExceeded} from 'getsentry/components/navBillingStatus'; import {SubscriptionStore} from 'getsentry/stores/subscriptionStore'; @@ -657,4 +680,121 @@ describe('PrimaryNavigationQuotaExceeded', () => { ).toBeInTheDocument(); }); }); + + describe('localStorage unavailable', () => { + beforeEach(() => { + // Simulate noopStorage fallback: getItem returns null, setItem is a no-op + jest.mocked(localStorageWrapper.getItem).mockReturnValue(null); + jest.mocked(localStorageWrapper.setItem).mockImplementation(() => undefined); + }); + + it('should render without crashing when localStorage is unavailable', async () => { + render(); + + expect( + await screen.findByRole('button', {name: 'Billing Status'}) + ).toBeInTheDocument(); + }); + + it('should auto-open the alert when localStorage is unavailable', async () => { + render(); + + // With getItem returning null, the component should auto-open + // because currentCategories !== lastShownCategories (null) + expect(await screen.findByText('Quotas Exceeded')).toBeInTheDocument(); + }); + + it('should handle setItem gracefully when localStorage is unavailable', async () => { + render(); + + // The component auto-opens and calls setItem to persist state. + // With noopStorage, setItem is a no-op — verify it was called without throwing. + expect(await screen.findByText('Quotas Exceeded')).toBeInTheDocument(); + expect(localStorageWrapper.setItem).toHaveBeenCalled(); + }); + }); + + describe('localStorage unavailable (integration)', () => { + // Integration tests that verify the full createStorage → noopStorage → component + // pipeline, covering the production crash on Sony BRAVIA 4K TV (Chrome Mobile + // WebView 90, Android 10) where window.localStorage is null. + + it('createStorage returns a safe noopStorage when window.localStorage is null', () => { + const originalLocalStorage = window.localStorage; + Object.defineProperty(window, 'localStorage', { + value: null, + configurable: true, + }); + try { + const storage = createStorage(() => window.localStorage); + + // noopStorage should be safe to call without throwing + expect(storage.getItem('any-key')).toBeNull(); + expect(() => storage.setItem('key', 'value')).not.toThrow(); + expect(() => storage.removeItem('key')).not.toThrow(); + expect(() => storage.clear()).not.toThrow(); + expect(storage).toHaveLength(0); + expect(storage.key(0)).toBeNull(); + } finally { + Object.defineProperty(window, 'localStorage', { + value: originalLocalStorage, + configurable: true, + }); + } + }); + + it('createStorage returns a safe noopStorage when storage.setItem throws', () => { + // Simulates environments where localStorage exists but is disabled (e.g. quota exceeded, private browsing) + const brokenStorage = { + ...window.localStorage, + setItem() { + throw new DOMException('The quota has been exceeded.'); + }, + } as Storage; + + const storage = createStorage(() => brokenStorage); + + expect(storage.getItem('any-key')).toBeNull(); + expect(() => storage.setItem('key', 'value')).not.toThrow(); + expect(storage).toHaveLength(0); + }); + + it('component allows dismiss interaction when localStorage is unavailable', async () => { + // Simulate noopStorage: getItem returns null, setItem is a no-op + jest.mocked(localStorageWrapper.getItem).mockReturnValue(null); + jest.mocked(localStorageWrapper.setItem).mockImplementation(() => undefined); + + render(); + + // Component auto-opens since getItem returns null (categories mismatch) + expect(await screen.findByText('Quotas Exceeded')).toBeInTheDocument(); + + // User should be able to dismiss the alert without crashing + await userEvent.click( + screen.getByRole('button', { + name: 'Dismiss alert for the rest of the billing cycle', + }) + ); + + // Verify dismiss triggers prompt snooze API calls (one per exceeded category) + expect(promptMock).toHaveBeenCalledTimes(4); + }); + + it('component reads and writes correct localStorage keys when storage is available', async () => { + // Verify the component uses localStorageWrapper (not raw localStorage) for reads/writes + render(); + + expect( + await screen.findByRole('button', {name: 'Billing Status'}) + ).toBeInTheDocument(); + + // Verify localStorageWrapper.getItem was called with expected keys + expect(localStorageWrapper.getItem).toHaveBeenCalledWith( + `billing-status-last-shown-categories-${organization.id}` + ); + expect(localStorageWrapper.getItem).toHaveBeenCalledWith( + `billing-status-last-shown-date-${organization.id}` + ); + }); + }); }); diff --git a/static/gsApp/components/navBillingStatus.tsx b/static/gsApp/components/navBillingStatus.tsx index b7436ee79a5957..60934b10a11dce 100644 --- a/static/gsApp/components/navBillingStatus.tsx +++ b/static/gsApp/components/navBillingStatus.tsx @@ -14,6 +14,7 @@ import {t, tct} from 'sentry/locale'; import {DataCategory} from 'sentry/types/core'; import type {Organization} from 'sentry/types/organization'; import {getDaysSinceDate} from 'sentry/utils/getDaysSinceDate'; +import {localStorageWrapper} from 'sentry/utils/localStorage'; import { PrimaryNavigation, usePrimaryNavigationButtonOverlay, @@ -443,10 +444,10 @@ export function PrimaryNavigationQuotaExceeded({ // either it has been more than a day since the last shown date, // the categories have changed, or // the last time it was shown was before the current usage cycle started - const lastShownCategories = localStorage.getItem( + const lastShownCategories = localStorageWrapper.getItem( `billing-status-last-shown-categories-${organization.id}` ); - const lastShownDate = localStorage.getItem( + const lastShownDate = localStorageWrapper.getItem( `billing-status-last-shown-date-${organization.id}` ); const daysSinceLastShown = lastShownDate ? getDaysSinceDate(lastShownDate) : 0; @@ -463,11 +464,11 @@ export function PrimaryNavigationQuotaExceeded({ ) { hasAutoOpenedAlertRef.current = true; overlayState.open(); - localStorage.setItem( + localStorageWrapper.setItem( `billing-status-last-shown-categories-${organization.id}`, currentCategories ); - localStorage.setItem( + localStorageWrapper.setItem( `billing-status-last-shown-date-${organization.id}`, moment().utc().toISOString() ); From fafd11fdf17ec210d8d029892e5246fba80b14a6 Mon Sep 17 00:00:00 2001 From: Alberto Leal Date: Tue, 14 Apr 2026 16:13:30 -0400 Subject: [PATCH 2/4] ref(billing): Simplify localStorage tests and fix isolation bug MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Consolidate 3 redundant unit tests into 1 focused test that covers render, auto-open, and setItem gracefully. Add afterEach with jest.restoreAllMocks() to prevent mockReturnValue leaking across describe blocks (clearMocks only resets call history, not implementations). Move createStorage utility tests to a dedicated createStorage.spec.tsx file — they test shared utility behavior, not component behavior. Remove implementation-detail test that only verified call arguments. Co-Authored-By: Claude Opus 4.6 --- static/app/utils/createStorage.spec.tsx | 38 ++++++++ .../components/navBillingStatus.spec.tsx | 97 ++----------------- 2 files changed, 46 insertions(+), 89 deletions(-) create mode 100644 static/app/utils/createStorage.spec.tsx diff --git a/static/app/utils/createStorage.spec.tsx b/static/app/utils/createStorage.spec.tsx new file mode 100644 index 00000000000000..aa12e89ab60d9c --- /dev/null +++ b/static/app/utils/createStorage.spec.tsx @@ -0,0 +1,38 @@ +import {createStorage} from 'sentry/utils/createStorage'; + +describe('createStorage', () => { + it('returns noopStorage when underlying storage is null', () => { + const storage = createStorage(() => null as unknown as Storage); + + expect(storage.getItem('any-key')).toBeNull(); + expect(() => storage.setItem('key', 'value')).not.toThrow(); + expect(() => storage.removeItem('key')).not.toThrow(); + expect(() => storage.clear()).not.toThrow(); + expect(storage).toHaveLength(0); + expect(storage.key(0)).toBeNull(); + }); + + it('returns noopStorage when storage.setItem throws', () => { + const brokenStorage = { + ...window.localStorage, + setItem() { + throw new DOMException('The quota has been exceeded.'); + }, + } as Storage; + + const storage = createStorage(() => brokenStorage); + + expect(storage.getItem('any-key')).toBeNull(); + expect(() => storage.setItem('key', 'value')).not.toThrow(); + expect(storage).toHaveLength(0); + }); + + it('returns real storage when it works correctly', () => { + const storage = createStorage(() => window.localStorage); + + storage.setItem('test-key', 'test-value'); + expect(storage.getItem('test-key')).toBe('test-value'); + storage.removeItem('test-key'); + expect(storage.getItem('test-key')).toBeNull(); + }); +}); diff --git a/static/gsApp/components/navBillingStatus.spec.tsx b/static/gsApp/components/navBillingStatus.spec.tsx index b136e649f3475c..2ab4696f45301b 100644 --- a/static/gsApp/components/navBillingStatus.spec.tsx +++ b/static/gsApp/components/navBillingStatus.spec.tsx @@ -8,7 +8,6 @@ import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary'; import {resetMockDate, setMockDate} from 'sentry-test/utils'; import {DataCategory} from 'sentry/types/core'; -import {createStorage} from 'sentry/utils/createStorage'; jest.mock('sentry/utils/localStorage', () => { const actual = jest.requireActual( @@ -683,118 +682,38 @@ describe('PrimaryNavigationQuotaExceeded', () => { describe('localStorage unavailable', () => { beforeEach(() => { - // Simulate noopStorage fallback: getItem returns null, setItem is a no-op jest.mocked(localStorageWrapper.getItem).mockReturnValue(null); jest.mocked(localStorageWrapper.setItem).mockImplementation(() => undefined); }); - it('should render without crashing when localStorage is unavailable', async () => { + afterEach(() => { + jest.restoreAllMocks(); + }); + + it('should render and auto-open without crashing when localStorage is unavailable', async () => { render(); expect( await screen.findByRole('button', {name: 'Billing Status'}) ).toBeInTheDocument(); - }); - - it('should auto-open the alert when localStorage is unavailable', async () => { - render(); - - // With getItem returning null, the component should auto-open - // because currentCategories !== lastShownCategories (null) - expect(await screen.findByText('Quotas Exceeded')).toBeInTheDocument(); - }); - - it('should handle setItem gracefully when localStorage is unavailable', async () => { - render(); - - // The component auto-opens and calls setItem to persist state. - // With noopStorage, setItem is a no-op — verify it was called without throwing. + // Auto-opens because currentCategories !== lastShownCategories (null) expect(await screen.findByText('Quotas Exceeded')).toBeInTheDocument(); + // setItem is a no-op but should not throw expect(localStorageWrapper.setItem).toHaveBeenCalled(); }); - }); - - describe('localStorage unavailable (integration)', () => { - // Integration tests that verify the full createStorage → noopStorage → component - // pipeline, covering the production crash on Sony BRAVIA 4K TV (Chrome Mobile - // WebView 90, Android 10) where window.localStorage is null. - - it('createStorage returns a safe noopStorage when window.localStorage is null', () => { - const originalLocalStorage = window.localStorage; - Object.defineProperty(window, 'localStorage', { - value: null, - configurable: true, - }); - try { - const storage = createStorage(() => window.localStorage); - - // noopStorage should be safe to call without throwing - expect(storage.getItem('any-key')).toBeNull(); - expect(() => storage.setItem('key', 'value')).not.toThrow(); - expect(() => storage.removeItem('key')).not.toThrow(); - expect(() => storage.clear()).not.toThrow(); - expect(storage).toHaveLength(0); - expect(storage.key(0)).toBeNull(); - } finally { - Object.defineProperty(window, 'localStorage', { - value: originalLocalStorage, - configurable: true, - }); - } - }); - - it('createStorage returns a safe noopStorage when storage.setItem throws', () => { - // Simulates environments where localStorage exists but is disabled (e.g. quota exceeded, private browsing) - const brokenStorage = { - ...window.localStorage, - setItem() { - throw new DOMException('The quota has been exceeded.'); - }, - } as Storage; - - const storage = createStorage(() => brokenStorage); - - expect(storage.getItem('any-key')).toBeNull(); - expect(() => storage.setItem('key', 'value')).not.toThrow(); - expect(storage).toHaveLength(0); - }); - - it('component allows dismiss interaction when localStorage is unavailable', async () => { - // Simulate noopStorage: getItem returns null, setItem is a no-op - jest.mocked(localStorageWrapper.getItem).mockReturnValue(null); - jest.mocked(localStorageWrapper.setItem).mockImplementation(() => undefined); + it('should allow dismiss interaction when localStorage is unavailable', async () => { render(); - // Component auto-opens since getItem returns null (categories mismatch) expect(await screen.findByText('Quotas Exceeded')).toBeInTheDocument(); - // User should be able to dismiss the alert without crashing await userEvent.click( screen.getByRole('button', { name: 'Dismiss alert for the rest of the billing cycle', }) ); - // Verify dismiss triggers prompt snooze API calls (one per exceeded category) expect(promptMock).toHaveBeenCalledTimes(4); }); - - it('component reads and writes correct localStorage keys when storage is available', async () => { - // Verify the component uses localStorageWrapper (not raw localStorage) for reads/writes - render(); - - expect( - await screen.findByRole('button', {name: 'Billing Status'}) - ).toBeInTheDocument(); - - // Verify localStorageWrapper.getItem was called with expected keys - expect(localStorageWrapper.getItem).toHaveBeenCalledWith( - `billing-status-last-shown-categories-${organization.id}` - ); - expect(localStorageWrapper.getItem).toHaveBeenCalledWith( - `billing-status-last-shown-date-${organization.id}` - ); - }); }); }); From 7b6bc24033ceb01e5ea7eeeb9fd752d6c89459d2 Mon Sep 17 00:00:00 2001 From: Alberto Leal Date: Tue, 14 Apr 2026 16:22:45 -0400 Subject: [PATCH 3/4] fix(billing): Use localStorageWrapper instead of raw localStorage in trialStarter Replace raw localStorage.removeItem call with localStorageWrapper.removeItem. On devices where window.localStorage is null (e.g. Android WebView without DOM Storage enabled), raw access throws TypeError. localStorageWrapper safely falls back to noopStorage. Add test verifying trial start succeeds when localStorage is unavailable. Co-Authored-By: Claude Opus 4.6 --- static/gsApp/components/trialStarter.spec.tsx | 71 +++++++++++++++++++ static/gsApp/components/trialStarter.tsx | 3 +- 2 files changed, 73 insertions(+), 1 deletion(-) diff --git a/static/gsApp/components/trialStarter.spec.tsx b/static/gsApp/components/trialStarter.spec.tsx index 9a03719987913b..0f8362f96e2fc4 100644 --- a/static/gsApp/components/trialStarter.spec.tsx +++ b/static/gsApp/components/trialStarter.spec.tsx @@ -6,6 +6,25 @@ import {TeamFixture} from 'sentry-fixture/team'; import {SubscriptionFixture} from 'getsentry-test/fixtures/subscription'; import {act, render, screen} from 'sentry-test/reactTestingLibrary'; +jest.mock('sentry/utils/localStorage', () => { + const actual = jest.requireActual( + 'sentry/utils/localStorage' + ); + return { + ...actual, + localStorageWrapper: { + ...actual.localStorageWrapper, + removeItem: jest.fn((...args: Parameters) => + actual.localStorageWrapper.removeItem(...args) + ), + }, + }; +}); + +const {localStorageWrapper} = jest.requireMock< + typeof import('sentry/utils/localStorage') +>('sentry/utils/localStorage'); + import TrialStarter from 'getsentry/components/trialStarter'; import {SubscriptionStore} from 'getsentry/stores/subscriptionStore'; @@ -106,4 +125,56 @@ describe('TrialStarter', () => { expect(startedCall.trialStarted).toBe(false); expect(startedCall.trialFailed).toBe(true); }); + + it('starts a trial successfully when localStorage is unavailable', async () => { + // Simulate noopStorage: removeItem is a no-op (e.g., Android WebView without DOM Storage) + jest.mocked(localStorageWrapper.removeItem).mockImplementation(() => null); + + const handleTrialStarted = jest.fn(); + // eslint-disable-next-line no-empty-pattern + const renderer = jest.fn(({}: RendererProps) =>
render text
); + MockApiClient.addMockResponse({ + url: `/organizations/${org.slug}/`, + body: org, + }); + MockApiClient.addMockResponse({ + url: `/organizations/${org.slug}/projects/`, + body: [ProjectFixture()], + }); + MockApiClient.addMockResponse({ + url: `/organizations/${org.slug}/teams/`, + body: [TeamFixture()], + }); + + render( + + {renderer} + + ); + + MockApiClient.addMockResponse({ + url: `/customers/${org.slug}/`, + method: 'PUT', + }); + MockApiClient.addMockResponse({ + url: `/customers/${org.slug}/`, + method: 'GET', + body: sub, + }); + + await act(() => renderer.mock.calls.at(-1)![0].startTrial()); + + expect(handleTrialStarted).toHaveBeenCalled(); + expect(localStorageWrapper.removeItem).toHaveBeenCalledWith( + 'sidebar-new-seen:customizable-dashboards' + ); + + const startedCall = renderer.mock.calls.at(-1)![0]; + expect(startedCall.trialStarted).toBe(true); + expect(startedCall.trialFailed).toBe(false); + }); }); diff --git a/static/gsApp/components/trialStarter.tsx b/static/gsApp/components/trialStarter.tsx index bf4a6481716d45..3c9ecd1ae934a3 100644 --- a/static/gsApp/components/trialStarter.tsx +++ b/static/gsApp/components/trialStarter.tsx @@ -2,6 +2,7 @@ import {useState} from 'react'; import {fetchOrganizationDetails} from 'sentry/actionCreators/organization'; import type {Organization} from 'sentry/types/organization'; +import {localStorageWrapper} from 'sentry/utils/localStorage'; import {useApi} from 'sentry/utils/useApi'; import {withSubscription} from 'getsentry/components/withSubscription'; @@ -69,7 +70,7 @@ function TrialStarter(props: Props) { // we showed the "new" icon for the upsell that wasn't the actual dashboard // we should clear this so folks can see "new" for the actual dashboard - localStorage.removeItem('sidebar-new-seen:customizable-dashboards'); + localStorageWrapper.removeItem('sidebar-new-seen:customizable-dashboards'); }; const {subscription, children} = props; From 74f46be79883ba4fb8bad2a4e7e80499fd5320bc Mon Sep 17 00:00:00 2001 From: Alberto Leal Date: Tue, 14 Apr 2026 16:39:37 -0400 Subject: [PATCH 4/4] fix(billing): Include all Storage methods in localStorageWrapper mock The jest.mock spread of localStorageWrapper missed prototype methods like removeItem because native Storage objects don't have own-property methods. The "Start Trial" button in billing status tests triggers trialStarter which calls localStorageWrapper.removeItem(), causing TypeError in CI. Explicitly mock all Storage methods as pass-throughs. Co-Authored-By: Claude Opus 4.6 --- static/gsApp/components/navBillingStatus.spec.tsx | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/static/gsApp/components/navBillingStatus.spec.tsx b/static/gsApp/components/navBillingStatus.spec.tsx index 2ab4696f45301b..1c801f6b64ddd4 100644 --- a/static/gsApp/components/navBillingStatus.spec.tsx +++ b/static/gsApp/components/navBillingStatus.spec.tsx @@ -13,16 +13,22 @@ jest.mock('sentry/utils/localStorage', () => { const actual = jest.requireActual( 'sentry/utils/localStorage' ); + const wrapper = actual.localStorageWrapper; return { ...actual, localStorageWrapper: { - ...actual.localStorageWrapper, getItem: jest.fn((...args: Parameters) => - actual.localStorageWrapper.getItem(...args) + wrapper.getItem(...args) ), setItem: jest.fn((...args: Parameters) => - actual.localStorageWrapper.setItem(...args) + wrapper.setItem(...args) ), + removeItem: jest.fn((...args: Parameters) => + wrapper.removeItem(...args) + ), + clear: jest.fn(() => wrapper.clear()), + key: jest.fn((index: number) => wrapper.key(index)), + length: wrapper.length, }, }; });