From fd48f6c57f425238fd51dc961a1b5b4a97ddeed1 Mon Sep 17 00:00:00 2001 From: iamtoruk Date: Thu, 16 Jul 2026 12:10:38 -0700 Subject: [PATCH] fix(app): telemetry flush drops permanently-rejected batches A 4xx response means the server will never accept that payload; retrying it wedges the queue at its cap and blocks all newer events. Drop on 4xx, keep retrying on 5xx and network failures. Matches the server's validation contract (batch cap 200). --- app/electron/telemetry.test.ts | 12 ++++++++++-- app/electron/telemetry.ts | 8 +++++++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/app/electron/telemetry.test.ts b/app/electron/telemetry.test.ts index 4f1f222..239c791 100644 --- a/app/electron/telemetry.test.ts +++ b/app/electron/telemetry.test.ts @@ -137,9 +137,9 @@ describe('events', () => { expect(telemetry.queueLength).toBe(before + 1) }) - it('keeps the queue on a failed POST and clears it on success', async () => { + it('keeps the queue on a transient failure (5xx) and clears it on success', async () => { let ok = false - const fetchFn = vi.fn(async () => ({ ok })) as unknown as typeof fetch + const fetchFn = vi.fn(async () => ({ ok, status: 503 })) as unknown as typeof fetch const { telemetry } = make({ fetchFn }) telemetry.completeOnboarding(true) expect(await telemetry.flush()).toBe(false) @@ -149,6 +149,14 @@ describe('events', () => { expect(telemetry.queueLength).toBe(0) }) + it('drops a permanently rejected batch (4xx) instead of wedging the queue', async () => { + const fetchFn = vi.fn(async () => ({ ok: false, status: 400 })) as unknown as typeof fetch + const { telemetry } = make({ fetchFn }) + telemetry.completeOnboarding(true) + expect(await telemetry.flush()).toBe(false) + expect(telemetry.queueLength).toBe(0) + }) + it('batches with the wire contract: schema, installId, app block, events', async () => { const { telemetry, posts } = make() telemetry.completeOnboarding(true) diff --git a/app/electron/telemetry.ts b/app/electron/telemetry.ts index 469a23e..a20d959 100644 --- a/app/electron/telemetry.ts +++ b/app/electron/telemetry.ts @@ -244,7 +244,13 @@ export class Telemetry { headers: { 'content-type': 'application/json' }, body, }) - if (!res.ok) return false + if (!res.ok) { + // 4xx is a permanent rejection of this batch (schema drift, bad shape): + // retrying the same payload forever would wedge the queue at its cap. + // Drop it. 5xx/network are transient — keep the batch for the next beat. + if (res.status >= 400 && res.status < 500) this.queue = this.queue.filter(e => !events.includes(e)) + return false + } // Only drop what was sent; events tracked mid-flight stay queued. this.queue = this.queue.filter(e => !events.includes(e)) return true