From a4c5db8aa95e3a4f3b6ea08f4bc319344bacf7b3 Mon Sep 17 00:00:00 2001 From: jdalton Date: Fri, 17 Apr 2026 21:27:02 -0400 Subject: [PATCH 1/6] refactor(error): route uncaught CLI errors through typed-error display MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The typed-error classes in `utils/error/errors.mts` (AuthError, InputError, ConfigError, NetworkError, FileSystemError, RateLimitError) plus the matching `formatErrorForDisplay`/`formatErrorForTerminal`/`formatErrorForJson` helpers were only ever exercised by tests — the CLI entry point used its own inline `if/else` dispatch that handled AuthError/InputError/Error. - Replace the inline dispatch in `cli-entry.mts` with calls to `formatErrorForTerminal` and `formatErrorForJson`, so any typed error now thrown in production gets consistent titles, recovery suggestions, and JSON/text formatting. - Convert `queryApi`'s "Socket API base URL is not configured" throw from a generic Error to a ConfigError keyed on CONFIG_KEY_API_BASE_URL. Users now see recovery suggestions directing them to `socket config set` instead of a bare stack trace. The remaining typed classes (NetworkError, FileSystemError, RateLimitError) stay parked. Forcing them into current CResult-returning sites would be a larger refactor and isn't needed for the display wiring itself to work — they slot in automatically the moment any throw site adopts them. --- packages/cli/src/cli-entry.mts | 42 +++++---------------------- packages/cli/src/utils/socket/api.mts | 6 +++- 2 files changed, 12 insertions(+), 36 deletions(-) diff --git a/packages/cli/src/cli-entry.mts b/packages/cli/src/cli-entry.mts index eba364335..309b4a565 100755 --- a/packages/cli/src/cli-entry.mts +++ b/packages/cli/src/cli-entry.mts @@ -28,7 +28,6 @@ process.emitWarning = function (warning, ...args) { return Reflect.apply(originalEmitWarning, this, [warning, ...args]) } -import { messageWithCauses, stackWithCauses } from 'pony-cause' import lookupRegistryAuthToken from 'registry-auth-token' import lookupRegistryUrl from 'registry-url' @@ -53,11 +52,10 @@ import { VITEST } from './env/vitest.mts' import meow from './meow.mts' import { meowWithSubcommands } from './utils/cli/with-subcommands.mts' import { - AuthError, - captureException, - InputError, -} from './utils/error/errors.mts' -import { failMsgWithBadge } from './utils/error/fail-msg-with-badge.mts' + formatErrorForJson, + formatErrorForTerminal, +} from './utils/error/display.mts' +import { captureException } from './utils/error/errors.mts' import { serializeResultJson } from './utils/output/result-json.mts' import { runPreflightDownloads } from './utils/preflight/downloads.mts' import { isSeaBinary } from './utils/sea/detect.mts' @@ -181,24 +179,6 @@ void (async () => { debug('CLI uncaught error') debugDir(e) - let errorBody: string | undefined - let errorTitle: string - let errorMessage = '' - if (e instanceof AuthError) { - errorTitle = 'Authentication error' - errorMessage = e.message - } else if (e instanceof InputError) { - errorTitle = 'Invalid input' - errorMessage = e.message - errorBody = e.body - } else if (e instanceof Error) { - errorTitle = 'Unexpected error' - errorMessage = messageWithCauses(e) - errorBody = stackWithCauses(e) - } else { - errorTitle = 'Unexpected error with no details' - } - // Try to parse the flags, find out if --json is set. const isJson = (() => { const cli = meow({ @@ -213,20 +193,12 @@ void (async () => { })() if (isJson) { - logger.log( - serializeResultJson({ - ok: false, - message: errorTitle, - cause: errorMessage, - }), - ) + logger.log(serializeResultJson(formatErrorForJson(e))) } else { // Add 2 newlines in stderr to bump below any spinner. logger.error('\n') - logger.fail(failMsgWithBadge(errorTitle, errorMessage)) - if (errorBody) { - debugDirNs('inspect', { errorBody }) - } + logger.error(formatErrorForTerminal(e)) + debugDirNs('inspect', { error: e }) } await captureException(e) diff --git a/packages/cli/src/utils/socket/api.mts b/packages/cli/src/utils/socket/api.mts index 15b8a0ed4..a94a1adaf 100644 --- a/packages/cli/src/utils/socket/api.mts +++ b/packages/cli/src/utils/socket/api.mts @@ -55,6 +55,7 @@ import { } from '../ecosystem/requirements.mts' import { buildErrorCause, + ConfigError, getNetworkErrorDiagnostics, } from '../error/errors.mts' @@ -383,7 +384,10 @@ export async function handleApiCallNoSpinner( export async function queryApi(path: string, apiToken: string) { const baseUrl = getDefaultApiBaseUrl() if (!baseUrl) { - throw new Error('Socket API base URL is not configured.') + throw new ConfigError( + 'Socket API base URL is not configured.', + CONFIG_KEY_API_BASE_URL, + ) } return await socketHttpRequest( From edf79065422f0690dbc7aa19176d751ca2a676ff Mon Sep 17 00:00:00 2001 From: jdalton Date: Fri, 17 Apr 2026 21:58:03 -0400 Subject: [PATCH 2/6] fix(error): preserve Error.cause chain in non-debug output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses Cursor bugbot feedback on PR #1238. The previous code walked the cause chain only when `showStack`/verbose was set, so non-debug users lost the wrapped-error diagnostic context that `messageWithCauses` used to concatenate automatically. - Always fold up to 5 `Error.cause` messages into `message` for generic Error instances, so the outer message reads like `outer: middle: root` — matching `pony-cause`'s behavior. - The verbose/debug path still renders the richer `Caused by [N]:` formatted body with stack traces. - Add regression tests covering both the cause-chain preservation and the 5-level truncation cap. --- packages/cli/src/utils/error/display.mts | 16 ++++++++++ .../test/unit/utils/error/display.test.mts | 32 +++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/packages/cli/src/utils/error/display.mts b/packages/cli/src/utils/error/display.mts index 668151fa7..cf221c03a 100644 --- a/packages/cli/src/utils/error/display.mts +++ b/packages/cli/src/utils/error/display.mts @@ -74,7 +74,23 @@ export function formatErrorForDisplay( body = error.body } else if (error instanceof Error) { title = opts.title || 'Unexpected error' + // Concatenate the cause chain into `message` (what non-debug users see) + // so diagnostic context from wrapped errors isn't silently dropped. + // `showStack` adds a richer formatted body with stack traces below. message = error.message + const plainCauses: string[] = [] + let walk: unknown = error.cause + let walkDepth = 1 + while (walk && walkDepth <= 5) { + plainCauses.push( + walk instanceof Error ? walk.message : String(walk), + ) + walk = walk instanceof Error ? walk.cause : undefined + walkDepth++ + } + if (plainCauses.length) { + message = `${message}: ${plainCauses.join(': ')}` + } if (showStack && error.stack) { // Format stack trace with proper indentation. diff --git a/packages/cli/test/unit/utils/error/display.test.mts b/packages/cli/test/unit/utils/error/display.test.mts index 806c9fd93..af6f19f54 100644 --- a/packages/cli/test/unit/utils/error/display.test.mts +++ b/packages/cli/test/unit/utils/error/display.test.mts @@ -150,6 +150,38 @@ describe('error/display', () => { expect(result.message).toBe('Something went wrong') }) + it('preserves Error.cause chain in message without debug mode', () => { + // Regression: formatErrorForDisplay used to only surface causes + // under showStack/verbose, so non-debug users lost the most useful + // diagnostic context. See PR #1238. + const inner = new Error('root DNS failure') + const middle = new Error('network call failed', { cause: inner }) + const outer = new Error('API request failed', { cause: middle }) + + const result = formatErrorForDisplay(outer) + + expect(result.message).toContain('API request failed') + expect(result.message).toContain('network call failed') + expect(result.message).toContain('root DNS failure') + }) + + it('stops walking causes at depth 5 to avoid runaway chains', () => { + // Build inside-out so the outer Error sits at index 10 and chains + // down through level-9, level-8, ..., level-0. + let e: Error | undefined + for (let i = 0; i <= 10; i++) { + e = new Error(`level-${i}`, e ? { cause: e } : undefined) + } + + const result = formatErrorForDisplay(e!) + + // Top message + 5 causes should appear. + expect(result.message).toContain('level-10') + expect(result.message).toContain('level-5') + // Anything beyond depth 5 should have been truncated. + expect(result.message).not.toContain('level-4') + }) + it('uses custom title when provided', () => { const error = new Error('Something went wrong') From e1525c5e336b6345d13462cb290098a8a017234d Mon Sep 17 00:00:00 2001 From: jdalton Date: Fri, 17 Apr 2026 22:41:12 -0400 Subject: [PATCH 3/6] fix(cli): stop any active spinner before printing error output --- packages/cli/src/cli-entry.mts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/cli-entry.mts b/packages/cli/src/cli-entry.mts index 309b4a565..f63729539 100755 --- a/packages/cli/src/cli-entry.mts +++ b/packages/cli/src/cli-entry.mts @@ -42,6 +42,7 @@ import { getSocketCliBootstrapSpec, } from '@socketsecurity/lib/env/socket-cli' import { getDefaultLogger } from '@socketsecurity/lib/logger' +import { getDefaultSpinner } from '@socketsecurity/lib/spinner' import { rootAliases, rootCommands } from './commands.mts' import { SOCKET_CLI_BIN_NAME } from './constants/packages.mts' @@ -174,6 +175,12 @@ void (async () => { } catch (e) { process.exitCode = 1 + // Stop any active spinner before emitting error output, otherwise + // its animation clashes with the error text on the same line. + // Spinner-wrapped command paths stop their own on catch, but any + // exception that bypasses those handlers reaches us here. + getDefaultSpinner()?.stop() + // Track CLI error for telemetry. await trackCliError(process.argv, cliStartTime, e, process.exitCode) debug('CLI uncaught error') @@ -195,8 +202,6 @@ void (async () => { if (isJson) { logger.log(serializeResultJson(formatErrorForJson(e))) } else { - // Add 2 newlines in stderr to bump below any spinner. - logger.error('\n') logger.error(formatErrorForTerminal(e)) debugDirNs('inspect', { error: e }) } From 76b2f0d31e9be3312d9385806765ff4316c6773d Mon Sep 17 00:00:00 2001 From: jdalton Date: Mon, 20 Apr 2026 14:33:23 -0400 Subject: [PATCH 4/6] chore(tests): drop self-referential and restating comments in display tests --- packages/cli/test/unit/utils/error/display.test.mts | 7 ------- 1 file changed, 7 deletions(-) diff --git a/packages/cli/test/unit/utils/error/display.test.mts b/packages/cli/test/unit/utils/error/display.test.mts index af6f19f54..639e3b661 100644 --- a/packages/cli/test/unit/utils/error/display.test.mts +++ b/packages/cli/test/unit/utils/error/display.test.mts @@ -151,9 +151,6 @@ describe('error/display', () => { }) it('preserves Error.cause chain in message without debug mode', () => { - // Regression: formatErrorForDisplay used to only surface causes - // under showStack/verbose, so non-debug users lost the most useful - // diagnostic context. See PR #1238. const inner = new Error('root DNS failure') const middle = new Error('network call failed', { cause: inner }) const outer = new Error('API request failed', { cause: middle }) @@ -166,8 +163,6 @@ describe('error/display', () => { }) it('stops walking causes at depth 5 to avoid runaway chains', () => { - // Build inside-out so the outer Error sits at index 10 and chains - // down through level-9, level-8, ..., level-0. let e: Error | undefined for (let i = 0; i <= 10; i++) { e = new Error(`level-${i}`, e ? { cause: e } : undefined) @@ -175,10 +170,8 @@ describe('error/display', () => { const result = formatErrorForDisplay(e!) - // Top message + 5 causes should appear. expect(result.message).toContain('level-10') expect(result.message).toContain('level-5') - // Anything beyond depth 5 should have been truncated. expect(result.message).not.toContain('level-4') }) From 8ebd9cc5ec560ea9ca615dc21ee7d2db0b47bc95 Mon Sep 17 00:00:00 2001 From: jdalton Date: Mon, 20 Apr 2026 15:31:57 -0400 Subject: [PATCH 5/6] refactor(error display): apply cause-chain walk to typed-error branches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extract the .cause walk into `appendCauseChain` and use it from every Error-instanceof branch, not just the generic fallback. AuthError / NetworkError / FileSystemError / ConfigError / InputError / RateLimitError constructors don't currently forward `{ cause }`, so the helper is a no-op for them today — but it future-proofs the display layer so the day a typed error does carry a cause, it surfaces without another edit. --- packages/cli/src/utils/error/display.mts | 45 ++++++++++++++---------- 1 file changed, 26 insertions(+), 19 deletions(-) diff --git a/packages/cli/src/utils/error/display.mts b/packages/cli/src/utils/error/display.mts index cf221c03a..d85d374fd 100644 --- a/packages/cli/src/utils/error/display.mts +++ b/packages/cli/src/utils/error/display.mts @@ -25,6 +25,25 @@ export type ErrorDisplayOptions = { verbose?: boolean | undefined } +/** + * Append the `.cause` chain (up to 5 levels) to a base message so + * non-debug users see the diagnostic context from wrapped errors. + * Typed errors (AuthError, NetworkError, …) can also carry causes. + */ +function appendCauseChain(baseMessage: string, cause: unknown): string { + const plainCauses: string[] = [] + let walk: unknown = cause + let walkDepth = 1 + while (walk && walkDepth <= 5) { + plainCauses.push(walk instanceof Error ? walk.message : String(walk)) + walk = walk instanceof Error ? walk.cause : undefined + walkDepth++ + } + return plainCauses.length + ? `${baseMessage}: ${plainCauses.join(': ')}` + : baseMessage +} + /** * Format an error for display with polish and clarity. * Uses LOG_SYMBOLS and colors for visual hierarchy. @@ -47,50 +66,38 @@ export function formatErrorForDisplay( if (error.retryAfter) { message += ` (retry after ${error.retryAfter}s)` } + message = appendCauseChain(message, error.cause) } else if (error instanceof AuthError) { title = 'Authentication error' - message = error.message + message = appendCauseChain(error.message, error.cause) } else if (error instanceof NetworkError) { title = 'Network error' message = error.message if (error.statusCode) { message += ` (HTTP ${error.statusCode})` } + message = appendCauseChain(message, error.cause) } else if (error instanceof FileSystemError) { title = 'File system error' message = error.message if (error.path) { message += ` (${error.path})` } + message = appendCauseChain(message, error.cause) } else if (error instanceof ConfigError) { title = 'Configuration error' message = error.message if (error.configKey) { message += ` (key: ${error.configKey})` } + message = appendCauseChain(message, error.cause) } else if (error instanceof InputError) { title = 'Invalid input' - message = error.message + message = appendCauseChain(error.message, error.cause) body = error.body } else if (error instanceof Error) { title = opts.title || 'Unexpected error' - // Concatenate the cause chain into `message` (what non-debug users see) - // so diagnostic context from wrapped errors isn't silently dropped. - // `showStack` adds a richer formatted body with stack traces below. - message = error.message - const plainCauses: string[] = [] - let walk: unknown = error.cause - let walkDepth = 1 - while (walk && walkDepth <= 5) { - plainCauses.push( - walk instanceof Error ? walk.message : String(walk), - ) - walk = walk instanceof Error ? walk.cause : undefined - walkDepth++ - } - if (plainCauses.length) { - message = `${message}: ${plainCauses.join(': ')}` - } + message = appendCauseChain(error.message, error.cause) if (showStack && error.stack) { // Format stack trace with proper indentation. From 6e8c5aa6c1c74199eb80ecf4fc88baf534648601 Mon Sep 17 00:00:00 2001 From: jdalton Date: Mon, 20 Apr 2026 15:37:15 -0400 Subject: [PATCH 6/6] refactor(error display): use messageWithCauses from @socketsecurity/lib/errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the hand-rolled cause-chain walker with the socket-lib re-export (originally pony-cause). Benefits: - Single source of truth for cause formatting across CLI + lib. - Proper cycle detection via a `seen` Set instead of a depth-5 cap that could silently truncate legitimate 5+ level chains. - api.mts drops its direct `pony-cause` import for the same re-export. - Removes `pony-cause` from root and cli devDependencies (it was imported in runtime code, a latent classification bug — socket-lib bundles it internally so direct dependence was never needed). Test updated to assert cycle termination instead of depth cap. --- package.json | 1 - packages/cli/package.json | 1 - packages/cli/src/utils/error/display.mts | 23 ++++++++----------- packages/cli/src/utils/socket/api.mts | 3 +-- .../test/unit/utils/error/display.test.mts | 18 +++++++-------- pnpm-lock.yaml | 9 -------- pnpm-workspace.yaml | 1 - 7 files changed, 20 insertions(+), 36 deletions(-) diff --git a/package.json b/package.json index 92f2fe2ec..f01e121bf 100644 --- a/package.json +++ b/package.json @@ -121,7 +121,6 @@ "oxfmt": "0.37.0", "oxlint": "1.52.0", "package-builder": "workspace:*", - "pony-cause": "catalog:", "postject": "catalog:", "registry-auth-token": "catalog:", "registry-url": "catalog:", diff --git a/packages/cli/package.json b/packages/cli/package.json index ac16ab786..4b3b30098 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -110,7 +110,6 @@ "npm-package-arg": "catalog:", "open": "catalog:", "package-builder": "workspace:*", - "pony-cause": "catalog:", "registry-auth-token": "catalog:", "registry-url": "catalog:", "semver": "catalog:", diff --git a/packages/cli/src/utils/error/display.mts b/packages/cli/src/utils/error/display.mts index d85d374fd..163befe25 100644 --- a/packages/cli/src/utils/error/display.mts +++ b/packages/cli/src/utils/error/display.mts @@ -2,6 +2,7 @@ import colors from 'yoctocolors-cjs' +import { messageWithCauses } from '@socketsecurity/lib/errors' import { LOG_SYMBOLS } from '@socketsecurity/lib/logger' import { stripAnsi } from '@socketsecurity/lib/strings' @@ -26,22 +27,18 @@ export type ErrorDisplayOptions = { } /** - * Append the `.cause` chain (up to 5 levels) to a base message so - * non-debug users see the diagnostic context from wrapped errors. - * Typed errors (AuthError, NetworkError, …) can also carry causes. + * Append the `.cause` chain to a decorated base message. Typed errors + * build their message with suffixes (e.g. ` (HTTP 500)`) before this + * is called, so we can't just `messageWithCauses(error)` — we decorate + * first, then delegate cause walking to socket-lib. */ function appendCauseChain(baseMessage: string, cause: unknown): string { - const plainCauses: string[] = [] - let walk: unknown = cause - let walkDepth = 1 - while (walk && walkDepth <= 5) { - plainCauses.push(walk instanceof Error ? walk.message : String(walk)) - walk = walk instanceof Error ? walk.cause : undefined - walkDepth++ + if (!cause) { + return baseMessage } - return plainCauses.length - ? `${baseMessage}: ${plainCauses.join(': ')}` - : baseMessage + const causeText = + cause instanceof Error ? messageWithCauses(cause) : String(cause) + return `${baseMessage}: ${causeText}` } /** diff --git a/packages/cli/src/utils/socket/api.mts b/packages/cli/src/utils/socket/api.mts index a94a1adaf..53acd468c 100644 --- a/packages/cli/src/utils/socket/api.mts +++ b/packages/cli/src/utils/socket/api.mts @@ -19,10 +19,9 @@ * - Falls back to configured apiBaseUrl or default API_V0_URL */ -import { messageWithCauses } from 'pony-cause' - import { debug, debugDir } from '@socketsecurity/lib/debug' import { getSocketCliApiBaseUrl } from '@socketsecurity/lib/env/socket-cli' +import { messageWithCauses } from '@socketsecurity/lib/errors' import { httpRequest } from '@socketsecurity/lib/http-request' import { getDefaultLogger } from '@socketsecurity/lib/logger' import { getDefaultSpinner } from '@socketsecurity/lib/spinner' diff --git a/packages/cli/test/unit/utils/error/display.test.mts b/packages/cli/test/unit/utils/error/display.test.mts index 639e3b661..ed323cb98 100644 --- a/packages/cli/test/unit/utils/error/display.test.mts +++ b/packages/cli/test/unit/utils/error/display.test.mts @@ -162,17 +162,17 @@ describe('error/display', () => { expect(result.message).toContain('root DNS failure') }) - it('stops walking causes at depth 5 to avoid runaway chains', () => { - let e: Error | undefined - for (let i = 0; i <= 10; i++) { - e = new Error(`level-${i}`, e ? { cause: e } : undefined) - } + it('terminates on cyclic cause chains', () => { + const a = new Error('a') + const b = new Error('b') + ;(a as Error & { cause?: unknown }).cause = b + ;(b as Error & { cause?: unknown }).cause = a - const result = formatErrorForDisplay(e!) + const result = formatErrorForDisplay(a) - expect(result.message).toContain('level-10') - expect(result.message).toContain('level-5') - expect(result.message).not.toContain('level-4') + expect(result.message).toContain('a') + expect(result.message).toContain('b') + expect(result.message).toContain('...') }) it('uses custom title when provided', () => { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 19cc1e347..063261e72 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -391,9 +391,6 @@ catalogs: open: specifier: 10.2.0 version: 10.2.0 - pony-cause: - specifier: 2.1.11 - version: 2.1.11 postject: specifier: 1.0.0-alpha.6 version: 1.0.0-alpha.6 @@ -702,9 +699,6 @@ importers: package-builder: specifier: workspace:* version: link:packages/package-builder - pony-cause: - specifier: 'catalog:' - version: 2.1.11 postject: specifier: 'catalog:' version: 1.0.0-alpha.6 @@ -900,9 +894,6 @@ importers: package-builder: specifier: workspace:* version: link:../package-builder - pony-cause: - specifier: 'catalog:' - version: 2.1.11 registry-auth-token: specifier: 'catalog:' version: 5.1.0 diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index a3d319646..ce9d7c6c9 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -116,7 +116,6 @@ catalog: oxlint: 1.52.0 packageurl-js: npm:@socketregistry/packageurl-js@^1.4.2 path-parse: npm:@socketregistry/path-parse@^1.0.8 - pony-cause: 2.1.11 postject: 1.0.0-alpha.6 react: 19.2.0 react-reconciler: 0.33.0