From 5c1ee377ccd4d40de159dbbf4d2ad5179cf4f750 Mon Sep 17 00:00:00 2001 From: Anna Garcia Date: Mon, 24 Aug 2026 07:19:42 -0400 Subject: [PATCH 1/4] fix(core): retry logs batches after exhausted transient failures Logs classified an exhausted 408/429/5xx as `fatal`, so the batch was dropped instead of held for the next flush cycle. Fixes #4570. --- .../core/src/__tests__/posthog.flush.spec.ts | 18 ++++++++++++++++++ packages/core/src/logs/index.ts | 2 +- packages/core/src/posthog-core-stateless.ts | 11 ++++------- 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/packages/core/src/__tests__/posthog.flush.spec.ts b/packages/core/src/__tests__/posthog.flush.spec.ts index 03ccb11366..6949474e37 100644 --- a/packages/core/src/__tests__/posthog.flush.spec.ts +++ b/packages/core/src/__tests__/posthog.flush.spec.ts @@ -454,6 +454,24 @@ describe('PostHog Core', () => { expect(Date.now() - time).toBeLessThan(1000) }) + it.each([408, 429, 500])('keeps logs batches retryable after exhausted retries with %s error', async (status) => { + ;[posthog, mocks] = createTestClient('TEST_API_KEY', { + fetchRetryCount: 0, + preloadFeatureFlags: false, + }) + mocks.fetch.mockResolvedValue({ + status, + text: async () => 'err', + json: async () => ({ status: 'err' }), + }) + + await expect(posthog._sendLogsBatch({ resourceLogs: [] })).resolves.toMatchObject({ + kind: 'retry-later', + error: { name: 'PostHogFetchHttpError', status }, + }) + expect(mocks.fetch).toHaveBeenCalledTimes(1) + }) + it('responds with an error after retries with network error ', async () => { mocks.fetch.mockImplementation(() => { return Promise.reject(new Error('network problems')) diff --git a/packages/core/src/logs/index.ts b/packages/core/src/logs/index.ts index 2ee89e0f99..3b642eab6e 100644 --- a/packages/core/src/logs/index.ts +++ b/packages/core/src/logs/index.ts @@ -262,7 +262,7 @@ export class PostHogLogs { } if (outcome.kind === 'retry-later') { - // Network error: keep records in the queue for the next flush cycle + // Transient failure: keep records in the queue for the next flush cycle // and surface the error so the caller can log/react. throw outcome.error } diff --git a/packages/core/src/posthog-core-stateless.ts b/packages/core/src/posthog-core-stateless.ts index 587c2a7564..8a2b226a16 100644 --- a/packages/core/src/posthog-core-stateless.ts +++ b/packages/core/src/posthog-core-stateless.ts @@ -222,13 +222,12 @@ function isPostHogEventProperties(value: JsonType | undefined): value is PostHog } /** - * Outcome of a logs batch send. Keeps HTTP error classification inside core - * (single source of truth — same policy events already use in `_flush()`) so + * Outcome of a logs batch send. Keeps HTTP error classification inside core so * PostHogLogs doesn't need to know about specific error types. * * - ok → records are accepted; drop them from the queue * - too-large → 413; caller should halve batch size and retry same records - * - retry-later → network error; caller keeps records and retries next cycle + * - retry-later → retryable network or HTTP error; caller keeps records and retries next cycle * - fatal → anything else (auth, malformed, etc.); caller drops the * batch and surfaces the error */ @@ -1635,9 +1634,7 @@ export abstract class PostHogCoreStateless { /** * Sends a pre-built OTLP logs payload to `/i/v1/logs`. Returns a tagged * outcome instead of throwing so PostHogLogs doesn't have to know about the - * core's error class hierarchy. Error classification lives here (single - * source of truth, same policy the events `_flush()` uses for its own - * 413 / network / fatal handling). + * core's error class hierarchy. Error classification lives here. * * 413 is passed through as `too-large` (not auto-retried) so the caller can * shrink `maxBatchRecordsPerPost` and retry the same records. @@ -1680,7 +1677,7 @@ export abstract class PostHogCoreStateless { if (isPostHogFetchContentTooLargeError(err)) { return { kind: 'too-large' } } - if (err instanceof PostHogFetchNetworkError) { + if (isPostHogFetchRetryableError(err)) { return { kind: 'retry-later', error: err } } return { kind: 'fatal', error: err } From 6ec3163c206b2c79232b55cdbc051301e18414f9 Mon Sep 17 00:00:00 2001 From: Anna Garcia Date: Mon, 24 Aug 2026 07:19:42 -0400 Subject: [PATCH 2/4] refactor(core): send logs and metrics through one OTLP batch sender Collapses both senders into `_sendOtlpBatch` so the retry policy is one decision, pins the classification with a per-signal test table, and retries 408 in the browser logs and metrics adapters. --- .changeset/logs-retry-transient-failures.md | 6 ++ .changeset/web-logs-metrics-retry-408.md | 5 ++ .../src/__tests__/posthog-logs.test.ts | 5 ++ .../src/__tests__/posthog-metrics.test.ts | 10 +++ packages/browser/src/posthog-logs.ts | 4 +- packages/browser/src/posthog-metrics.ts | 4 +- .../core/src/__tests__/posthog.flush.spec.ts | 70 +++++++++++---- packages/core/src/logs/index.spec.ts | 46 ++++++++++ packages/core/src/posthog-core-stateless.ts | 89 +++++++------------ 9 files changed, 159 insertions(+), 80 deletions(-) create mode 100644 .changeset/logs-retry-transient-failures.md create mode 100644 .changeset/web-logs-metrics-retry-408.md diff --git a/.changeset/logs-retry-transient-failures.md b/.changeset/logs-retry-transient-failures.md new file mode 100644 index 0000000000..81865d7cc7 --- /dev/null +++ b/.changeset/logs-retry-transient-failures.md @@ -0,0 +1,6 @@ +--- +'posthog-react-native': patch +'@posthog/core': patch +--- + +Fix buffered logs being dropped instead of retried after HTTP 408, 429 or 5xx diff --git a/.changeset/web-logs-metrics-retry-408.md b/.changeset/web-logs-metrics-retry-408.md new file mode 100644 index 0000000000..fd8ebcb2a1 --- /dev/null +++ b/.changeset/web-logs-metrics-retry-408.md @@ -0,0 +1,5 @@ +--- +'posthog-js': patch +--- + +Fix logs and metrics batches being dropped instead of retried after HTTP 408 diff --git a/packages/browser/src/__tests__/posthog-logs.test.ts b/packages/browser/src/__tests__/posthog-logs.test.ts index baadb3f6f2..97a95b3c7d 100644 --- a/packages/browser/src/__tests__/posthog-logs.test.ts +++ b/packages/browser/src/__tests__/posthog-logs.test.ts @@ -1098,6 +1098,11 @@ describe('posthog-logs', () => { expect((logs as any)._queue).toHaveLength(1) }) + it('keeps records on a 408 so they retry later', async () => { + await flushWith(408) + expect((logs as any)._queue).toHaveLength(1) + }) + it('drops records on a 4xx client error', async () => { await flushWith(400) expect((logs as any)._queue).toHaveLength(0) diff --git a/packages/browser/src/__tests__/posthog-metrics.test.ts b/packages/browser/src/__tests__/posthog-metrics.test.ts index fd3d35ad0e..a8e032da9d 100644 --- a/packages/browser/src/__tests__/posthog-metrics.test.ts +++ b/packages/browser/src/__tests__/posthog-metrics.test.ts @@ -93,6 +93,16 @@ describe('posthog-metrics', () => { expect(last.sum.dataPoints[0].asDouble).toBe(2) }) + it('retains the window on a 408', async () => { + respondWithStatus(408) + metrics.count('a', 5) + await metrics.flush() + + respondWithStatus(200) + await metrics.flush() + expect(sentRequests()[1].data.resourceMetrics[0].scopeMetrics[0].metrics[0].sum.dataPoints[0].asDouble).toBe(5) + }) + it('captures nothing when the instance is not capturing', async () => { ;(mockPostHog.is_capturing as jest.Mock).mockReturnValue(false) metrics.count('a', 1) diff --git a/packages/browser/src/posthog-logs.ts b/packages/browser/src/posthog-logs.ts index 4abbce74d7..5fa3d4ee7f 100644 --- a/packages/browser/src/posthog-logs.ts +++ b/packages/browser/src/posthog-logs.ts @@ -343,8 +343,8 @@ export class PostHogLogs implements Extension { settle({ kind: 'ok' }) } else if (status === 413) { settle({ kind: 'too-large' }) - } else if (status === 0 || status === 429 || status >= 500) { - // Transient (network / rate-limit / server error): keep and retry. + } else if (status === 0 || status === 408 || status === 429 || status >= 500) { + // Transient (network / timeout / rate-limit / server error): keep and retry. if (status === 0) { // `_send_request` already logs fetch failures. Bare status 0 is the // XHR/synthetic path, so keep one warning for it here. diff --git a/packages/browser/src/posthog-metrics.ts b/packages/browser/src/posthog-metrics.ts index 538b0eb962..a2989fb620 100644 --- a/packages/browser/src/posthog-metrics.ts +++ b/packages/browser/src/posthog-metrics.ts @@ -151,8 +151,8 @@ export class PostHogMetrics implements Extension { settle({ kind: 'ok' }) } else if (status === 413) { settle({ kind: 'too-large' }) - } else if (status === 0 || status === 429 || status >= 500) { - // Transient (network / rate-limit / server error): keep and retry. + } else if (status === 0 || status === 408 || status === 429 || status >= 500) { + // Transient (network / timeout / rate-limit / server error): keep and retry. settle({ kind: 'retry-later', error: response.error ?? new Error(`metrics request failed with status ${status}`), diff --git a/packages/core/src/__tests__/posthog.flush.spec.ts b/packages/core/src/__tests__/posthog.flush.spec.ts index 6949474e37..a1b6a33f81 100644 --- a/packages/core/src/__tests__/posthog.flush.spec.ts +++ b/packages/core/src/__tests__/posthog.flush.spec.ts @@ -454,24 +454,6 @@ describe('PostHog Core', () => { expect(Date.now() - time).toBeLessThan(1000) }) - it.each([408, 429, 500])('keeps logs batches retryable after exhausted retries with %s error', async (status) => { - ;[posthog, mocks] = createTestClient('TEST_API_KEY', { - fetchRetryCount: 0, - preloadFeatureFlags: false, - }) - mocks.fetch.mockResolvedValue({ - status, - text: async () => 'err', - json: async () => ({ status: 'err' }), - }) - - await expect(posthog._sendLogsBatch({ resourceLogs: [] })).resolves.toMatchObject({ - kind: 'retry-later', - error: { name: 'PostHogFetchHttpError', status }, - }) - expect(mocks.fetch).toHaveBeenCalledTimes(1) - }) - it('responds with an error after retries with network error ', async () => { mocks.fetch.mockImplementation(() => { return Promise.reject(new Error('network problems')) @@ -778,4 +760,56 @@ describe('PostHog Core', () => { ]) }) }) + + describe('OTLP batch senders', () => { + // Both share one `_sendOtlpBatch`; the table pins them to the same + // classification so a wrapper can't reintroduce a per-signal retry policy. + const senders = { + logs: (client: PostHogCoreTestClient) => client._sendLogsBatch({ resourceLogs: [] }), + metrics: (client: PostHogCoreTestClient) => client._sendMetricsBatch({ resourceMetrics: [] }), + } + + const cases: [number, string][] = [ + [408, 'retry-later'], + [429, 'retry-later'], + [500, 'retry-later'], + [503, 'retry-later'], + [413, 'too-large'], + [400, 'fatal'], + [401, 'fatal'], + ] + + describe.each(Object.entries(senders))('%s', (_name, send) => { + it.each(cases)('classifies an exhausted %i as %s', async (status, kind) => { + ;[posthog, mocks] = createTestClient('TEST_API_KEY', { + fetchRetryCount: 0, + preloadFeatureFlags: false, + }) + mocks.fetch.mockResolvedValue({ + status, + text: async () => 'err', + json: async () => ({ status: 'err' }), + }) + + await expect(send(posthog)).resolves.toMatchObject({ kind }) + expect(mocks.fetch).toHaveBeenCalledTimes(1) + }) + + it('carries the HTTP error on a retry-later outcome', async () => { + ;[posthog, mocks] = createTestClient('TEST_API_KEY', { + fetchRetryCount: 0, + preloadFeatureFlags: false, + }) + mocks.fetch.mockResolvedValue({ + status: 503, + text: async () => 'err', + json: async () => ({ status: 'err' }), + }) + + await expect(send(posthog)).resolves.toMatchObject({ + error: { name: 'PostHogFetchHttpError', status: 503 }, + }) + }) + }) + }) }) diff --git a/packages/core/src/logs/index.spec.ts b/packages/core/src/logs/index.spec.ts index 49d135d518..d8f6d0b399 100644 --- a/packages/core/src/logs/index.spec.ts +++ b/packages/core/src/logs/index.spec.ts @@ -1,4 +1,5 @@ import { PostHogPersistedProperty } from '../types' +import { createTestClient, PostHogCoreTestClient } from '../testing' import type { Logger } from '../types' import { PostHogLogs } from './index' import type { BufferedLogEntry, ResolvedPostHogLogsConfig } from './types' @@ -80,6 +81,51 @@ const getContextFor = (instance: any) => (): { distinctId?: string; sessionId?: sessionId: instance.getSessionId() || undefined, }) +// Drives a real core host rather than a stubbed `_sendLogsBatch`, so the sender's +// error classification and the queue bookkeeping are exercised together. +describe('PostHogLogs over the core sender', () => { + const createLogsOverCore = (status: number): { logs: PostHogLogs; client: PostHogCoreTestClient } => { + const [client, mocks] = createTestClient('TEST_API_KEY', { + fetchRetryCount: 0, + preloadFeatureFlags: false, + }) + mocks.fetch.mockResolvedValue({ + status, + text: () => Promise.resolve('err'), + json: () => Promise.resolve({ status: 'err' }), + }) + const logs = new PostHogLogs( + client, + resolveForTest(), + createMockLogger(), + () => ({ distinctId: 'user-123' }), + immediateOnReady + ) + return { logs, client } + } + + const queueOf = (client: PostHogCoreTestClient): BufferedLogEntry[] => + client.getPersistedProperty(PostHogPersistedProperty.LogsQueue) ?? [] + + it.each([408, 429, 500, 503])('keeps records queued when the endpoint answers %i', async (status) => { + const { logs, client } = createLogsOverCore(status) + logs.captureLog({ body: 'keep me' }) + + await expect(logs.flush()).rejects.toHaveProperty('name', 'PostHogFetchHttpError') + + expect(queueOf(client)).toHaveLength(1) + }) + + it('drops the batch when the endpoint answers 401', async () => { + const { logs, client } = createLogsOverCore(401) + logs.captureLog({ body: 'unauthorized' }) + + await expect(logs.flush()).rejects.toHaveProperty('name', 'PostHogFetchHttpError') + + expect(queueOf(client)).toHaveLength(0) + }) +}) + describe('PostHogLogs', () => { let mockInstance: any let logger: Logger diff --git a/packages/core/src/posthog-core-stateless.ts b/packages/core/src/posthog-core-stateless.ts index 8a2b226a16..36d9b95385 100644 --- a/packages/core/src/posthog-core-stateless.ts +++ b/packages/core/src/posthog-core-stateless.ts @@ -237,6 +237,17 @@ export type SendLogsBatchOutcome = | { kind: 'retry-later'; error: unknown } | { kind: 'fatal'; error: unknown } +/** + * Each signal keeps its own exported outcome type because each belongs to a + * separate host contract. The wrappers return this value directly, so one + * drifting out of shape fails to compile. + */ +type SendOtlpBatchOutcome = + | { kind: 'ok' } + | { kind: 'too-large' } + | { kind: 'retry-later'; error: unknown } + | { kind: 'fatal'; error: unknown } + export enum QuotaLimitedFeature { FeatureFlags = 'feature_flags', Recordings = 'recordings', @@ -1632,20 +1643,28 @@ export abstract class PostHogCoreStateless { } /** - * Sends a pre-built OTLP logs payload to `/i/v1/logs`. Returns a tagged - * outcome instead of throwing so PostHogLogs doesn't have to know about the - * core's error class hierarchy. Error classification lives here. + * Shared implementation behind the OTLP senders, which differ only in path. + * Returns a tagged outcome instead of throwing so the queue owners don't + * have to know the core's error class hierarchy. * - * 413 is passed through as `too-large` (not auto-retried) so the caller can - * shrink `maxBatchRecordsPerPost` and retry the same records. + * Exhausted 408/429/5xx stay `retry-later`, unlike the events `_flush()` + * which drops anything that isn't a network error: every OTLP queue is + * bounded and retried with backoff, so holding a batch through an outage can + * neither grow without limit nor spin. */ - async _sendLogsBatch(payload: OtlpLogsPayload): Promise { + private async _sendOtlpBatch({ + path, + payload, + }: { + path: 'logs' | 'metrics' + payload: OtlpLogsPayload | OtlpMetricsPayload + }): Promise { if (this.disabled) { return { kind: 'fatal', error: new Error('The client is disabled') } } const serialized = JSON.stringify(payload) - const url = `${this.host}/i/v1/logs?token=${encodeURIComponent(this.apiKey)}` + const url = `${this.host}/i/v1/${path}?token=${encodeURIComponent(this.apiKey)}` const gzippedPayload = !this.disableCompression ? await this.compressPayload(serialized) : null const fetchOptions: PostHogFetchOptions = { @@ -1684,58 +1703,12 @@ export abstract class PostHogCoreStateless { } } - /** - * Sends a pre-built OTLP metrics payload to `/i/v1/metrics`. Same tagged - * outcome contract and error classification as `_sendLogsBatch` — this is - * the `MetricsHost._sendMetricsBatch` implementation, so `PostHogMetrics` - * can use any core-based SDK as its host. - */ - async _sendMetricsBatch(payload: OtlpMetricsPayload): Promise { - if (this.disabled) { - return { kind: 'fatal', error: new Error('The client is disabled') } - } - - const serialized = JSON.stringify(payload) - const url = `${this.host}/i/v1/metrics?token=${encodeURIComponent(this.apiKey)}` - - const gzippedPayload = !this.disableCompression ? await this.compressPayload(serialized) : null - const fetchOptions: PostHogFetchOptions = { - method: 'POST', - headers: { - ...this.getCustomHeaders(), - 'Content-Type': 'application/json', - ...(gzippedPayload !== null && { 'Content-Encoding': 'gzip' }), - }, - body: gzippedPayload || serialized, - } + async _sendLogsBatch(payload: OtlpLogsPayload): Promise { + return this._sendOtlpBatch({ path: 'logs', payload }) + } - try { - await this.fetchWithRetry( - url, - fetchOptions, - { type: 'successful-write' }, - { - retryCheck: (err) => { - if (isPostHogFetchContentTooLargeError(err)) { - return false - } - return isPostHogFetchRetryableError(err) - }, - } - ) - return { kind: 'ok' } - } catch (err) { - if (isPostHogFetchContentTooLargeError(err)) { - return { kind: 'too-large' } - } - // Exhausted retries on a retryable failure (network error, 408/429/5xx) - // still classify as retry-later so the window rides the next flush; only - // non-retryable HTTP errors (and 413 above) drop the batch. - if (isPostHogFetchRetryableError(err)) { - return { kind: 'retry-later', error: err } - } - return { kind: 'fatal', error: err } - } + async _sendMetricsBatch(payload: OtlpMetricsPayload): Promise { + return this._sendOtlpBatch({ path: 'metrics', payload }) } private fetchWithRetry( From 4c740beae4776b8f5b44a89f506a0409b912aaeb Mon Sep 17 00:00:00 2001 From: Anna Garcia Date: Mon, 24 Aug 2026 07:39:25 -0400 Subject: [PATCH 3/4] test(core): pin the shared OTLP retryCheck The classification cases run with retries disabled, so nothing exercised the predicate that keeps 413 out of the retry loop. --- .../core/src/__tests__/posthog.flush.spec.ts | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/packages/core/src/__tests__/posthog.flush.spec.ts b/packages/core/src/__tests__/posthog.flush.spec.ts index a1b6a33f81..32a2830c35 100644 --- a/packages/core/src/__tests__/posthog.flush.spec.ts +++ b/packages/core/src/__tests__/posthog.flush.spec.ts @@ -795,6 +795,31 @@ describe('PostHog Core', () => { expect(mocks.fetch).toHaveBeenCalledTimes(1) }) + // The classification cases run with retries off, so they never reach the + // `retryCheck` the senders share. This pins it: 413 has to leave the + // transport on the first response for the caller to shrink its batch and + // retry the same records, where a 5xx is worth re-sending as-is. + it.each([ + [500, 3], + [413, 1], + [400, 1], + ])('sends %i %i time(s) before returning', async (status, attempts) => { + jest.useRealTimers() + ;[posthog, mocks] = createTestClient('TEST_API_KEY', { + fetchRetryCount: 2, + fetchRetryDelay: 1, + preloadFeatureFlags: false, + }) + mocks.fetch.mockResolvedValue({ + status, + text: async () => 'err', + json: async () => ({ status: 'err' }), + }) + + await send(posthog) + expect(mocks.fetch).toHaveBeenCalledTimes(attempts) + }) + it('carries the HTTP error on a retry-later outcome', async () => { ;[posthog, mocks] = createTestClient('TEST_API_KEY', { fetchRetryCount: 0, From 1aad604bcae54366d6dbd1af6a0f3d4781076ac9 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 24 Aug 2026 11:42:28 +0000 Subject: [PATCH 4/4] Repair PR #4623: fix(core): send logs and metrics through one OTLP batch sender --- .../core/src/__tests__/posthog.flush.spec.ts | 3 ++ packages/core/src/logs/index.spec.ts | 31 +++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/packages/core/src/__tests__/posthog.flush.spec.ts b/packages/core/src/__tests__/posthog.flush.spec.ts index 32a2830c35..1372d6ff87 100644 --- a/packages/core/src/__tests__/posthog.flush.spec.ts +++ b/packages/core/src/__tests__/posthog.flush.spec.ts @@ -800,7 +800,10 @@ describe('PostHog Core', () => { // transport on the first response for the caller to shrink its batch and // retry the same records, where a 5xx is worth re-sending as-is. it.each([ + [408, 3], + [429, 3], [500, 3], + [503, 3], [413, 1], [400, 1], ])('sends %i %i time(s) before returning', async (status, attempts) => { diff --git a/packages/core/src/logs/index.spec.ts b/packages/core/src/logs/index.spec.ts index d8f6d0b399..f1500773fc 100644 --- a/packages/core/src/logs/index.spec.ts +++ b/packages/core/src/logs/index.spec.ts @@ -116,6 +116,37 @@ describe('PostHogLogs over the core sender', () => { expect(queueOf(client)).toHaveLength(1) }) + it('retains and resends records after transport retries are exhausted', async () => { + jest.useRealTimers() + const [client, mocks] = createTestClient('TEST_API_KEY', { + fetchRetryCount: 2, + fetchRetryDelay: 1, + preloadFeatureFlags: false, + }) + const unavailableResponse = { status: 503, text: async () => 'unavailable', json: async () => ({}) } + mocks.fetch + .mockResolvedValueOnce(unavailableResponse) + .mockResolvedValueOnce(unavailableResponse) + .mockResolvedValueOnce(unavailableResponse) + .mockResolvedValueOnce({ status: 200, text: async () => 'ok', json: async () => ({}) }) + const logs = new PostHogLogs( + client, + resolveForTest(), + createMockLogger(), + () => ({ distinctId: 'user-123' }), + immediateOnReady + ) + + logs.captureLog({ body: 'retry me' }) + await expect(logs.flush()).rejects.toHaveProperty('name', 'PostHogFetchHttpError') + expect(mocks.fetch).toHaveBeenCalledTimes(3) + expect(queueOf(client)).toHaveLength(1) + + await expect(logs.flush()).resolves.toBeUndefined() + expect(mocks.fetch).toHaveBeenCalledTimes(4) + expect(queueOf(client)).toHaveLength(0) + }) + it('drops the batch when the endpoint answers 401', async () => { const { logs, client } = createLogsOverCore(401) logs.captureLog({ body: 'unauthorized' })