fix(analytics): stop reporting user-network failures as wizard exceptions - #1023
Draft
posthog[bot] wants to merge 1 commit into
Draft
fix(analytics): stop reporting user-network failures as wizard exceptions#1023posthog[bot] wants to merge 1 commit into
posthog[bot] wants to merge 1 commit into
Conversation
…ions A machine behind a TLS-intercepting corporate proxy, or one missing an intermediate CA, fails every HTTPS request before it reaches PostHog. The Slack connectivity poll already degrades to its nudge copy when that happens, but it still reported the throw to error tracking — so a user's certificate problem arrived as a wizard exception we cannot act on. Adds `isBenignNetworkError` / `isTlsTrustError` in `src/utils/network-errors.ts`, following the `BENIGN_FS_ERROR_CODES` precedent in `bounded-fs.ts`: recognise the expected-and-handled failures, log them to the debug file, and keep error tracking for things we can act on. Transport-only by construction — an HTTP response the server actually sent (401, 403, 5xx) is never matched, so real API problems keep flowing through. Applied to the two call sites that already degrade gracefully: the Slack connectivity poll and flag evaluation. `handleApiError` also grows a certificate branch. A certificate rejection is an AxiosError with no `response`, so it used to fall through to a bare "Failed to <operation>" and strip the one detail the user can act on; it now names `--use-system-ca` and `NODE_EXTRA_CA_CERTS`. Generated-By: PostHog Code Task-Id: d186e757-b6f5-4e55-bc2b-e6f62eec1d4b
🧙 Wizard CIRun the Wizard CI and test your changes against wizard-workbench example apps by replying with a GitHub comment using one of the following commands: Test all apps:
Test all apps in a directory:
Test an individual app:
Show more apps
Results will be posted here when complete. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Users on TLS-intercepting corporate networks turn their own certificate problems into our error tracking noise.
A machine behind an intercepting proxy — or simply missing an intermediate CA — fails every HTTPS request before it reaches PostHog. The Slack connectivity poll in
SlackConnectScreenalready degrades to its nudge copy when that happens, and the run continues fine. But it still reported the throw to error tracking, so a user's certificate problem arrived as a wizard exception nobody can act on. The wizard also gave that user a dead end:handleApiErrorsees a certificate rejection as anAxiosErrorwith noresponse, so it fell through to a bare"Failed to fetch user data"and stripped the one detail they could act on.The wizard already does the right thing for the filesystem equivalent —
BENIGN_FS_ERROR_CODESinbounded-fs.tslogs expected errno codes instead of capturing them, after per-entryEPERM/EACCESnoise buried real errors. There was no network counterpart.Changes
src/utils/network-errors.ts(new) —isBenignNetworkError/isTlsTrustError, pure predicates that walk thecausechain (undici reports every transport failure as an opaqueTypeError: fetch failedand hangs the real reason offcause). Covers OpenSSL trust codes, transport errno codes, axiosERR_NETWORK, and undici'sSocketError/ConnectTimeoutErrorfamily. Transport-only by construction: an HTTP response the server actually sent (401, 403, 5xx) is never matched, so real API problems keep flowing to error tracking.analytics.ts(which already continues on defaults). Everything else is untouched; paths where a network failure is a hard stop still report.handleApiErrornow has a certificate branch that surfaces Node's own--use-system-cahint plus theNODE_EXTRA_CA_CERTSroute, so people on intercepting proxies learn how to fix their environment.Nothing about the fallback behaviour changes — the suppressed errors were already handled.
Note this is one of two surfaces; the larger share of this noise comes from the MCP server's
StateManagercatch-all inPostHog/posthog, which needs the same treatment and ships separately.Test plan
pnpm build && pnpm test && pnpm lint— 1697 tests pass, lint clean (492 pre-existing warnings, zero new).Verified against a real TLS rejection rather than hand-built errors, since the open question was whether axios preserves the code and message through its wrapper. Ran the wizard's own
fetchSlackConnectedagainst a local HTTPS server with a genuine self-signed cert:code: DEPTH_ZERO_SELF_SIGNED_CERTwith a message byte-identical to the one in the reported error-tracking issue, andname: Error(axios propagates the raw TLS error rather than wrapping it — which is why the new branch sits above theaxios.isAxiosErrorchecks).handleApiErrorreturned the actionable hint instead of"Failed to fetch user data".Then drove the real
SlackConnectScreencomponent against that server:step: 'slack_connected_check'.New regression tests:
network-errors.test.ts— 30 cases, including the verbatim production error string and the negative cases (401 responses, ordinaryTypeErrors, fs errnos, cycliccausechains).slack-connect-error-reporting.test.tsx— renders the real screen and locks both halves: an unreachable network stays out of error tracking, a 401 still reports. Mutation-checked — disabling the guard fails the first test and leaves the second passing, so the harness is genuinely sensitive.api-error-handling.test.ts— the certificate hint, plus 401/403/404 messages unchanged.Created with PostHog Desktop from this inbox report.