From c26c4961f9e186136c7e204e16daa0f457ef4152 Mon Sep 17 00:00:00 2001 From: "posthog[bot]" <206114724+posthog[bot]@users.noreply.github.com> Date: Mon, 31 Aug 2026 20:26:55 +0000 Subject: [PATCH 1/4] fix(analytics): drop user-side transport errors at the capture site The Slack connect poll (and other callers) already degrade correctly when the user's own network drops a socket, then still call `analytics.captureException`. Each errno and host string fingerprints as a separate error tracking issue, so every one-off opens a fresh issue that buries real wizard bugs. Filter transport-level errno codes once at `captureException`, reusing the shape of `BENIGN_FS_ERROR_CODES` in `bounded-fs.ts`. Reads the error `code` first, then falls back to scanning the message so API errors that fold the errno into their text (and drop `code`) are caught too. Generated-By: PostHog Desktop Task-Id: 217e822f-fa35-4794-bb03-c40575e49608 --- src/utils/__tests__/analytics.test.ts | 50 +++++++++++++++++++++++++++ src/utils/analytics.ts | 44 +++++++++++++++++++++++ 2 files changed, 94 insertions(+) diff --git a/src/utils/__tests__/analytics.test.ts b/src/utils/__tests__/analytics.test.ts index c8943957b..cb5c46745 100644 --- a/src/utils/__tests__/analytics.test.ts +++ b/src/utils/__tests__/analytics.test.ts @@ -226,6 +226,56 @@ describe('Analytics', () => { }, ); }); + + it('drops a raw socket error carrying a transport errno code', () => { + const error = Object.assign(new Error('read ECONNRESET'), { + code: 'ECONNRESET', + }); + + analytics.captureException(error); + + expect(mockPostHogInstance.captureException).not.toHaveBeenCalled(); + }); + + it('drops a host-unreachable socket error', () => { + const error = Object.assign( + new Error('connect EHOSTUNREACH 1.2.3.4:443'), + { + code: 'EHOSTUNREACH', + }, + ); + + analytics.captureException(error); + + expect(mockPostHogInstance.captureException).not.toHaveBeenCalled(); + }); + + it('drops a wrapped API error that folds the errno into its message', () => { + // api.ts drops `code` and leaves the errno only in the message text. + const error = new Error('Failed to fetch user data (ECONNRESET)'); + + analytics.captureException(error); + + expect(mockPostHogInstance.captureException).not.toHaveBeenCalled(); + }); + + it('drops a filesystem timeout on a network-backed mount', () => { + const error = Object.assign(new Error('ETIMEDOUT: operation timed out'), { + code: 'ETIMEDOUT', + }); + + analytics.captureException(error); + + expect(mockPostHogInstance.captureException).not.toHaveBeenCalled(); + }); + + it('still captures a genuine wizard error', () => { + const error = new Error('Something the wizard did wrong'); + + analytics.captureException(error); + + expect(mockPostHogInstance.captureException).toHaveBeenCalledTimes(1); + }); }); describe('flag exposure', () => { diff --git a/src/utils/analytics.ts b/src/utils/analytics.ts index bea5214b6..e52509b03 100644 --- a/src/utils/analytics.ts +++ b/src/utils/analytics.ts @@ -68,6 +68,43 @@ export function groupsFromUser( return groups; } +/** + * Transport-level errno codes that mean the user's own network or machine + * dropped a connection mid-call, not that the wizard is broken. Every caller + * that hits these already degrades on its own — the Slack poll falls back to + * the connect nudge, a project-tree walk skips the entry, an API caller + * retries — so a capture adds only noise. And because each errno (and the + * host string Node folds into a raw socket message) fingerprints as its own + * error tracking issue, every one-off opens a fresh issue that buries real + * wizard bugs. Mirrors BENIGN_FS_ERROR_CODES in bounded-fs.ts. + */ +const BENIGN_TRANSPORT_ERROR_CODES: ReadonlySet = new Set([ + 'ECONNRESET', // connection reset by peer / socket dropped + 'ECONNREFUSED', // nothing listening at the far end + 'ETIMEDOUT', // connection or network-backed filesystem read timed out + 'EHOSTUNREACH', // no route to host + 'ENETUNREACH', // no route to network + 'ENETDOWN', // local network interface down + 'EPIPE', // wrote to a closed socket + 'EAI_AGAIN', // temporary DNS resolution failure +]); + +/** + * The benign transport errno for an error, or undefined. Reads the `code` + * field first (raw socket and filesystem errors carry it), then falls back to + * scanning the message — api.ts folds the errno into the ApiError message and + * drops `code`, so "(ECONNRESET)" in the text is the only trace left. + */ +function benignTransportCode(error: unknown): string | undefined { + const code = (error as NodeJS.ErrnoException | null)?.code; + if (code && BENIGN_TRANSPORT_ERROR_CODES.has(code)) return code; + const message = error instanceof Error ? error.message : ''; + for (const candidate of BENIGN_TRANSPORT_ERROR_CODES) { + if (message.includes(candidate)) return candidate; + } + return undefined; +} + const WIZARD_FLAGS: ReadonlySet = new Set(WIZARD_FLAG_KEYS); // Widen back to the SDK's shape — a filter on `true` never matches `'true'`. @@ -240,6 +277,13 @@ export class Analytics { } captureException(error: Error, properties: Record = {}) { + // Drop transport-level failures on the user's side. They never mean the + // wizard is broken and each variant opens its own error tracking issue. + const benign = benignTransportCode(error); + if (benign) { + logToFile(`[analytics] skipped benign transport error (${benign})`); + return; + } this.client.captureException(error, this.distinctId ?? this.anonymousId, { team: ANALYTICS_TEAM_TAG, ...this.tags, From 4b5b83240e654a29573dd11ad539410992b5487d Mon Sep 17 00:00:00 2001 From: "posthog[bot]" <206114724+posthog[bot]@users.noreply.github.com> Date: Mon, 31 Aug 2026 20:57:38 +0000 Subject: [PATCH 2/4] fix(analytics): match only the parenthesized errno wrapper when dropping transport noise MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit benignTransportCode's message fallback used message.includes(candidate), a bare substring match. Several install-failure reporters (Codex/Claude MCP add) wrap raw CLI stderr in a plain new Error(...) with no `code`, so the message scan is the only classifier that runs on them. If that stderr merely quoted a benign errno (e.g. a "retrying ECONNRESET" retry line) while the command actually failed for auth/permission/config reasons, the whole install failure was silently dropped from error tracking. Narrow the fallback to the exact "(CODE)" wrapper api.ts emits (`Failed to ... (ECONNRESET)`), which still matches the intended ApiError cases — including the CI re-wrap that appends text after the wrapper — but no longer fires on an errno embedded elsewhere in wrapped tool output. Add a negative test proving an install failure whose stderr mentions ECONNRESET still captures. Generated-By: PostHog Desktop Task-Id: 80a79146-2554-4271-88bc-894c3053ef14 --- src/utils/__tests__/analytics.test.ts | 13 +++++++++++++ src/utils/analytics.ts | 11 ++++++++--- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/utils/__tests__/analytics.test.ts b/src/utils/__tests__/analytics.test.ts index cb5c46745..12bab0b4c 100644 --- a/src/utils/__tests__/analytics.test.ts +++ b/src/utils/__tests__/analytics.test.ts @@ -259,6 +259,19 @@ describe('Analytics', () => { expect(mockPostHogInstance.captureException).not.toHaveBeenCalled(); }); + it('still captures an install failure whose embedded CLI stderr mentions an errno', () => { + // A wrapped tool failure that merely quotes a benign errno in its stderr + // must still report — the errno is not the "(ECONNRESET)" wrapper api.ts + // emits, so it does not mean the user's own transport dropped. + const error = new Error( + 'Codex MCP add failed: request failed ECONNRESET, retrying\npermission denied', + ); + + analytics.captureException(error); + + expect(mockPostHogInstance.captureException).toHaveBeenCalledTimes(1); + }); + it('drops a filesystem timeout on a network-backed mount', () => { const error = Object.assign(new Error('ETIMEDOUT: operation timed out'), { code: 'ETIMEDOUT', diff --git a/src/utils/analytics.ts b/src/utils/analytics.ts index e52509b03..cd5df1674 100644 --- a/src/utils/analytics.ts +++ b/src/utils/analytics.ts @@ -92,15 +92,20 @@ const BENIGN_TRANSPORT_ERROR_CODES: ReadonlySet = new Set([ /** * The benign transport errno for an error, or undefined. Reads the `code` * field first (raw socket and filesystem errors carry it), then falls back to - * scanning the message — api.ts folds the errno into the ApiError message and - * drops `code`, so "(ECONNRESET)" in the text is the only trace left. + * the message — api.ts folds the errno into the ApiError message and drops + * `code`, so the parenthesized "(ECONNRESET)" wrapper is the only trace left. + * The fallback matches only that wrapper, never a bare mention: several callers + * wrap raw CLI stderr in a `new Error(...)` when an install fails, and that + * output can quote a benign errno (a "retrying ECONNRESET" log line) while the + * command actually failed for an unrelated reason. A bare substring match would + * silently drop those install failures — a class the team wants to see. */ function benignTransportCode(error: unknown): string | undefined { const code = (error as NodeJS.ErrnoException | null)?.code; if (code && BENIGN_TRANSPORT_ERROR_CODES.has(code)) return code; const message = error instanceof Error ? error.message : ''; for (const candidate of BENIGN_TRANSPORT_ERROR_CODES) { - if (message.includes(candidate)) return candidate; + if (message.includes(`(${candidate})`)) return candidate; } return undefined; } From 01752747ade2c52acd9055c25d52db24a38e7230 Mon Sep 17 00:00:00 2001 From: "posthog[bot]" <206114724+posthog[bot]@users.noreply.github.com> Date: Mon, 31 Aug 2026 21:00:11 +0000 Subject: [PATCH 3/4] fix(analytics): keep operation context in the dropped-transport-error log line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The skip log for a benign transport error wrote only the errno. Since these failures are deliberately withheld from error tracking, that debug line is the only surviving record — and it's a user-facing support artifact (printed as "Full logs: " on failure and rendered in the RunScreen LogViewer). A bare "(ETIMEDOUT)" can't be attributed to the operation that produced it, whether a Slack poll, a project-tree read, or a doctor fetch. Include the operation context (properties.step / properties.source) and error.message alongside the errno, mirroring bounded-fs's skip log that this PR already cites as precedent. Callers redact secrets from these fields before reporting, and only step/source are read from properties, so no sensitive values reach the log. Generated-By: PostHog Desktop Task-Id: 80a79146-2554-4271-88bc-894c3053ef14 --- src/utils/analytics.ts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/utils/analytics.ts b/src/utils/analytics.ts index cd5df1674..64204c7e4 100644 --- a/src/utils/analytics.ts +++ b/src/utils/analytics.ts @@ -286,7 +286,17 @@ export class Analytics { // wizard is broken and each variant opens its own error tracking issue. const benign = benignTransportCode(error); if (benign) { - logToFile(`[analytics] skipped benign transport error (${benign})`); + // This debug line is the only record of a dropped failure, and the same + // errno can come from unrelated operations (a Slack poll, a project-tree + // read, a doctor fetch). Keep the operation context (step/source) and the + // message so support can name what failed — callers already redact + // secrets from these before reporting. Mirrors bounded-fs's skip log. + const op = properties.step ?? properties.source; + logToFile( + `[analytics] skipped benign transport error (${benign})${ + op ? ` [${String(op)}]` : '' + }: ${error.message}`, + ); return; } this.client.captureException(error, this.distinctId ?? this.anonymousId, { From 4fa7bb898a456da160bcbffbe5d7cda93ac2ec84 Mon Sep 17 00:00:00 2001 From: "posthog[bot]" <206114724+posthog[bot]@users.noreply.github.com> Date: Mon, 31 Aug 2026 21:10:08 +0000 Subject: [PATCH 4/4] fix(analytics): quiet ENOTFOUND DNS failures like other transport drops MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ENOTFOUND is Node/Axios's DNS-lookup-failed errno — the code a machine that is offline or behind a captive portal returns most often. It sits in the same user-side transport-failure class as EAI_AGAIN (temporary DNS, already allowlisted) and ECONNREFUSED, but was missing from BENIGN_TRANSPORT_ERROR_CODES, so a wizard user with no working DNS still opened a "Failed to fetch user data (ENOTFOUND)" error-tracking issue per operation — the exact noise this PR removes for ECONNRESET/ETIMEDOUT. handleApiError folds the errno into the ApiError message and drops `code`, so the "(ENOTFOUND)" wrapper is the only trace; the existing message scan in benignTransportCode now catches it. Adds a regression test that routes an ENOTFOUND AxiosError through handleApiError into captureException and asserts it is not reported. Generated-By: PostHog Desktop Task-Id: e271ea17-9e5a-4d29-93f1-74d5be2bd974 --- src/utils/__tests__/analytics.test.ts | 17 ++++++++++++++++- src/utils/analytics.ts | 1 + 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/utils/__tests__/analytics.test.ts b/src/utils/__tests__/analytics.test.ts index 12bab0b4c..8db5aff26 100644 --- a/src/utils/__tests__/analytics.test.ts +++ b/src/utils/__tests__/analytics.test.ts @@ -1,9 +1,10 @@ import { Analytics, groupsFromUser } from '@utils/analytics'; import { PostHog } from 'posthog-node'; +import { AxiosError } from 'axios'; import { v4 as uuidv4 } from 'uuid'; import { ANALYTICS_TEAM_TAG, WIZARD_FLAG_KEYS } from '@lib/constants'; import { VERSION } from '@lib/version'; -import type { ApiUser } from '@lib/api'; +import { handleApiError, type ApiUser } from '@lib/api'; vi.mock('posthog-node'); vi.mock('uuid'); @@ -272,6 +273,20 @@ describe('Analytics', () => { expect(mockPostHogInstance.captureException).toHaveBeenCalledTimes(1); }); + it('drops an ENOTFOUND ApiError produced by handleApiError (DNS lookup failure)', () => { + // A user with no working DNS. api.ts folds the errno into the message and + // drops `code`, so the "(ENOTFOUND)" wrapper is the only trace — the same + // path the message scan handles. Must not open an error tracking issue. + const axiosError = new AxiosError('connect error'); + axiosError.config = { url: '/api/users/@me/' } as never; + axiosError.code = 'ENOTFOUND'; + const apiError = handleApiError(axiosError, 'fetch user data'); + + analytics.captureException(apiError); + + expect(mockPostHogInstance.captureException).not.toHaveBeenCalled(); + }); + it('drops a filesystem timeout on a network-backed mount', () => { const error = Object.assign(new Error('ETIMEDOUT: operation timed out'), { code: 'ETIMEDOUT', diff --git a/src/utils/analytics.ts b/src/utils/analytics.ts index 64204c7e4..b5e4b4779 100644 --- a/src/utils/analytics.ts +++ b/src/utils/analytics.ts @@ -87,6 +87,7 @@ const BENIGN_TRANSPORT_ERROR_CODES: ReadonlySet = new Set([ 'ENETDOWN', // local network interface down 'EPIPE', // wrote to a closed socket 'EAI_AGAIN', // temporary DNS resolution failure + 'ENOTFOUND', // DNS lookup failed — host not found (offline / captive portal) ]); /**