From b9db91876cde85c955a493ddf5744cb79b9feeac Mon Sep 17 00:00:00 2001 From: Arcadio Quintero Date: Thu, 5 Mar 2026 14:17:14 -0500 Subject: [PATCH 1/2] fix(analytics): update storage handling during keepalive flush to prevent duplicate event sends --- .../queue/dot-analytics.queue.utils.spec.ts | 8 +++++--- .../shared/queue/dot-analytics.queue.utils.ts | 18 +++++++++--------- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/core-web/libs/sdk/analytics/src/lib/core/shared/queue/dot-analytics.queue.utils.spec.ts b/core-web/libs/sdk/analytics/src/lib/core/shared/queue/dot-analytics.queue.utils.spec.ts index 218a10c33da7..432d4a2736f6 100644 --- a/core-web/libs/sdk/analytics/src/lib/core/shared/queue/dot-analytics.queue.utils.spec.ts +++ b/core-web/libs/sdk/analytics/src/lib/core/shared/queue/dot-analytics.queue.utils.spec.ts @@ -807,7 +807,7 @@ describe('createAnalyticsQueue', () => { expect(sessionStorageRemoveItem).toHaveBeenCalled(); }); - it('should NOT clear storage on keepalive flush', () => { + it('should clear storage on keepalive flush to prevent duplicate sends', () => { const queue = createAnalyticsQueue(mockConfig); queue.initialize(); @@ -829,8 +829,10 @@ describe('createAnalyticsQueue', () => { // Simulate flush with keepalive sendBatchCallback([mockEvent], []); - // Should NOT clear storage (keepalive flush leaves backup) - expect(sessionStorageRemoveItem).not.toHaveBeenCalled(); + // Storage should be cleared even for keepalive flushes. + // sessionStorage writes are synchronous and complete before unload, + // so leaving stale events causes the next page to re-send them. + expect(sessionStorageRemoveItem).toHaveBeenCalled(); }); it('should handle corrupted storage gracefully', () => { diff --git a/core-web/libs/sdk/analytics/src/lib/core/shared/queue/dot-analytics.queue.utils.ts b/core-web/libs/sdk/analytics/src/lib/core/shared/queue/dot-analytics.queue.utils.ts index 4e1e9bc6a350..bc2e64fbf17f 100644 --- a/core-web/libs/sdk/analytics/src/lib/core/shared/queue/dot-analytics.queue.utils.ts +++ b/core-web/libs/sdk/analytics/src/lib/core/shared/queue/dot-analytics.queue.utils.ts @@ -237,15 +237,15 @@ export const createAnalyticsQueue = (config: DotCMSAnalyticsConfig) => { (e) => !events.some((sent) => sent === e) ); - // Clear storage after normal flush (not keepalive) - // For keepalive flushes (page unload), keep events in storage as backup - if (!keepalive) { - if (eventsForPersistence.length === 0) { - clearStorage(); - } else { - // Update storage with remaining events - persistToStorage(); - } + // Always update storage after sending — even for keepalive flushes. + // sessionStorage writes are synchronous and complete before page unload, + // so the persistence state stays consistent. Previously, keepalive flushes + // skipped the storage update as a "safety net", but that caused the next + // page load to re-send the same events from persistence, producing duplicates. + if (eventsForPersistence.length === 0) { + clearStorage(); + } else { + persistToStorage(); } }; From 9c8b4b5bf300c654b5ed5f80104471a77eb0e12a Mon Sep 17 00:00:00 2001 From: Arcadio Quintero Date: Thu, 5 Mar 2026 16:13:22 -0500 Subject: [PATCH 2/2] fix(analytics): address PR review comments on storage handling - Clarify sendBatch comment to reflect fire-and-forget nature of sendAnalyticsEvent - Strengthen test assertions: use toHaveBeenCalledTimes(1) instead of toHaveBeenCalled() - Clear spy before flush in normal flush test to isolate the assertion --- .../core/shared/queue/dot-analytics.queue.utils.spec.ts | 9 ++++++--- .../lib/core/shared/queue/dot-analytics.queue.utils.ts | 8 +++++--- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/core-web/libs/sdk/analytics/src/lib/core/shared/queue/dot-analytics.queue.utils.spec.ts b/core-web/libs/sdk/analytics/src/lib/core/shared/queue/dot-analytics.queue.utils.spec.ts index 432d4a2736f6..e6f00e0ec849 100644 --- a/core-web/libs/sdk/analytics/src/lib/core/shared/queue/dot-analytics.queue.utils.spec.ts +++ b/core-web/libs/sdk/analytics/src/lib/core/shared/queue/dot-analytics.queue.utils.spec.ts @@ -800,11 +800,14 @@ describe('createAnalyticsQueue', () => { queue.enqueue(mockEvent, mockContext); + // Clear previous calls from initialize/enqueue + sessionStorageRemoveItem.mockClear(); + // Simulate normal flush (keepalive=false) sendBatchCallback([mockEvent], []); - // Should clear storage after successful send - expect(sessionStorageRemoveItem).toHaveBeenCalled(); + // Should clear storage after dispatching send + expect(sessionStorageRemoveItem).toHaveBeenCalledTimes(1); }); it('should clear storage on keepalive flush to prevent duplicate sends', () => { @@ -832,7 +835,7 @@ describe('createAnalyticsQueue', () => { // Storage should be cleared even for keepalive flushes. // sessionStorage writes are synchronous and complete before unload, // so leaving stale events causes the next page to re-send them. - expect(sessionStorageRemoveItem).toHaveBeenCalled(); + expect(sessionStorageRemoveItem).toHaveBeenCalledTimes(1); }); it('should handle corrupted storage gracefully', () => { diff --git a/core-web/libs/sdk/analytics/src/lib/core/shared/queue/dot-analytics.queue.utils.ts b/core-web/libs/sdk/analytics/src/lib/core/shared/queue/dot-analytics.queue.utils.ts index bc2e64fbf17f..394dc49f2904 100644 --- a/core-web/libs/sdk/analytics/src/lib/core/shared/queue/dot-analytics.queue.utils.ts +++ b/core-web/libs/sdk/analytics/src/lib/core/shared/queue/dot-analytics.queue.utils.ts @@ -237,11 +237,13 @@ export const createAnalyticsQueue = (config: DotCMSAnalyticsConfig) => { (e) => !events.some((sent) => sent === e) ); - // Always update storage after sending — even for keepalive flushes. + // Always update storage after dispatching the send — even for keepalive flushes. + // Note: sendAnalyticsEvent is fire-and-forget (the returned promise is not awaited), + // so this runs regardless of whether the HTTP request succeeds. // sessionStorage writes are synchronous and complete before page unload, // so the persistence state stays consistent. Previously, keepalive flushes - // skipped the storage update as a "safety net", but that caused the next - // page load to re-send the same events from persistence, producing duplicates. + // skipped the storage update, which caused the next page load to re-send + // the same events from persistence, producing duplicates. if (eventsForPersistence.length === 0) { clearStorage(); } else {