diff --git a/cli/src/channel/currentBundle.ts b/cli/src/channel/currentBundle.ts index ddf352ef56..10cc13ab96 100644 --- a/cli/src/channel/currentBundle.ts +++ b/cli/src/channel/currentBundle.ts @@ -2,6 +2,7 @@ import type { ChannelCurrentBundleOptions } from '../schemas/channel' import { intro, log } from '@clack/prompts' import { trackEvent, withSupabaseSource } from '../analytics/track' import { check2FAComplianceForApp } from '../api/app' +import { CliUserError } from '../shared/cli-user-error' import { createSupabaseClient, findSavedKey, @@ -33,13 +34,13 @@ export async function currentBundleInternal(channel: string, appId: string, opti if (!options.apikey) { if (!silent) log.error('Missing API key, you need to provide an API key to upload your bundle') - throw new Error('Missing API key') + throw new CliUserError('Missing API key') } if (!appId) { if (!silent) log.error('Missing argument, you need to provide a appId, or be in a capacitor project') - throw new Error('Missing appId') + throw new CliUserError('Missing appId') } const supabase = await createSupabaseClient(options.apikey, options.supaHost, options.supaAnon) @@ -49,7 +50,7 @@ export async function currentBundleInternal(channel: string, appId: string, opti if (!channel) { if (!silent) log.error('Please provide a channel to get the bundle from.') - throw new Error('Channel name missing') + throw new CliUserError('Channel name missing') } const { data: supabaseChannel, error } = await withSupabaseSource('channels.currentBundle', () => supabase @@ -62,15 +63,14 @@ export async function currentBundleInternal(channel: string, appId: string, opti if (error || !supabaseChannel?.length) { if (!silent) log.error(`Error retrieving channel ${channel} for app ${appId}. Perhaps the channel does not exist?`) - throw new Error(`Channel ${channel} not found for app ${appId}`) + throw new CliUserError('Channel not found for app', { appId, channel }) } const { id: channelId, version } = supabaseChannel[0] as Channel if (!(await hasCliPermission(supabase, options.apikey, 'channel.read', { appId, channelId }))) { - const msg = `Insufficient permissions for channel ${channel}. Required RBAC permission for this action: channel.read.` if (!silent) - log.error(msg) - throw new Error(msg) + log.error(`Insufficient permissions for channel ${channel}. Required RBAC permission for this action: channel.read.`) + throw new CliUserError('Insufficient permissions for channel. Required RBAC permission for this action: channel.read.', { appId, channel }) } void trackEvent({ channel: 'channel', event: 'Channel Current Bundle Viewed', icon: '📦', tags: { has_bundle: Boolean(version) } }) @@ -78,7 +78,7 @@ export async function currentBundleInternal(channel: string, appId: string, opti if (!version) { if (!silent) log.error(`Error retrieving channel ${channel} for app ${appId}. Perhaps the channel does not exist?`) - throw new Error(`Channel ${channel} does not have a bundle linked`) + throw new CliUserError('Channel does not have a bundle linked', { appId, channel }) } const { data: bundleRows, error: bundleError } = await withSupabaseSource('channels.currentBundleName', () => supabase @@ -91,7 +91,7 @@ export async function currentBundleInternal(channel: string, appId: string, opti if (bundleError || !bundleName) { if (!silent) log.error(`Error retrieving current bundle for channel ${channel}.`) - throw new Error(`Channel ${channel} does not have a readable current bundle`) + throw new CliUserError('Channel does not have a readable current bundle', { appId, channel }) } if (!silent) { diff --git a/cli/src/index.ts b/cli/src/index.ts index eb2b5f2ee2..dde40c070b 100644 --- a/cli/src/index.ts +++ b/cli/src/index.ts @@ -1323,12 +1323,14 @@ void (async () => { await Promise.all([capturePromise, flushAnalytics(), finishActiveCliReplay().catch(() => {})]) exit(exitCode) } - const capturePromise = capturePosthogException({ - error, - functionName: currentCommandPath, - kind: 'unhandled_error', - status: 1, - }) + const capturePromise = shouldCapturePosthogException(error) + ? capturePosthogException({ + error, + functionName: currentCommandPath, + kind: 'unhandled_error', + status: 1, + }) + : Promise.resolve(false) // For non-Commander errors, show full error details log.error(`Error: ${formatError(error)}`) trackCommandFailed(currentCommandPath, { errorCategory: categorizeCliError(error), exitCode: 1 }) diff --git a/cli/src/posthog.ts b/cli/src/posthog.ts index 27d705d8f8..d31c50a1ed 100644 --- a/cli/src/posthog.ts +++ b/cli/src/posthog.ts @@ -2,6 +2,7 @@ import type { Command } from 'commander' import { homedir, platform, release } from 'node:os' import { arch, cwd, env, version as nodeVersion } from 'node:process' import pack from '../package.json' +import { CliUserError } from './shared/cli-user-error' const POSTHOG_EXCEPTION_URL = 'https://eu.i.posthog.com/i/v0/e/' const CAPGO_POSTHOG_PROJECT_TOKEN = 'phc_NXDyDajQaTQVwb25DEhIVZfxVUn4R0Y348Z7vWYHZUi' @@ -177,6 +178,11 @@ function getCommanderCode(error: unknown) { } export function shouldCapturePosthogException(error: unknown) { + // Expected user-facing failures (missing input, a channel with no bundle, + // insufficient permissions, …) are legitimate states, not crashes — never + // open an error tracking issue for them. + if (error instanceof CliUserError) + return false return !getCommanderCode(error)?.startsWith('commander.') } diff --git a/cli/src/shared/cli-user-error.ts b/cli/src/shared/cli-user-error.ts new file mode 100644 index 0000000000..6167c99595 --- /dev/null +++ b/cli/src/shared/cli-user-error.ts @@ -0,0 +1,24 @@ +/** + * Marker for *expected* CLI failures that represent a legitimate user-facing + * state — missing input, a channel with no bundle linked, insufficient + * permissions — rather than a crash. The CLI still prints a clear message and + * exits non-zero, but `shouldCapturePosthogException` skips these so they never + * open an error tracking `$exception` issue. They are still counted via + * `trackCommandFailed` / `categorizeCliError`, so failure analytics stay intact. + * + * Keep any dynamic identifier (e.g. a channel name) OUT of the message and pass + * it via `context` instead: interpolating it into the message makes error + * tracking fingerprint a separate issue per value, which is exactly the noise + * this class exists to avoid. + * + * Precedent for domain-specific error markers already exists in the CLI with + * `MacOSSigningError` and `BuildRecordReadError`. + */ +export class CliUserError extends Error { + readonly context?: Record + constructor(message: string, context?: Record) { + super(message) + this.name = 'CliUserError' + this.context = context + } +} diff --git a/cli/test/test-posthog-exception.mjs b/cli/test/test-posthog-exception.mjs index 828d775fb1..1cf44e28f6 100644 --- a/cli/test/test-posthog-exception.mjs +++ b/cli/test/test-posthog-exception.mjs @@ -7,6 +7,7 @@ import { getCommandPath, shouldCapturePosthogException, } from '../src/posthog.ts' +import { CliUserError } from '../src/shared/cli-user-error.ts' const originalFetch = globalThis.fetch const originalEnv = { @@ -132,6 +133,17 @@ try { assert.equal(shouldCapturePosthogException({ code: 'ENOENT' }), true) assert.equal(shouldCapturePosthogException(new Error('boom')), true) + // Expected user-facing CLI failures must never open an error tracking issue, + // regardless of the (dynamic) channel context attached to them. + assert.equal(shouldCapturePosthogException(new CliUserError('Channel does not have a bundle linked', { appId: 'com.example.app', channel: 'production' })), false) + assert.equal(shouldCapturePosthogException(new CliUserError('Missing API key')), false) + // Two failures on different channels must be treated identically (one issue, + // not one per channel), since the channel name lives in context, not the message. + assert.equal( + new CliUserError('Channel does not have a bundle linked', { channel: 'production' }).message, + new CliUserError('Channel does not have a bundle linked', { channel: 'canary' }).message, + ) + console.log('CLI PostHog exception capture tests passed') } finally {