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 8 commits into
Conversation
Supabase CLI previewnpx --yes https://pkg.pr.new/supabase/cli/supabase@8bec921a428a067c1621fd5b4b54474f80841eb2Preview package for commit |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
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". |
| export const scopedPosthogClient = (apiKey: string, host: string) => | ||
| Effect.acquireRelease( | ||
| Effect.sync( | ||
| () => | ||
| new PostHog(apiKey, { | ||
| host, | ||
| flushAt: 1, | ||
| flushInterval: 0, | ||
| requestTimeout: 2_000, | ||
| fetch: fireAndForgetFetch, | ||
| }), | ||
| ), | ||
| // Catch on the promise: Effect.promise turns rejections into defects, | ||
| // which escape Effect.ignore and fail the command. | ||
| (client) => Effect.promise(() => client._shutdown(5_000).catch(() => undefined)), |
There was a problem hiding this comment.
issue one event can be in flight while a later exit event remains queued. Shutdown then performs two sequential 2-second requests (eg: shutdownMs: 3987 with two captures against the pinned SDK). Apply the deadline to the entire shutdown and add a two-event blackhole test.
The existing port-9 test rejects immediately and misses this case
2 participants
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. A 2srequestTimeoutbounds how long a blackholed connection can hold exit.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.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