improve Segment.io retries (#714)

Author: chrisradek

.changeset/clean-lizards-nail.md

@@ -0,0 +1,5 @@
+---
+'@segment/analytics-next': patch
+---
+
+Improves Segment.io retries to include exponential backoff and work across page loads.

packages/browser-integration-tests/package.json

@@ -20,6 +20,7 @@
     "@playwright/test": "^1.28.1",
     "@segment/analytics-next": "workspace:^",
     "http-server": "14.1.1",
-    "nock": "^13.2.9"
+    "nock": "^13.2.9",
+    "tslib": "^2.4.1"
   }
 }

packages/browser-integration-tests/src/segment-retries.test.ts

@@ -1,4 +1,4 @@
-import { test, expect } from '@playwright/test'
+import { Request, test, expect } from '@playwright/test'
 import { SettingsBuilder } from './fixtures/settings'
 import { standaloneMock } from './helpers/standalone-mock'
 
@@ -25,7 +25,9 @@ test.describe('Standalone tests', () => {
       )
     })
 
-    test.skip('supports retries on page navigation', async ({ page }) => {
+    test('supports retrying failed requests on page navigation', async ({
+      page,
+    }) => {
       // Load analytics.js
       await page.goto('/standalone.html')
       await page.evaluate(() => window.analytics.load('fake-key'))
@@ -40,16 +42,16 @@ test.describe('Standalone tests', () => {
           times: 1,
         }
       )
-      const requestFailure = new Promise((resolve) => {
-        page.once('requestfailed', resolve)
+      const requestFailure = new Promise<Record<string, any>>((resolve) => {
+        page.once('requestfailed', (request) => resolve(request.postDataJSON()))
       })
 
       // trigger an event
       await page.evaluate(() => {
         void window.analytics.track('test event')
       })
 
-      await requestFailure
+      const { messageId } = await requestFailure
       await page.reload()
 
       // load analytics.js again and wait for a new request.
@@ -59,6 +61,55 @@ test.describe('Standalone tests', () => {
       ])
 
       expect(request.method()).toBe('POST')
+      expect(request.postDataJSON().messageId).toBe(messageId)
+    })
+
+    test('supports retrying in-flight requests on page navigation', async ({
+      page,
+    }) => {
+      // Load analytics.js
+      await page.goto('/standalone.html')
+      await page.evaluate(() => window.analytics.load('fake-key'))
+
+      // blackhole the request so that it stays in-flight when we reload the page
+      await page.route(
+        'https://api.segment.io/v1/t',
+        async () => {
+          // do nothing
+        },
+        {
+          times: 1,
+        }
+      )
+
+      // Detect when we've seen a track request initiated by the browser
+      const requestSent = new Promise<Record<string, any>>((resolve) => {
+        const onRequest: (req: Request) => void = (req) => {
+          if (req.url() === 'https://api.segment.io/v1/t') {
+            page.off('request', onRequest)
+            resolve(req.postDataJSON())
+          }
+        }
+
+        page.on('request', onRequest)
+      })
+
+      // trigger an event
+      await page.evaluate(() => {
+        void window.analytics.track('test event')
+      })
+
+      const { messageId } = await requestSent
+      await page.reload()
+
+      // load analytics.js again and wait for a new request.
+      const [request] = await Promise.all([
+        page.waitForRequest('https://api.segment.io/v1/t'),
+        page.evaluate(() => window.analytics.load('fake-key')),
+      ])
+
+      expect(request.method()).toBe('POST')
+      expect(request.postDataJSON().messageId).toBe(messageId)
     })
   })
 })

packages/browser-integration-tests/src/shims.d.ts

@@ -0,0 +1,7 @@
+import type { AnalyticsSnippet } from '@segment/analytics-next'
+
+declare global {
+  interface Window {
+    analytics: AnalyticsSnippet
+  }
+}

packages/browser/package.json

