diff --git a/src/utils/__tests__/analytics.test.ts b/src/utils/__tests__/analytics.test.ts index c8943957b..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'); @@ -226,6 +227,83 @@ 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('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 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', + }); + + 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..b5e4b4779 100644 --- a/src/utils/analytics.ts +++ b/src/utils/analytics.ts @@ -68,6 +68,49 @@ 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 + 'ENOTFOUND', // DNS lookup failed — host not found (offline / captive portal) +]); + +/** + * 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 + * 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; + } + 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 +283,23 @@ 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) { + // 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, { team: ANALYTICS_TEAM_TAG, ...this.tags,