Amp 149016 global is enabled locked

packages/analytics-browser/e2e/cookies-is-enabled.spec.ts

@@ -0,0 +1,26 @@
+import { test, expect } from '@playwright/test';
+
+const RUNS = 10;
+
+test.describe('CookieStorage', () => {
+  // Regression Test to cover re-entrancy issues fixed by https://github.com/amplitude/Amplitude-TypeScript/pull/1539
+  test('.isEnabled works when called multiple times concurrently', async ({ page }) => {
+    const consoleErrors: string[] = [];
+    page.on('console', (msg) => {
+      if (msg.type() === 'error') {
+        consoleErrors.push(msg.text());
+      }
+    });
+
+    const pageErrors: string[] = [];
+    page.on('pageerror', (err) => {
+      pageErrors.push(err.message);
+    });
+
+    for (let i = 0; i < RUNS; i++) {
+      await page.goto('/cookies/is-enabled.html');
+      expect(pageErrors, `Expected no page errors, but got: ${pageErrors.join('; ')}`).toHaveLength(0);
+      expect(consoleErrors, `Expected no console.error calls, but got: ${consoleErrors.join('; ')}`).toHaveLength(0);
+    }
+  });
+});

packages/analytics-browser/src/config.ts

@@ -31,6 +31,7 @@ import {
   isDomainEqual,
   CookieStorageConfig,
   decodeCookieValue,
+  getGlobalScope,
 } from '@amplitude/analytics-core';
 
 import { LocalStorage } from './storage/local-storage';
