fix(core): consume response body in _flush() and _capture() to prevent CF Workers warnings#3211
Conversation
|
@jgarrison929 is attempting to deploy a commit to the PostHog Team on Vercel. A member of the Team first needs to authorize it. |
|
…t Cloudflare Workers warnings
`_flush()` and the single-event `_capture()` path both call
`fetchWithRetry()` but discard the returned response without reading
the body. In runtimes like Cloudflare Workers that enforce response
body consumption, this causes cross-request promise resolution warnings
and may silently cancel post-flush error handling continuations.
Consume the response body with `res.text().catch(() => {})` at both
call sites on the success path. Also consume the response body held by
`PostHogFetchHttpError` on error paths (e.g. 413 batch-size reduction)
to prevent the same leak on failed requests.
The `fetchWithRetry()` return type is preserved so callers that need
the response (e.g. surveys API) can still read it.
Fixes PostHog#3173
45b2a56 to
f7036c0
Compare
|
Thanks for the review! The latest force-push ( The approach of eagerly consuming the body at |
|
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the |
|
Still active — this fixes a real Cloudflare Workers deployment issue where unconsumed response bodies cause warnings/errors. Happy to address any maintainer feedback. |
Problem
_flush()and the single-event capture path inposthog-core-stateless.tsboth callfetchWithRetry()but discard the returned response without reading the body.In Cloudflare Workers (and potentially other edge runtimes), an unconsumed response body forces the runtime to clean up the underlying connection later — potentially in a different request's context — which triggers:
The runtime may also cancel continuations for the leaked promises, so post-flush error handling may silently not run.
This is the same class of bug the Sentry SDK had and fixed in getsentry/sentry-javascript#18534.
Fix
Consume the response body at all sites where it was previously discarded:
Success paths
_flush()(~line 1142) — the batched event flush path_capture()(~line 967) — the single-event capture path (used whenflushAt: 1)Error paths
catchblocks now also consume the response body held byPostHogFetchHttpError. This covers the 413 batch-size reduction path and other HTTP error paths where the error response was previously discarded.The
fetchWithRetry()return type is unchanged so callers that need the response body (e.g., the surveys API endpoint) can still read it normally.Note: Intermediate retry attempts inside
retriable()may still produce unconsumed error responses during the retry loop. This is a separate concern in theretriableutility and would be best addressed separately if needed.Changes
packages/core/src/posthog-core-stateless.ts— 4 sites updated (+19 lines, -2 lines)Fixes #3173