Skip to content

Keep PostHog off the request path; point backend at PostHog US - #574

Open
kcarnold wants to merge 1 commit into
mainfrom
fix/dont-crash-on-posthog
Open

Keep PostHog off the request path; point backend at PostHog US#574
kcarnold wants to merge 1 commit into
mainfrom
fix/dont-crash-on-posthog

Conversation

@kcarnold

Copy link
Copy Markdown
Contributor

A transient 526 from POSTHOG_HOST (the e.thoughtful-ai.com reverse proxy) brought the backend down and failed a deploy. The 526 was the trigger; the cause is that telemetry sat on the request path.

What actually happened

posthogMiddleware did await posthog.flush() after every request, mounted on '*':

  1. Telemetry decided the response status. flush() rejects on any non-2xx from the host. That rejection propagated out of the middleware into app.onError, which returns a 500 — for a request that had already succeeded.
  2. It covered everything/api/ping, the Better Auth routes, the OpenAI proxy, and the static frontend catch-all. A total outage of the container, not a degraded analytics path. The readiness probe could never pass, so the rollout failed.
  3. It hung, too. posthog-node's defaults are requestTimeout: 10s x 4 attempts with 3s between (a 526 passes retryCheck), so ~9s minimum and ~49s worst case per request. Worse, flush() serializes behind the previous flushPromise, so with N concurrent requests the Nth waited on all N−1 preceding failed flushes.
  4. Doubled on the error path. onErrorcaptureException → which also awaited flush(), chained behind the one that had just failed.

Not a crash loop: flushBackground() catches its own errors and captureException had a try/catch, so nothing could take the process down via an unhandled rejection. The blast radius was 500s and hangs.

Changes

  • Dropped both await flush() calls (middleware and captureException). Ingestion rides posthog-node's background batching (20 events or 10s), which swallows its own errors; shutdownPosthog() still drains the tail on SIGTERM, so nothing is lost on a normal rollout.
  • Capped the client: requestTimeout: 3000, fetchRetryCount: 1, fetchRetryDelay: 1000. A dead host now costs seconds of background time.
  • /api/ping is no longer captured — health probes aren't product analytics, and they queued an event every few seconds forever.
  • POSTHOG_HOST defaults to https://us.i.posthog.com, in code and in both compose files. The proxy earns its keep for the browser SDK (ad blockers); server-side it's pure added failure surface. The frontend and the experiment service are unchanged.
  • New backend/src/__tests__/posthog.test.ts: runs a real local HTTP server as a hostile PostHog (526, plus a variant that accepts and never responds) and asserts requests stay 200 and fast. Verified non-vacuous — restoring await posthog.flush() fails 3 of its 4 tests. It uses a real server rather than a mocked fetch because posthog-node resolves the global fetch once at its own module load, outside Vitest's module registry, so vi.stubGlobal doesn't reliably reach it.

Deploy note

⚠️ The k8s deployment sets POSTHOG_HOST explicitly, so the new default does nothing until that env var is removed there (or set to https://us.i.posthog.com). Nothing in this PR changes the deployed value.

Follow-ups not done here

  • The middleware captures ${method} ${path} as the event name, so with the static catch-all every asset URL mints a distinct event — high-cardinality noise plus ingestion cost. Narrowing to /api/*, or capturing the matched route pattern, would help; it changes the analytics data, so it's a separate call.
  • Worth deciding whether the health probe should bypass middleware entirely rather than relying on each middleware to be well-behaved.

Tests: 110 passed, tsc --noEmit clean.

🤖 Generated with Claude Code

A transient 526 from the analytics reverse proxy took the whole backend
down and failed a deploy. posthogMiddleware awaited posthog.flush() after
every request, on every route ('*'), and flush() rejects on any non-2xx —
so an ingestion failure became a 500 from a request that had already
succeeded, health probe and static frontend included.

It also hung: posthog-node's defaults are a 10s timeout x 4 attempts with
3s between them, and flush() serializes behind the previous flush, so
concurrent requests queued up multiplicatively. The onError path then paid
it a second time via captureException's own await flush().

Ingestion now rides posthog-node's background batching, which swallows its
own errors; shutdownPosthog() still drains the tail on SIGTERM. Retries and
timeouts are capped so a dead host costs seconds, not ~49s. /api/ping is no
longer captured — health probes aren't product analytics.

POSTHOG_HOST now defaults to PostHog US directly. The e.thoughtful-ai.com
proxy exists to get the browser SDK past ad blockers; server-side it only
adds a hop that can fail. The frontend and experiment app still use it.
NOTE: the deployment sets POSTHOG_HOST explicitly, so it must be removed
there for this default to take effect.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant