Add test for metrics client

src/analytics/metrics/client.test.ts

@@ -0,0 +1,176 @@
+import {MetricsClient} from './client'
+
+let appStateCallback: (state: string) => void
+
+jest.mock('#/lib/appState', () => ({
+  onAppStateChange: jest.fn(cb => {
+    appStateCallback = cb
+    return {remove: jest.fn()}
+  }),
+}))
+
+jest.mock('#/logger', () => ({
+  Logger: {
+    create: () => ({
+      info: jest.fn(),
+      debug: jest.fn(),
+      error: jest.fn(),
+    }),
+    Context: {Metric: 'metric'},
+  },
+}))
+
+jest.mock('#/env', () => ({
+  METRICS_API_HOST: 'https://test.metrics.api',
+  IS_WEB: false,
+}))
+
+type TestEvents = {
+  click: {button: string}
+  view: {screen: string}
+}
+
+describe('MetricsClient', () => {
+  let fetchMock: jest.Mock
+  let fetchRequests: {body: any}[]
+
+  beforeEach(() => {
+    jest.useFakeTimers({advanceTimers: true})
+    fetchRequests = []
+    fetchMock = jest.fn().mockImplementation(async (_url, options) => {
+      const body = JSON.parse(options.body)
+      fetchRequests.push({body})
+      return {ok: true, status: 200}
+    })
+    global.fetch = fetchMock
+  })
+
+  afterEach(() => {
+    jest.useRealTimers()
+    jest.clearAllMocks()
+  })
+
+  it('flushes events on interval', async () => {
+    const client = new MetricsClient<TestEvents>()
+    client.track('click', {button: 'submit'})
+    client.track('view', {screen: 'home'})
+
+    expect(fetchRequests).toHaveLength(0)
+
+    // Advance past the 10 second interval
+    await jest.advanceTimersByTimeAsync(10_000)
+
+    expect(fetchRequests).toHaveLength(1)
+    expect(fetchRequests[0].body.events).toHaveLength(2)
+    expect(fetchRequests[0].body.events[0].event).toBe('click')
+    expect(fetchRequests[0].body.events[1].event).toBe('view')
+  })
+
+  it('flushes when maxBatchSize is exceeded', async () => {
+    const client = new MetricsClient<TestEvents>()
+    client.maxBatchSize = 5
+
+    // Add events up to maxBatchSize (should not flush yet)
+    for (let i = 0; i < 5; i++) {
+      client.track('click', {button: `btn-${i}`})
+    }
+
+    expect(fetchRequests).toHaveLength(0)
+
+    // One more event should trigger flush (> maxBatchSize)
+    client.track('click', {button: 'btn-trigger'})
+
+    // Allow microtasks to run
+    await jest.advanceTimersByTimeAsync(0)
+
+    expect(fetchRequests).toHaveLength(1)
+    expect(fetchRequests[0].body.events).toHaveLength(6)
+  })
+
+  it('retries failed events once on 500 response', async () => {
+    let requestCount = 0
+
+    fetchMock.mockImplementation(async (_url, options) => {
+      requestCount++
+      const body = JSON.parse(options.body)
+
+      if (requestCount === 1) {
+        // First request fails with 500 - "Failed to fetch" triggers isNetworkError
+        return {
+          ok: false,
+          status: 500,
+          text: async () => 'Internal Server Error',
+        }
+      }
+
+      // Retry succeeds
+      fetchRequests.push({body})
+      return {ok: true, status: 200}
+    })
+
+    const client = new MetricsClient<TestEvents>()
+    client.track('click', {button: 'submit'})
+
+    // Trigger flush via interval
+    await jest.advanceTimersByTimeAsync(10_000)
+
+    expect(requestCount).toBe(1)
+    expect(fetchRequests).toHaveLength(0)
+
+    // Simulate app coming to foreground to trigger retry
+    appStateCallback('active')
+    await jest.advanceTimersByTimeAsync(0)
+
+    expect(requestCount).toBe(2)
+    expect(fetchRequests).toHaveLength(1)
+    expect(fetchRequests[0].body.events).toHaveLength(1)
+    expect(fetchRequests[0].body.events[0].event).toBe('click')
+  })
+
+  it('does not retry more than once', async () => {
+    let requestCount = 0
+
+    fetchMock.mockImplementation(async () => {
+      requestCount++
+      // Always fail with network-like error
+      return {
+        ok: false,
+        status: 500,
+        text: async () => 'Internal Server Error',
+      }
+    })
+
+    const client = new MetricsClient<TestEvents>()
+    client.track('click', {button: 'submit'})
+
+    // First flush fails
+    await jest.advanceTimersByTimeAsync(10_000)
+
+    expect(requestCount).toBe(1)
+
+    // Retry also fails
+    appStateCallback('active')
+    await jest.advanceTimersByTimeAsync(0)
+
+    expect(requestCount).toBe(2)
+
+    // Another foreground event should not retry again (events are dropped)
+    appStateCallback('active')
+    await jest.advanceTimersByTimeAsync(0)
+
+    expect(requestCount).toBe(2) // No additional requests
+  })
+
+  it('flushes when app goes to background', async () => {
+    const client = new MetricsClient<TestEvents>()
+    client.track('click', {button: 'submit'})
+
+    expect(fetchRequests).toHaveLength(0)
+
+    // Simulate app going to background
+    appStateCallback('background')
+    await jest.advanceTimersByTimeAsync(0)
+
+    expect(fetchRequests).toHaveLength(1)
+  })
+})

src/analytics/metrics/client.ts

@@ -1,11 +1,8 @@
 import {onAppStateChange} from '#/lib/appState'
 import {isNetworkError} from '#/lib/strings/errors'
 import {Logger} from '#/logger'
-import {Sentry} from '#/logger/sentry/lib'
 import * as env from '#/env'
 
-// TODO debug logging
-
 type Event<M extends Record<string, any>> = {
   time: number
   event: keyof M
@@ -17,6 +14,8 @@ const TRACKING_ENDPOINT = env.METRICS_API_HOST + '/t'
 const logger = Logger.create(Logger.Context.Metric, {})
 
 export class MetricsClient<M extends Record<string, any>> {
+  maxBatchSize = 100
+
   private started: boolean = false
   private queue: Event<M>[] = []
   private failedQueue: Event<M>[] = []
@@ -51,12 +50,12 @@ export class MetricsClient<M extends Record<string, any>> {
       metadata,
     })
 
-    logger.debug(`event: ${event as string}`, {
+    logger.info(`event: ${event as string}`, {
       payload,
       metadata,
     })
 
-    if (this.queue.length > 100) {
+    if (this.queue.length > this.maxBatchSize) {
       this.flush()
     }
   }
@@ -64,7 +63,6 @@ export class MetricsClient<M extends Record<string, any>> {
   flush() {
     if (!this.queue.length) return
     const events = this.queue.splice(0, this.queue.length)
-    this.queue = []
     this.sendBatch(events)
   }
 
@@ -76,17 +74,21 @@ export class MetricsClient<M extends Record<string, any>> {
     try {
       const body = JSON.stringify({events})
       if (env.IS_WEB && 'navigator' in globalThis && navigator.sendBeacon) {
-        navigator.sendBeacon(
+        const success = navigator.sendBeacon(
           TRACKING_ENDPOINT,
           new Blob([body], {type: 'application/json'}),
         )
+        if (!success) {
+          // construct a "network error" for `isNetworkError` to work
+          throw new Error(`Failed to fetch: sendBeacon returned false`)
+        }
       } else {
         const res = await fetch(TRACKING_ENDPOINT, {
           method: 'POST',
           headers: {
             'Content-Type': 'application/json',
           },
-          body: JSON.stringify(events),
+          body: JSON.stringify({events}),
           keepalive: true,
         })
 
@@ -102,10 +104,8 @@ export class MetricsClient<M extends Record<string, any>> {
         this.failedQueue.push(...events)
         return
       }
-      Sentry.captureException(`Failed to send metrics`, {
+      logger.error(`Failed to send metrics`, {
-        extra: {
         safeMessage: e.toString(),
-        },
       })
     }
   }
@@ -113,7 +113,6 @@ export class MetricsClient<M extends Record<string, any>> {
   private retryFailedLogs() {
     if (!this.failedQueue.length) return
     const events = this.failedQueue.splice(0, this.failedQueue.length)
-    this.failedQueue = []
     this.sendBatch(events, true)
   }
 }