fix(cli): stop telemetry hangs and errors on blocked networks#5880
fix(cli): stop telemetry hangs and errors on blocked networks#5880pamelachia wants to merge 9 commits into
Conversation
Supabase CLI previewnpx --yes https://pkg.pr.new/supabase/cli/supabase@878576ba751d060a0c566956cc8df3e363c31d4aPreview package for commit |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a96c6f5826
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e38cb95741
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Summary
On networks that block or blackhole
eu.posthog.com(corporate proxies, CI and Docker egress policies, air-gapped machines), CLI telemetry currently breaks the CLI itself. Reproduced on stable v2.109.1 with the PostHog host pointed at an unreachable address: every command pays a 5s trailing hang and then exits 1 with{"code":"UnknownError","message":"Timeout while shutting down PostHog. Some events may not have been sent."}, even though the command's real work succeeded. Long-running commands additionally print raw SDK stack traces to stderr, and the Go binary's shutdown wait is unbounded. The only workaround issupabase telemetry disable, which permanently removes those users from CLI analytics. This PR makes telemetry strictly fire-and-forget: a failed delivery never changes a command's output, exit code, or runtime beyond a ~2s cap.Changes
ShutdownTimeouton the PostHog client (the SDK default waits indefinitely for the final flush, including retries) and route SDK logs to the--debuglogger instead of unconditional stderr.scopedPosthogClientfactory inshared/telemetry/posthog-client.ts, used by bothanalytics.layer.tsandlegacy-analytics.layer.ts. It wrapsfetchto report delivery failures as delivered: posthog-node has no logger hook and prints failures through hardcodedconsole.errorcalls, so making the transport infallible from the SDK's perspective is the one supported way to get no retries, no stderr output, and no shutdown stall.EXIT_DELAY_CAP_MS) bounds both each request (requestTimeout) and the entire shutdown:_shutdownreceives the same value, which the SDK races against the whole drain (the in-flight request plus the queued flush loop). The previous 5s shutdown deadline left room for two sequential 2s request timeouts, so a command with one event in flight and a second event queued exited in ~4s instead of ~2s. Covered by a two-event blackhole regression test that holds the first request in flight and asserts total shutdown stays under the cap (~4.0s before, ~2.0s after).Effect.promise(() => client._shutdown(5_000)).pipe(Effect.ignore), butEffect.promiseturns a promise rejection into a defect andEffect.ignoreonly swallows typed failures, so a shutdown timeout failed the whole command. The rejection is now caught on the promise itself, so telemetry shutdown can never fail a command.Context for review
flushAt: 1, flushInterval: 0is kept deliberately. For a short-lived CLI process, eager flush overlaps the send with command execution; batching would move the network wait to process exit where the user feels it.@posthog/coreis debug-gated (only active after an explicit.debug()call, which the CLI never makes), and the ungatedconsole.errorpath (logFlushError) is unreachable because the wrapped fetch never throws.telemetry statusin 5s with exit 1 and the UnknownError payload; this branch runs it instantly with exit 0 and empty stderr. Blackholed (non-routable address): GoClose()returns in 2.0s (previously unbounded), TS capture plus shutdown completes in 2.0s with zero stderr bytes.--debugon the Go side. On the TS side they are dropped silently.Linear