From 02391843c80f84f7b3e5883bd175dc86916e1aa0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matu=CC=81s=CC=8C=20Tomlein?= Date: Mon, 18 Nov 2024 14:56:54 +0100 Subject: [PATCH] Serialize new session cookie synchronously to avoid overlapping sessions (close #1381) --- ...e-duplicate_sessions_2024-11-18-13-57.json | 10 +++++ ...e-duplicate_sessions_2024-11-18-13-57.json | 10 +++++ common/config/rush/pnpm-lock.yaml | 16 +++---- common/config/rush/repo-state.json | 2 +- .../src/tracker/cookie_storage.ts | 13 +++--- .../browser-tracker-core/src/tracker/index.ts | 12 +++++ .../test/helpers/index.ts | 8 +++- .../test/tracker/session_data.test.ts | 9 ++++ trackers/javascript-tracker/package.json | 2 +- .../test/integration/cookies.test.ts | 24 ++++++++++ .../test/pages/multi_page_cookies/iframe.html | 45 +++++++++++++++++++ .../test/pages/multi_page_cookies/index.html | 21 +++++++++ 12 files changed, 154 insertions(+), 18 deletions(-) create mode 100644 common/changes/@snowplow/browser-tracker-core/issue-duplicate_sessions_2024-11-18-13-57.json create mode 100644 common/changes/@snowplow/javascript-tracker/issue-duplicate_sessions_2024-11-18-13-57.json create mode 100644 trackers/javascript-tracker/test/integration/cookies.test.ts create mode 100644 trackers/javascript-tracker/test/pages/multi_page_cookies/iframe.html create mode 100644 trackers/javascript-tracker/test/pages/multi_page_cookies/index.html diff --git a/common/changes/@snowplow/browser-tracker-core/issue-duplicate_sessions_2024-11-18-13-57.json b/common/changes/@snowplow/browser-tracker-core/issue-duplicate_sessions_2024-11-18-13-57.json new file mode 100644 index 000000000..d7b4f490a --- /dev/null +++ b/common/changes/@snowplow/browser-tracker-core/issue-duplicate_sessions_2024-11-18-13-57.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@snowplow/browser-tracker-core", + "comment": "Serialize new session cookie synchronously to avoid overlapping sessions (#1381)", + "type": "none" + } + ], + "packageName": "@snowplow/browser-tracker-core" +} \ No newline at end of file diff --git a/common/changes/@snowplow/javascript-tracker/issue-duplicate_sessions_2024-11-18-13-57.json b/common/changes/@snowplow/javascript-tracker/issue-duplicate_sessions_2024-11-18-13-57.json new file mode 100644 index 000000000..093f698d7 --- /dev/null +++ b/common/changes/@snowplow/javascript-tracker/issue-duplicate_sessions_2024-11-18-13-57.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@snowplow/javascript-tracker", + "comment": "Serialize new session cookie synchronously to avoid overlapping sessions (#1381)", + "type": "none" + } + ], + "packageName": "@snowplow/javascript-tracker" +} \ No newline at end of file diff --git a/common/config/rush/pnpm-lock.yaml b/common/config/rush/pnpm-lock.yaml index e7b0a4c7f..a3877f873 100644 --- a/common/config/rush/pnpm-lock.yaml +++ b/common/config/rush/pnpm-lock.yaml @@ -2241,8 +2241,8 @@ importers: specifier: 4.1.2 version: 4.1.2 chromedriver: - specifier: ~129.0.0 - version: 129.0.4 + specifier: ~131.0.0 + version: 131.0.0 dockerode: specifier: ~3.3.1 version: 3.3.5 @@ -2299,7 +2299,7 @@ importers: version: 4.6.4 wdio-chromedriver-service: specifier: ~8.1.1 - version: 8.1.1(@wdio/types@8.39.0)(chromedriver@129.0.4)(webdriverio@8.39.1(encoding@0.1.13)(typescript@4.6.4)) + version: 8.1.1(@wdio/types@8.39.0)(chromedriver@131.0.0)(webdriverio@8.39.1(encoding@0.1.13)(typescript@4.6.4)) wdio-edgedriver-service: specifier: ~3.0.3 version: 3.0.3(@wdio/types@8.39.0) @@ -3604,8 +3604,8 @@ packages: resolution: {integrity: sha512-bIomtDF5KGpdogkLd9VspvFzk9KfpyyGlS8YFVZl7TGPBHL5snIOnxeshwVgPteQ9b4Eydl+pVbIyE1DcvCWgQ==} engines: {node: '>=10'} - chromedriver@129.0.4: - resolution: {integrity: sha512-j5I55cQwodFJUaYa1tWUmj2ss9KcPRBWmUa5Qonq3X8kqv2ASPyTboFYb4YB/YLztkYTUUw2E43txXw0wYzT/A==} + chromedriver@131.0.0: + resolution: {integrity: sha512-ukYmdCox2eRsjpCYUB4AOLV1fSfWQ1ZPfcUc0PIUWZKoyjyXKEl8i4DJ14bcNzNbEvaVx2Z2pnx/nLK2CM+ruQ==} engines: {node: '>=18'} hasBin: true @@ -9185,7 +9185,7 @@ snapshots: chownr@2.0.0: {} - chromedriver@129.0.4: + chromedriver@131.0.0: dependencies: '@testim/chrome-version': 1.1.4 axios: 1.7.7 @@ -13367,7 +13367,7 @@ snapshots: dependencies: defaults: 1.0.4 - wdio-chromedriver-service@8.1.1(@wdio/types@8.39.0)(chromedriver@129.0.4)(webdriverio@8.39.1(encoding@0.1.13)(typescript@4.6.4)): + wdio-chromedriver-service@8.1.1(@wdio/types@8.39.0)(chromedriver@131.0.0)(webdriverio@8.39.1(encoding@0.1.13)(typescript@4.6.4)): dependencies: '@wdio/logger': 8.38.0 fs-extra: 11.2.0 @@ -13376,7 +13376,7 @@ snapshots: webdriverio: 8.39.1(encoding@0.1.13)(typescript@4.6.4) optionalDependencies: '@wdio/types': 8.39.0 - chromedriver: 129.0.4 + chromedriver: 131.0.0 transitivePeerDependencies: - supports-color diff --git a/common/config/rush/repo-state.json b/common/config/rush/repo-state.json index ccb6e240d..523297ad7 100644 --- a/common/config/rush/repo-state.json +++ b/common/config/rush/repo-state.json @@ -1,5 +1,5 @@ // DO NOT MODIFY THIS FILE MANUALLY BUT DO COMMIT IT. It is generated and used by Rush. { - "pnpmShrinkwrapHash": "6693fc661ad5b6461d8a0fbbecf81c5f61d703bb", + "pnpmShrinkwrapHash": "77e8d084b2ff087ff8f5dfa5df436f42dbaa623c", "preferredVersionsHash": "bf21a9e8fbc5a3846fb05b4fa0859e0917b2202f" } diff --git a/libraries/browser-tracker-core/src/tracker/cookie_storage.ts b/libraries/browser-tracker-core/src/tracker/cookie_storage.ts index b7b2dfd07..8e329ae62 100644 --- a/libraries/browser-tracker-core/src/tracker/cookie_storage.ts +++ b/libraries/browser-tracker-core/src/tracker/cookie_storage.ts @@ -44,6 +44,11 @@ export interface CookieStorage { * @param secure - Boolean to specify if cookie should be secure */ deleteCookie(name: string, path?: string, domainName?: string, sameSite?: string, secure?: boolean): void; + + /** + * Write all pending cookies. + */ + flush(): void; } export interface AsyncCookieStorage extends CookieStorage { @@ -51,11 +56,6 @@ export interface AsyncCookieStorage extends CookieStorage { * Clear the cookie storage cache (does not delete any cookies) */ clearCache(): void; - - /** - * Write all pending cookies. - */ - flush(): void; } interface Cookie { @@ -203,5 +203,6 @@ export const syncCookieStorage: CookieStorage = { cookie(name, value, ttl, path, domain, samesite, secure); return document.cookie.indexOf(`${name}=`) !== -1; }, - deleteCookie + deleteCookie, + flush: () => {}, }; diff --git a/libraries/browser-tracker-core/src/tracker/index.ts b/libraries/browser-tracker-core/src/tracker/index.ts index 555b0a240..238aab44a 100755 --- a/libraries/browser-tracker-core/src/tracker/index.ts +++ b/libraries/browser-tracker-core/src/tracker/index.ts @@ -701,6 +701,11 @@ export function Tracker( // Update currentVisitTs updateNowTsInIdCookie(idCookie); setDomainUserIdCookie(idCookie); + + if (!eventIndexFromIdCookie(idCookie)) { + // Synchronously update the cookies to persist the new session ASAP + cookieStorage.flush(); + } } } @@ -920,6 +925,9 @@ export function Tracker( onSessionUpdateCallback && !manualSessionUpdateCalled ) { + // Synchronously update the cookies to persist the new session ASAP + cookieStorage.flush(); + onSessionUpdateCallback(clientSession); manualSessionUpdateCalled = false; } @@ -969,6 +977,10 @@ export function Tracker( const clientSession = clientSessionFromIdCookie(idCookie, configStateStorageStrategy, configAnonymousTracking); setDomainUserIdCookie(idCookie); const sessionIdentifierPersisted = setSessionCookie(); + + // Synchronously update the cookies to persist the new session ASAP + cookieStorage.flush(); + if (sessionIdentifierPersisted && onSessionUpdateCallback) { manualSessionUpdateCalled = true; onSessionUpdateCallback(clientSession); diff --git a/libraries/browser-tracker-core/test/helpers/index.ts b/libraries/browser-tracker-core/test/helpers/index.ts index cff0d8bfb..e59a175b0 100644 --- a/libraries/browser-tracker-core/test/helpers/index.ts +++ b/libraries/browser-tracker-core/test/helpers/index.ts @@ -75,8 +75,12 @@ export function createTestSessionIdCookie(params?: CreateTestSessionIdCookie) { return `_sp_ses.${domainHash}=*; Expires=; Path=/; SameSite=Lax; Secure;`; } -export function createTracker(configuration?: TrackerConfiguration, sharedState?: SharedState) { +export function createTracker( + configuration?: TrackerConfiguration, + sharedState?: SharedState, + syncCookieWrite: boolean = true +) { let id = 'sp-' + Math.random(); - configuration = { ...configuration, synchronousCookieWrite: true }; + configuration = { ...configuration, synchronousCookieWrite: syncCookieWrite }; return addTracker(id, id, '', '', sharedState ?? new SharedState(), configuration); } diff --git a/libraries/browser-tracker-core/test/tracker/session_data.test.ts b/libraries/browser-tracker-core/test/tracker/session_data.test.ts index 2994063ba..ff1904d06 100644 --- a/libraries/browser-tracker-core/test/tracker/session_data.test.ts +++ b/libraries/browser-tracker-core/test/tracker/session_data.test.ts @@ -57,6 +57,15 @@ describe('Tracker API: ', () => { jest.clearAllMocks(); }); + it('Writes cookies synchronously on session change', () => { + const tracker = createTracker(undefined, undefined, false); // async cookie writes enabled + + expect(cookieJar).toContain('_sp_ses'); + + tracker?.newSession(); + expect(cookieJar).toContain(tracker?.getDomainUserInfo().slice(1).join('.')); + }); + it('Sets initial domain session index on first session', () => { const tracker = createTracker(); diff --git a/trackers/javascript-tracker/package.json b/trackers/javascript-tracker/package.json index 14c1e1e70..aa1667863 100644 --- a/trackers/javascript-tracker/package.json +++ b/trackers/javascript-tracker/package.json @@ -87,7 +87,7 @@ "@wdio/static-server-service": "~8.39.0", "@wdio/types": "~8.39.0", "chalk": "4.1.2", - "chromedriver": "~129.0.0", + "chromedriver": "~131.0.0", "dockerode": "~3.3.1", "jest": "~27.5.1", "jest-environment-jsdom": "~27.5.1", diff --git a/trackers/javascript-tracker/test/integration/cookies.test.ts b/trackers/javascript-tracker/test/integration/cookies.test.ts new file mode 100644 index 000000000..e811b735a --- /dev/null +++ b/trackers/javascript-tracker/test/integration/cookies.test.ts @@ -0,0 +1,24 @@ +import { fetchResults } from '../micro'; +import { pageSetup } from './helpers'; + +describe('Tracker created domain cookies across multiple pages', () => { + let log: Array = []; + let testIdentifier = ''; + + beforeAll(async () => { + testIdentifier = await pageSetup(); + await browser.url('/multi_page_cookies'); + await browser.pause(5000); + + log = await browser.call(async () => await fetchResults()); + log = log.filter((ev) => (ev as any).event.app_id === 'cookies-iframe-' + testIdentifier); + }); + + it('all events should have the same session id', () => { + expect(log.length).toBeGreaterThanOrEqual(15); + + const sessionIds = log.map((ev) => (ev as any).event.domain_sessionid); + const uniqueSessionIds = Array.from(new Set(sessionIds)); + expect(uniqueSessionIds.length).toBe(1); + }); +}); diff --git a/trackers/javascript-tracker/test/pages/multi_page_cookies/iframe.html b/trackers/javascript-tracker/test/pages/multi_page_cookies/iframe.html new file mode 100644 index 000000000..f2427bba9 --- /dev/null +++ b/trackers/javascript-tracker/test/pages/multi_page_cookies/iframe.html @@ -0,0 +1,45 @@ + + + + Cookies iframe test page + + +
+
+ + + diff --git a/trackers/javascript-tracker/test/pages/multi_page_cookies/index.html b/trackers/javascript-tracker/test/pages/multi_page_cookies/index.html new file mode 100644 index 000000000..b5da19e21 --- /dev/null +++ b/trackers/javascript-tracker/test/pages/multi_page_cookies/index.html @@ -0,0 +1,21 @@ + + + + + Cookies test page + + + + + + +