diff --git a/integrations/agent-plugin-core/typescript/src/telemetry.ts b/integrations/agent-plugin-core/typescript/src/telemetry.ts index 656ed9858..0beffe6b9 100644 --- a/integrations/agent-plugin-core/typescript/src/telemetry.ts +++ b/integrations/agent-plugin-core/typescript/src/telemetry.ts @@ -94,12 +94,19 @@ export function createTelemetry(config: TelemetryConfig) { const maxQueueSize = config.maxQueueSize ?? 100; const deliver = config.delivery ?? (async (batch: Record[]) => { - await fetch(POSTHOG_BATCH_URL, { + const response = await fetch(POSTHOG_BATCH_URL, { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ api_key: POSTHOG_API_KEY, batch }), signal: AbortSignal.timeout(3_000), }); + // fetch only rejects on a network-level failure. Without this check a 500, + // a 503 or a 429 resolved normally and the batch was counted as delivered + // and dropped, which is the likelier outage than a refused connection. + // Any non-2xx is retried, matching the Python core: the backoff and the + // queue bound contain a payload that will never be accepted, because the + // re-queued batch sits at the front and is the first thing evicted. + if (!response.ok) throw new Error(`posthog responded ${response.status}`); }); async function flush(): Promise { diff --git a/integrations/agent-plugin-core/typescript/tests/telemetry.test.ts b/integrations/agent-plugin-core/typescript/tests/telemetry.test.ts index 3e5ab9fba..5a8ca0439 100644 --- a/integrations/agent-plugin-core/typescript/tests/telemetry.test.ts +++ b/integrations/agent-plugin-core/typescript/tests/telemetry.test.ts @@ -214,3 +214,46 @@ test("a long outage costs the oldest events, not unbounded memory", async () => assert.ok(telemetry.queueForTesting().length <= 3, "queue grew past maxQueueSize"); telemetry.resetForTesting(); }); + +test("an HTTP error response is a failure, not a delivery", async () => { + // fetch only rejects on a network-level failure, so a 500 used to resolve + // normally and the batch was dropped as delivered. Exercises the real default + // delivery path rather than an injected one, which is where this hid. + const realFetch = globalThis.fetch; + let calls = 0; + globalThis.fetch = (async () => { + calls += 1; + return new Response("upstream is unwell", { status: 503 }); + }) as typeof fetch; + + const telemetry = createTelemetry({ + host: "h", source: "S", version: "1", distinctId: "d", flushThreshold: 1000, + }); + try { + telemetry.capture("during.outage"); + await telemetry.flush(); + + assert.equal(calls, 1, "never reached the network"); + assert.equal(telemetry.queueForTesting().length, 1, "a 503 was counted as delivered"); + } finally { + globalThis.fetch = realFetch; + telemetry.resetForTesting(); + } +}); + +test("a 2xx is a delivery", async () => { + const realFetch = globalThis.fetch; + globalThis.fetch = (async () => new Response("ok", { status: 200 })) as typeof fetch; + + const telemetry = createTelemetry({ + host: "h", source: "S", version: "1", distinctId: "d", flushThreshold: 1000, + }); + try { + telemetry.capture("fine"); + await telemetry.flush(); + assert.equal(telemetry.queueForTesting().length, 0, "a good response did not clear the queue"); + } finally { + globalThis.fetch = realFetch; + telemetry.resetForTesting(); + } +});