Skip to content

Commit 236093a

Browse files
committed
Serialize new session cookie synchronously to avoid overlapping sessions (close #1381)
PR #1382
1 parent e835323 commit 236093a

File tree

12 files changed

+154
-18
lines changed

12 files changed

+154
-18
lines changed
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"changes": [
3+
{
4+
"packageName": "@snowplow/browser-tracker-core",
5+
"comment": "Serialize new session cookie synchronously to avoid overlapping sessions (#1381)",
6+
"type": "none"
7+
}
8+
],
9+
"packageName": "@snowplow/browser-tracker-core"
10+
}
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"changes": [
3+
{
4+
"packageName": "@snowplow/javascript-tracker",
5+
"comment": "Serialize new session cookie synchronously to avoid overlapping sessions (#1381)",
6+
"type": "none"
7+
}
8+
],
9+
"packageName": "@snowplow/javascript-tracker"
10+
}

common/config/rush/pnpm-lock.yaml

Lines changed: 8 additions & 8 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

common/config/rush/repo-state.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
// DO NOT MODIFY THIS FILE MANUALLY BUT DO COMMIT IT. It is generated and used by Rush.
22
{
3-
"pnpmShrinkwrapHash": "6693fc661ad5b6461d8a0fbbecf81c5f61d703bb",
3+
"pnpmShrinkwrapHash": "77e8d084b2ff087ff8f5dfa5df436f42dbaa623c",
44
"preferredVersionsHash": "bf21a9e8fbc5a3846fb05b4fa0859e0917b2202f"
55
}

libraries/browser-tracker-core/src/tracker/cookie_storage.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,18 +44,18 @@ export interface CookieStorage {
4444
* @param secure - Boolean to specify if cookie should be secure
4545
*/
4646
deleteCookie(name: string, path?: string, domainName?: string, sameSite?: string, secure?: boolean): void;
47+
48+
/**
49+
* Write all pending cookies.
50+
*/
51+
flush(): void;
4752
}
4853

4954
export interface AsyncCookieStorage extends CookieStorage {
5055
/**
5156
* Clear the cookie storage cache (does not delete any cookies)
5257
*/
5358
clearCache(): void;
54-
55-
/**
56-
* Write all pending cookies.
57-
*/
58-
flush(): void;
5959
}
6060

6161
interface Cookie {
@@ -203,5 +203,6 @@ export const syncCookieStorage: CookieStorage = {
203203
cookie(name, value, ttl, path, domain, samesite, secure);
204204
return document.cookie.indexOf(`${name}=`) !== -1;
205205
},
206-
deleteCookie
206+
deleteCookie,
207+
flush: () => {},
207208
};

libraries/browser-tracker-core/src/tracker/index.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -701,6 +701,11 @@ export function Tracker(
701701
// Update currentVisitTs
702702
updateNowTsInIdCookie(idCookie);
703703
setDomainUserIdCookie(idCookie);
704+
705+
if (!eventIndexFromIdCookie(idCookie)) {
706+
// Synchronously update the cookies to persist the new session ASAP
707+
cookieStorage.flush();
708+
}
704709
}
705710
}
706711

@@ -920,6 +925,9 @@ export function Tracker(
920925
onSessionUpdateCallback &&
921926
!manualSessionUpdateCalled
922927
) {
928+
// Synchronously update the cookies to persist the new session ASAP
929+
cookieStorage.flush();
930+
923931
onSessionUpdateCallback(clientSession);
924932
manualSessionUpdateCalled = false;
925933
}
@@ -969,6 +977,10 @@ export function Tracker(
969977
const clientSession = clientSessionFromIdCookie(idCookie, configStateStorageStrategy, configAnonymousTracking);
970978
setDomainUserIdCookie(idCookie);
971979
const sessionIdentifierPersisted = setSessionCookie();
980+
981+
// Synchronously update the cookies to persist the new session ASAP
982+
cookieStorage.flush();
983+
972984
if (sessionIdentifierPersisted && onSessionUpdateCallback) {
973985
manualSessionUpdateCalled = true;
974986
onSessionUpdateCallback(clientSession);

libraries/browser-tracker-core/test/helpers/index.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,12 @@ export function createTestSessionIdCookie(params?: CreateTestSessionIdCookie) {
7575
return `_sp_ses.${domainHash}=*; Expires=; Path=/; SameSite=Lax; Secure;`;
7676
}
7777

78-
export function createTracker(configuration?: TrackerConfiguration, sharedState?: SharedState) {
78+
export function createTracker(
79+
configuration?: TrackerConfiguration,
80+
sharedState?: SharedState,
81+
syncCookieWrite: boolean = true
82+
) {
7983
let id = 'sp-' + Math.random();
80-
configuration = { ...configuration, synchronousCookieWrite: true };
84+
configuration = { ...configuration, synchronousCookieWrite: syncCookieWrite };
8185
return addTracker(id, id, '', '', sharedState ?? new SharedState(), configuration);
8286
}

libraries/browser-tracker-core/test/tracker/session_data.test.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,15 @@ describe('Tracker API: ', () => {
5757
jest.clearAllMocks();
5858
});
5959

60+
it('Writes cookies synchronously on session change', () => {
61+
const tracker = createTracker(undefined, undefined, false); // async cookie writes enabled
62+
63+
expect(cookieJar).toContain('_sp_ses');
64+
65+
tracker?.newSession();
66+
expect(cookieJar).toContain(tracker?.getDomainUserInfo().slice(1).join('.'));
67+
});
68+
6069
it('Sets initial domain session index on first session', () => {
6170
const tracker = createTracker();
6271

trackers/javascript-tracker/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@
8787
"@wdio/static-server-service": "~8.39.0",
8888
"@wdio/types": "~8.39.0",
8989
"chalk": "4.1.2",
90-
"chromedriver": "~129.0.0",
90+
"chromedriver": "~131.0.0",
9191
"dockerode": "~3.3.1",
9292
"jest": "~27.5.1",
9393
"jest-environment-jsdom": "~27.5.1",
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import { fetchResults } from '../micro';
2+
import { pageSetup } from './helpers';
3+
4+
describe('Tracker created domain cookies across multiple pages', () => {
5+
let log: Array<unknown> = [];
6+
let testIdentifier = '';
7+
8+
beforeAll(async () => {
9+
testIdentifier = await pageSetup();
10+
await browser.url('/multi_page_cookies');
11+
await browser.pause(5000);
12+
13+
log = await browser.call(async () => await fetchResults());
14+
log = log.filter((ev) => (ev as any).event.app_id === 'cookies-iframe-' + testIdentifier);
15+
});
16+
17+
it('all events should have the same session id', () => {
18+
expect(log.length).toBeGreaterThanOrEqual(15);
19+
20+
const sessionIds = log.map((ev) => (ev as any).event.domain_sessionid);
21+
const uniqueSessionIds = Array.from(new Set(sessionIds));
22+
expect(uniqueSessionIds.length).toBe(1);
23+
});
24+
});

0 commit comments

Comments
 (0)