@@ -295,7 +296,9 @@ export const useBrowserConfig = async (
   const identityStorage = options.identityStorage || DEFAULT_IDENTITY_STORAGE;
   const cookieOptions = {
     domain:
-      identityStorage !== DEFAULT_IDENTITY_STORAGE ? '' : options.cookieOptions?.domain ?? (await getTopLevelDomain()),
+      identityStorage !== DEFAULT_IDENTITY_STORAGE
+        ? ''
+        : options.cookieOptions?.domain ?? (await getTopLevelDomain(undefined, diagnosticsClient)),
     expiration: 365,
     sameSite: 'Lax' as const,
     secure: false,
@@ -468,9 +471,51 @@ export const createTransport = (transport?: TransportTypeOrOptions) => {
   return new FetchTransport(headers);
 };
 
-export const getTopLevelDomain = async (url?: string) => {
+export const getTopLevelDomain = async (url?: string, diagnosticsClient?: IDiagnosticsClient) => {
   if (
-    !(await new CookieStorage<number>().isEnabled()) ||
+    !(await new CookieStorage<number>(undefined, { diagnosticsClient }).isEnabled()) ||
+    (!url && (typeof location === 'undefined' || !location.hostname))
+  ) {
+    return '';
+  }
+
+  const global = getGlobalScope();
+  if (!global?.navigator?.locks) {
+    return legacyGetTopLevelDomain(url, diagnosticsClient);
+  }
+
+  return await global.navigator.locks.request('com/amplitude/get-top-level-domain', async () => {
+    const host = url ?? location.hostname;
+    const parts = host.split('.');
+    const levels = [];
+    const storageKey = 'AMP_TLDTEST';
+
+    for (let i = parts.length - 2; i >= 0; --i) {
+      levels.push(parts.slice(i).join('.'));
+    }
+    for (let i = 0; i < levels.length; i++) {
+      const domain = levels[i];
+      const options = { domain: '.' + domain };
+      const storage = new CookieStorage<number>(options);
+      try {
+        await storage.set(storageKey, 1);
+        const value = await storage.get(storageKey);
+        if (value) {
+          console.log('TLD -- exiting with domain:', '.' + domain);
+          return '.' + domain;
+        }
+      } finally {
+        await storage.remove(storageKey);
+      }
+    }
+
+    return '';
+  });
+};
+
+const legacyGetTopLevelDomain = async (url?: string, diagnosticsClient?: IDiagnosticsClient) => {
+  if (
+    !(await new CookieStorage<number>(undefined, { diagnosticsClient }).isEnabled()) ||
     (!url && (typeof location === 'undefined' || !location.hostname))
   ) {
     return '';

packages/analytics-browser/test/config.test.ts

@@ -393,6 +393,7 @@ describe('config', () => {
         ...testCookieStorage,
         options: {},
         config: {},
+        legacyIsEnabled: jest.fn().mockResolvedValueOnce(Promise.resolve(false)),
       });
       const domain = await Config.getTopLevelDomain();
       expect(domain).toBe('');
@@ -421,11 +422,13 @@ describe('config', () => {
           ...testCookieStorage,
           options: {},
           config: {},
+          legacyIsEnabled: jest.fn().mockResolvedValueOnce(Promise.resolve(true)),
         })
         .mockReturnValue({
           ...actualCookieStorage,
           options: {},
           config: {},
+          legacyIsEnabled: jest.fn().mockResolvedValueOnce(Promise.resolve(true)),
         });
       expect(await Config.getTopLevelDomain('www.legislation.gov.uk')).toBe('.legislation.gov.uk');
     });
@@ -461,6 +464,21 @@ describe('config', () => {
         configurable: true,
       });
     });
+
+    test('should record a diagnostics event when no access to cookies', async () => {
+      const mockDiagnosticsClient = {
+        recordEvent: jest.fn(),
+        setTag: jest.fn(),
+        increment: jest.fn(),
+        recordHistogram: jest.fn(),
+        _flush: jest.fn(),
+        _setSampleRate: jest.fn(),
+      };
+      await Config.getTopLevelDomain('www.amplitude.com', mockDiagnosticsClient);
+      expect(mockDiagnosticsClient.recordEvent).toHaveBeenCalledWith('cookies.tld.failure', {
+        reason: 'Could not determine TLD for host www.amplitude.com',
+      });
+    });
   });
 
   describe('fetchRemoteConfig', () => {

packages/analytics-browser/test/cookie-migration/index.test.ts

@@ -1,7 +1,6 @@
-import { Storage, UserSession } from '@amplitude/analytics-core';
+import { Storage, UserSession, MemoryStorage, CookieStorage, getOldCookieName } from '@amplitude/analytics-core';
 import { decode, parseLegacyCookies, parseTime } from '../../src/cookie-migration';
 import * as LocalStorageModule from '../../src/storage/local-storage';
-import { MemoryStorage, CookieStorage, getOldCookieName } from '@amplitude/analytics-core';
 
 describe('cookie-migration', () => {
   const API_KEY = 'asdfasdf';

packages/analytics-core/src/storage/cookie.ts

@@ -35,6 +35,59 @@ export class CookieStorage<T> implements Storage<T> {
     if (!globalScope || !globalScope.document) {
       return false;
     }
+    const { navigator } = globalScope;
+    if (!navigator?.locks) {
+      return await this.legacyIsEnabled();
+    }
+
+    return await navigator.locks.request('com/amplitude/cookie-storage-is-enabled', async () => {
+      const testValue = String(Date.now());
+      const testCookieOptions = { ...this.options };
+      const testStorage = new CookieStorage<string>(testCookieOptions);
+      const testKey = 'AMP_TEST';
+      try {
+        await testStorage.set(testKey, testValue);
+        const value = await testStorage.get(testKey);
+
+        /* istanbul ignore next */
+        if (value !== testValue && this.config.diagnosticsClient) {
+          this.config.diagnosticsClient?.recordEvent('cookies.isEnabled.failure', {
+            reason: 'Test Value mismatch',
+            testKey,
+            testValue,
+          });
+        }
+        console.log('isEnabled -- exiting with result:', value === testValue);
+        return value === testValue;
+      } catch (e){
+        /* istanbul ignore next */
+        if (this.config.diagnosticsClient) {
+          const errMessage = e instanceof Error ? e.message : String(e);
+          this.config.diagnosticsClient?.recordEvent('cookies.isEnabled.failure', {
+            reason: 'Cookie getter/setter failed',
+            testKey,
+            testValue,
+            error: errMessage,
+          });
+        }
+        return false;
+      } finally {
+        await testStorage.remove(testKey);
+      }
+    });
+  }
+
+  /**
+   * Detects if cookies are enabled by setting a test cookie and checking if it is set.
+   * Legacy version that doesn't use navigator.locks
+   * @returns boolean
+   */
+  async legacyIsEnabled(): Promise<boolean> {
+    const globalScope = getGlobalScope();
+    /* istanbul ignore if */
+    if (!globalScope || !globalScope.document) {
+      return false;
+    }
 
     const testValue = String(Date.now());
     const testCookieOptions = {
@@ -46,9 +99,26 @@ export class CookieStorage<T> implements Storage<T> {
     try {
       await testStorage.set(testKey, testValue);
       const value = await testStorage.get(testKey);
+      /* istanbul ignore next */
+      if (value !== testValue && this.config.diagnosticsClient) {
+        this.config.diagnosticsClient?.recordEvent('cookies.isEnabled.failure', {
+          reason: 'Test Value mismatch',
+          testKey,
+          testValue,
+        });
+      }
       return value === testValue;
-    } catch {
+    } catch (e) {
       /* istanbul ignore next */
+      if (this.config.diagnosticsClient) {
+        const errMessage = e instanceof Error ? e.message : String(e);
+        this.config.diagnosticsClient?.recordEvent('cookies.isEnabled.failure', {
+          reason: 'Cookie getter/setter failed',
+          testKey,
+          testValue,
+          error: errMessage,
+        });
+      }
       return false;
     } finally {
       await testStorage.remove(testKey);

packages/analytics-core/test/storage/cookies.test.ts

@@ -9,22 +9,119 @@ import * as GlobalScopeModule from '../../src/global-scope';
 
 describe('cookies', () => {
   describe('isEnabled', () => {
-    test('regression test re-entrancy issue', async () => {
-      const c1 = new CookieStorage();
-      const c2 = new CookieStorage();
-      // calling isEnabled one after the other should not cause a re-entrancy issue
-      const promises = [c1.isEnabled(), c2.isEnabled()];
-      const res = await Promise.all(promises);
-      expect(res).toEqual([true, true]);
+    describe('concurrent calls', () => {
+      beforeEach(() => {
+        jest.useFakeTimers();
+      });
+      afterEach(() => {
+        jest.useRealTimers();
+      });
+
+      test('regression test re-entrancy issue', async () => {
+        const c1 = new CookieStorage();
+        const c2 = new CookieStorage();
+        // calling isEnabled one after the other should not cause a re-entrancy issue
+        const p1 = c1.isEnabled();
+        jest.advanceTimersByTime(10);
+        const p2 = c2.isEnabled();
+        await Promise.all([p1, p2]);
+        expect(await p1).toBe(true);
+        expect(await p2).toBe(true);
+      });
     });
+
     test('should return true', async () => {
       const cookies = new CookieStorage();
       expect(await cookies.isEnabled()).toBe(true);
     });
-    test('should return false when document is not available', async () => {
-      jest.spyOn(GlobalScopeModule, 'getGlobalScope').mockReturnValueOnce({} as typeof globalThis);
-      const cookies = new CookieStorage();
-      expect(await cookies.isEnabled()).toBe(false);
+
+    describe('when document is not available', () => {
+      let getGlobalScopeSpy: jest.SpyInstance;
+      beforeEach(() => {
+        getGlobalScopeSpy = jest.spyOn(GlobalScopeModule, 'getGlobalScope').mockReturnValue({} as typeof globalThis);
+      });
+      afterEach(() => {
+        getGlobalScopeSpy.mockRestore();
+      });
+      test('should return false', async () => {
+        const cookies = new CookieStorage();
+        expect(await cookies.isEnabled()).toBe(false);
+      });
+    });
+
+    describe('when document.cookie throws an error', () => {
+      let getGlobalScopeSpy: jest.SpyInstance;
+      beforeEach(() => {
+        getGlobalScopeSpy = jest.spyOn(GlobalScopeModule, 'getGlobalScope').mockReturnValue({} as typeof globalThis);
+        getGlobalScopeSpy.mockImplementation(() => {
+          return {
+            document: {
+              cookie: {
+                get() {
+                  throw new Error('getter error');
+                },
+                set() {
+                  throw new Error('setter error');
+                },
+              },
+            },
+          };
+        });
+      });
+      afterEach(() => {
+        getGlobalScopeSpy.mockRestore();
+      });
+
+      test('should return false', async () => {
+        const mockDiagnosticsClient = {
+          recordEvent: jest.fn(),
+          increment: jest.fn(),
+          recordHistogram: jest.fn(),
+          setTag: jest.fn(),
+          _flush: jest.fn(),
+          _setSampleRate: jest.fn(),
+        };
+        const cookies = new CookieStorage(undefined, { diagnosticsClient: mockDiagnosticsClient as any });
+        expect(await cookies.isEnabled()).toBe(false);
+        expect(mockDiagnosticsClient.recordEvent).toHaveBeenCalledTimes(1);
+      });
+    });
+
+    describe('when document.cookie returns wrong test value', () => {
+      let getGlobalScopeSpy: jest.SpyInstance;
+      beforeEach(() => {
+        getGlobalScopeSpy = jest.spyOn(GlobalScopeModule, 'getGlobalScope').mockReturnValue({} as typeof globalThis);
+        getGlobalScopeSpy.mockImplementation(() => {
+          return {
+            document: {
+              cookie: {
+                get() {
+                  return 'wrong value';
+                },
+                set() {
+                  return 'wrong value';
+                },
+              },
+            },
+          };
+        });
+      });
+      afterEach(() => {
+        getGlobalScopeSpy.mockRestore();
+      });
+      test('should return false', async () => {
+        const mockDiagnosticsClient = {
+          recordEvent: jest.fn(),
+          increment: jest.fn(),
+          recordHistogram: jest.fn(),
+          setTag: jest.fn(),
+          _flush: jest.fn(),
+          _setSampleRate: jest.fn(),
+        };
+        const cookies = new CookieStorage(undefined, { diagnosticsClient: mockDiagnosticsClient as any });
+        expect(await cookies.isEnabled()).toBe(false);
+        expect(mockDiagnosticsClient.recordEvent).toHaveBeenCalledTimes(1);
+      });
     });
   });