@@ -56,7 +56,7 @@
     "js-cookie": "3.0.1",
     "node-fetch": "^2.6.7",
     "spark-md5": "^3.0.1",
-    "tslib": "^2.4.0",
+    "tslib": "^2.4.1",
     "unfetch": "^4.1.0"
   },
   "devDependencies": {

packages/browser/src/lib/__tests__/on-page-leave.test.ts

@@ -2,7 +2,8 @@ import { onPageLeave } from '../on-page-leave'
 
 const helpers = {
   dispatchEvent(event: 'pagehide' | 'visibilitychange') {
-    document.dispatchEvent(new Event(event))
+    const target = event === 'pagehide' ? window : document
+    target.dispatchEvent(new Event(event))
   },
   setVisibilityState(state: DocumentVisibilityState) {
     Object.defineProperty(document, 'visibilityState', {

packages/browser/src/lib/on-page-leave.ts

@@ -10,13 +10,13 @@
 export const onPageLeave = (cb: (...args: unknown[]) => void) => {
   let unloaded = false // prevents double firing if both are supported
 
-  // using document instead of window because of bug affecting browsers before safari 14 (detail in footnotes https://caniuse.com/?search=visibilitychange)
-  document.addEventListener('pagehide', () => {
+  window.addEventListener('pagehide', () => {
     if (unloaded) return
     unloaded = true
     cb()
   })
 
+  // using document instead of window because of bug affecting browsers before safari 14 (detail in footnotes https://caniuse.com/?search=visibilitychange)
   document.addEventListener('visibilitychange', () => {
     if (document.visibilityState == 'hidden') {
       if (unloaded) return

packages/browser/src/lib/priority-queue/__tests__/persisted.test.ts

@@ -41,7 +41,7 @@ describe('Persisted Priority Queue', () => {
     let onUnload: any = jest.fn()
 
     jest
-      .spyOn(document, 'addEventListener')
+      .spyOn(window, 'addEventListener')
       .mockImplementation((evt, handler) => {
         if (evt === 'pagehide') {
           onUnload = handler
@@ -102,7 +102,7 @@ describe('Persisted Priority Queue', () => {
       let onUnload: any = jest.fn()
 
       jest
-        .spyOn(document, 'addEventListener')
+        .spyOn(window, 'addEventListener')
         .mockImplementation((evt, handler) => {
           if (evt === 'pagehide') {
             onUnload = handler
@@ -200,7 +200,7 @@ describe('Persisted Priority Queue', () => {
       const onUnloadFunctions: any[] = []
 
       jest
-        .spyOn(document, 'addEventListener')
+        .spyOn(window, 'addEventListener')
         .mockImplementation((evt, handler) => {
           if (evt === 'pagehide') {
             onUnloadFunctions.push(handler)

packages/browser/src/lib/priority-queue/persisted.ts

@@ -109,7 +109,7 @@ export class PersistedPriorityQueue extends PriorityQueue<Context> {
       }
     })
 
-    document.addEventListener('pagehide', () => {
+    window.addEventListener('pagehide', () => {
       // we deliberately want to use the less powerful 'pagehide' API to only persist on events where the analytics instance gets destroyed, and not on tab away.
       if (this.todo > 0) {
         const items = [...this.queue, ...this.future]

packages/browser/src/plugins/segmentio/__tests__/batched-dispatcher.test.ts

@@ -224,7 +224,7 @@ describe('Batching', () => {
 
       expect(fetch).not.toHaveBeenCalled()
 
-      document.dispatchEvent(new Event('pagehide'))
+      window.dispatchEvent(new Event('pagehide'))
 
       expect(fetch).toHaveBeenCalledTimes(1)
 
@@ -253,7 +253,7 @@ describe('Batching', () => {
 
       expect(fetch).not.toHaveBeenCalled()
 
-      document.dispatchEvent(new Event('pagehide'))
+      window.dispatchEvent(new Event('pagehide'))
 
       expect(fetch).toHaveBeenCalledTimes(2)
     })