fix: switch to in-memory storage for events when necessary by saikumarrs · Pull Request #2494 · rudderlabs/rudder-sdk-js

saikumarrs · 2025-09-23T19:52:18Z

PR Description

Switched to in-memory storage for events when local storage is unavailable. This is needed in restrictive environments like Safari v26 and above.

Linear task (optional)

https://linear.app/rudderstack/issue/SDK-3972/switch-to-in-memory-storage-for-storing-events-when-persistent-storage

Cross Browser Tests

Please confirm you have tested for the following browsers:

Chrome
Firefox
Safari

Sanity Suite

All sanity suite test cases pass locally

Security

The code changed/added as part of this pull request won't create any security issues with how the software is being used.

Summary by CodeRabbit

New Features
- Queue entries now include an optional reclaimed flag for improved visibility of item state.
Bug Fixes
- Analytics queue now falls back to in-memory storage when local storage isn’t available, improving reliability in restricted environments.
Chores
- CI/CD workflows updated to supply NPM authentication and enable authenticated, verbose publishing steps.

coderabbitai · 2025-09-23T19:52:27Z

📝 Walkthrough

Walkthrough

📝 Walkthrough

XhrQueue now falls back to in-memory storage when localStorage is unavailable; queue item type gains an optional reclaimed?: boolean field and tests updated to assert it. CI workflows were updated to expose and use an NPM_TOKEN secret for authenticated npm publish/deprecate steps.

Changes

Cohort / File(s)	Summary
Types & Tests `packages/analytics-js-plugins/src/types/plugins.ts`, `packages/analytics-js-plugins/__tests__/xhrQueue/index.test.ts`	Added optional `reclaimed?: boolean` to exported `QueueItemData` type; tests updated to expect `reclaimed` (often `undefined`) in queue item payloads.
XhrQueue runtime behavior `packages/analytics-js-plugins/src/xhrQueue/index.ts`	Initialization now selects storage backend at runtime: use `LOCAL_STORAGE` if available, otherwise fall back to `MEMORY_STORAGE`. Comment documents fallback.
CI workflows (npm auth) `.github/workflows/deploy-beta.yml`, `.github/workflows/deploy-npm.yml`, `.github/workflows/publish-new-release.yml`, `.github/workflows/rollback.yml`	Add `NPM_TOKEN` secret exposure and set up npm auth (`npm set //registry.npmjs.org/:_authToken`) and/or `NODE_AUTH_TOKEN` in publish/deprecate steps; enable authenticated npm publish/deprecation with verbose publish flag in some steps.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant App
    participant XhrQueue
    participant StorageSelector
    participant LocalStorage as "LOCAL_STORAGE"
    participant MemoryStorage as "MEMORY_STORAGE"

    App->>XhrQueue: initialize()
    XhrQueue->>StorageSelector: isLocalStorageAvailable?
    alt localStorage available
        StorageSelector-->>LocalStorage: select
        XhrQueue-->>App: use LOCAL_STORAGE
    else fallback
        StorageSelector-->>MemoryStorage: select
        XhrQueue-->>App: use MEMORY_STORAGE
    end
    Note right of XhrQueue: queue items include optional\n`reclaimed?: boolean` in stored shape

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

feat: add events processing and retry headers #2066 — Adds reclaimed?: boolean to queue item data/type (same field addition to queue types).

Pre-merge checks

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title "fix: switch to in-memory storage for events when necessary" is concise and accurately describes the main change — falling back to in-memory storage when localStorage is unavailable (as implemented for event/XhrQueue storage); it is clear, focused, and appropriate for a single-line PR title.
Description Check	✅ Passed	The PR description includes a clear summary, motivation, and the Linear task link and notes Chrome testing, but it diverges from the repository template by listing Safari instead of the template's IE11 entry and leaves Firefox, Sanity Suite, and Security checkboxes unchecked; the core change and rationale are present so the description is mostly complete but some required checklist items and template conformity are missing.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

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 and usage tips.}

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (3)

packages/analytics-js-plugins/src/xhrQueue/index.ts (1)

128-129: Good fallback; add a one-time log for observability when memory storage is used.

This helps support/debugging in environments without localStorage (SW, Safari ITP, third‑party iframes). Suggest logging once at init.

Apply this diff:

         storeManager,
-        // If local storage is available, use it, else use memory storage
-        state.capabilities.storage.isLocalStorageAvailable.value ? LOCAL_STORAGE : MEMORY_STORAGE,
+        // If local storage is available, use it, else use memory storage
+        (() => {
+          const storageType = state.capabilities.storage.isLocalStorageAvailable.value
+            ? LOCAL_STORAGE
+            : MEMORY_STORAGE;
+          if (storageType === MEMORY_STORAGE) {
+            logger?.info?.(
+              'XhrQueue: Using in-memory storage for queue (localStorage unavailable)',
+            );
+          }
+          return storageType;
+        })(),
         logger,

packages/analytics-js-plugins/__tests__/xhrQueue/index.test.ts (2)

216-216: Avoid asserting reclaimed: undefined in strict deep equals.

This is brittle and ties tests to an internal optional field being materialized as undefined. Prefer partial/object match.

Example change (pattern):

-    expect(queue.getStorageEntry('queue')).toStrictEqual([
-      {
-        item: { /* ... */ },
-        attemptNumber: 1,
-        lastAttemptedAt: expect.any(Number),
-        firstAttemptedAt: expect.any(Number),
-        reclaimed: undefined,
-        id: 'sample_uuid',
-        time: 1 + 1000 * 2 ** 1,
-        type: 'Single',
-        retryReason: 'server-429',
-      },
-    ]);
+    expect(queue.getStorageEntry('queue')).toMatchObject([
+      {
+        item: { /* ... */ },
+        attemptNumber: 1,
+        lastAttemptedAt: expect.any(Number),
+        firstAttemptedAt: expect.any(Number),
+        id: 'sample_uuid',
+        time: 1 + 1000 * 2 ** 1,
+        type: 'Single',
+        retryReason: 'server-429',
+      },
+    ]);

Repeat similarly for the other two assertions.

Also applies to: 395-395, 503-503

55-66: Add a unit test for the storage fallback path.

Set state.capabilities.storage.isLocalStorageAvailable.value = false and assert we log the in‑memory fallback during init.

You can add a test like:

it('falls back to memory storage when localStorage is unavailable', () => {
  batch(() => {
    state.capabilities.storage.isLocalStorageAvailable.value = false;
  });

  (XhrQueue()?.dataplaneEventsQueue as ExtensionPoint).init?.(
    state,
    defaultHttpClient,
    defaultStoreManager,
    undefined,
    defaultLogger,
  );

  expect(defaultLogger.info).toHaveBeenCalledWith(
    'XhrQueue: Using in-memory storage for queue (localStorage unavailable)',
  );
});

If defaultLogger.info isn’t available in the mock, switch to warn in both code and test.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9f62677 and f6e41e7.

📒 Files selected for processing (2)

packages/analytics-js-plugins/__tests__/xhrQueue/index.test.ts (3 hunks)
packages/analytics-js-plugins/src/xhrQueue/index.ts (2 hunks)

🧰 Additional context used

📓 Path-based instructions (6)

**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/002-javascript-typescript.mdc)

Code must work in browsers AND service workers (no Node.js APIs)

Files:

packages/analytics-js-plugins/__tests__/xhrQueue/index.test.ts
packages/analytics-js-plugins/src/xhrQueue/index.ts

packages/*/__tests__/**