Skip to content

fix: remove redundant cookie writes causing race conditions#1560

Draft
daniel-graham-amplitude wants to merge 14 commits intomainfrom
AMP-149299-fix-cookie-overuse
Draft

fix: remove redundant cookie writes causing race conditions#1560
daniel-graham-amplitude wants to merge 14 commits intomainfrom
AMP-149299-fix-cookie-overuse

Conversation

@daniel-graham-amplitude
Copy link
Collaborator

@daniel-graham-amplitude daniel-graham-amplitude commented Feb 27, 2026

Summary

PROBLEM

This block of code has a race condition in it:

  1. it writes deviceId which triggers updateStorage which won't include lastEventId, lastEventTime, etc... because it hasn't been written yet
  2. it writes the rest of the values, which also update storage

If you refresh between #1 and #2 then it won't have lastEventId or lastEventTime in the cookies, which will cause them to be reset the next page and cause a new session_start event.

SOLUTION

This fix adds a variable to disable update storage temporarily.

Checklist

  • Does your PR title have the correct title format?
  • Does your PR have a breaking change?:

Note

Medium Risk
Changes when BrowserConfig persists identity/session fields by batching initial writes into a single updateStorage() call, which could affect cookie/state consistency if any consumers relied on intermediate writes. Scope is limited to browser-side session storage updates and a small test/mock adjustment.

Overview
Prevents redundant/partial cookie writes during BrowserConfig construction by temporarily disabling updateStorage() while setters run, then re-enabling and persisting once at the end (guarded via a new _disableStorageUpdate flag).

Adds a note in the context plugin about atomizing event-id/time writes, and updates getTopLevelDomain tests to stop stubbing a removed/unused CookieStorage._isEnabled field.

Written by Cursor Bugbot for commit 8b2befe. This will update automatically on new commits. Configure here.

Base automatically changed from AMP-149016-is-enabled-diagnostics to main February 27, 2026 19:03
@daniel-graham-amplitude daniel-graham-amplitude marked this pull request as draft February 27, 2026 19:47
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