From 36ca0b3d0ff041eb9c2ce769f0774176bdb2c107 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Wed, 4 Nov 2026 09:36:10 +0000 Subject: [PATCH 1/2] fix(cli): Report dashboard widget failures through the shared error policy Widget query failures were sent with a raw captureException, skipping the 4xx silencing rules and the cli_error tags. They now go through reportCliError, and the widget shows the API detail, wrapped to the widget width. --- packages/cli/src/lib/api/dashboards.ts | 27 ++++++-- packages/cli/src/lib/formatters/dashboard.ts | 24 +++++-- packages/cli/test/lib/api/dashboards.test.ts | 68 ++++++++++++++++++- .../cli/test/lib/formatters/dashboard.test.ts | 20 ++++++ 4 files changed, 125 insertions(+), 14 deletions(-) diff --git a/packages/cli/src/lib/api/dashboards.ts b/packages/cli/src/lib/api/dashboards.ts index 09ba5532f..8b9025373 100644 --- a/packages/cli/src/lib/api/dashboards.ts +++ b/packages/cli/src/lib/api/dashboards.ts @@ -11,8 +11,6 @@ import { listOrganizationDashboards, updateOrganizationDashboard, } from "@sentry/api"; -// oxlint-disable-next-line sentry-cli/no-namespace-import -- Sentry SDK recommends namespace import -import * as Sentry from "@sentry/node"; import { resolveEventsSort } from "@sentry/toolkit-core/events-sort"; import { array, safeParse } from "valibot"; @@ -37,7 +35,8 @@ import { type TimeseriesResult, type WidgetDataResult, } from "../../types/dashboard.js"; -import { stringifyUnknown } from "../errors.js"; +import { reportCliError } from "../error-reporting.js"; +import { ApiError, stringifyUnknown } from "../errors.js"; import { resolveOrgRegion } from "../region.js"; @@ -630,14 +629,32 @@ async function queryWidgetData( reason: `Display type '${widget.displayType}' is not yet supported`, }; } catch (error) { - Sentry.captureException(error); + // Best-effort catch: the other widgets still render. Report through the + // shared policy so 401–499 failures are silenced and 400s keep API context. + reportCliError(error); return { type: "error", - message: stringifyUnknown(error), + message: widgetErrorMessage(error), } satisfies ErrorResult; } } +/** + * One-line widget failure message. Keeps the API's `detail`, which explains + * why the widget's query was rejected; the status line alone does not. + */ +function widgetErrorMessage(error: unknown): string { + const message = stringifyUnknown(error); + const detail = + error instanceof ApiError + ? error.detail?.replace(/\s+/g, " ").trim() + : undefined; + // Bodyless responses fall back to the status text, already in the message. + return detail && !message.includes(detail) + ? `${message}: ${detail}` + : message; +} + /** * Query data for all widgets in a dashboard in parallel. * diff --git a/packages/cli/src/lib/formatters/dashboard.ts b/packages/cli/src/lib/formatters/dashboard.ts index f783f76b3..d6a0d210f 100644 --- a/packages/cli/src/lib/formatters/dashboard.ts +++ b/packages/cli/src/lib/formatters/dashboard.ts @@ -1527,9 +1527,18 @@ function renderTextContent(content: string, innerWidth: number): string[] { return wrapped.split("\n"); } -/** Render placeholder content for unsupported/error widgets (no title/border). */ -function renderPlaceholderContent(message: string): string[] { - return [isPlainOutput() ? `(${message})` : muted(`(${message})`)]; +/** + * Render placeholder content for unsupported/error widgets (no title/border). + * Wraps to the widget width so long API details stay readable. + */ +function renderPlaceholderContent( + message: string, + innerWidth: number, +): string[] { + const lines = wrapAnsi(`(${message})`, innerWidth, { hard: true }).split( + "\n", + ); + return isPlainOutput() ? lines : lines.map((line) => muted(line)); } // --------------------------------------------------------------------------- @@ -1706,13 +1715,16 @@ function renderContentLines(opts: { return renderTextContent(data.content, innerWidth); case "unsupported": - return renderPlaceholderContent(data.reason); + return renderPlaceholderContent(data.reason, innerWidth); case "error": - return renderPlaceholderContent(`query failed: ${data.message}`); + return renderPlaceholderContent( + `query failed: ${data.message}`, + innerWidth, + ); default: - return renderPlaceholderContent("unknown widget data type"); + return renderPlaceholderContent("unknown widget data type", innerWidth); } } diff --git a/packages/cli/test/lib/api/dashboards.test.ts b/packages/cli/test/lib/api/dashboards.test.ts index 2f92e22aa..09b4fe1d6 100644 --- a/packages/cli/test/lib/api/dashboards.test.ts +++ b/packages/cli/test/lib/api/dashboards.test.ts @@ -12,14 +12,21 @@ import { queryAllWidgets, restoreDashboardRevision, } from "../../../src/lib/api/dashboards.js"; +import { ApiError } from "../../../src/lib/errors.js"; import type { DashboardDetail, DashboardWidget, } from "../../../src/types/dashboard.js"; -const { apiRequestToRegionMock, resolveOrgRegionMock } = vi.hoisted(() => ({ - apiRequestToRegionMock: vi.fn(), - resolveOrgRegionMock: vi.fn(), +const { apiRequestToRegionMock, reportCliErrorMock, resolveOrgRegionMock } = + vi.hoisted(() => ({ + apiRequestToRegionMock: vi.fn(), + reportCliErrorMock: vi.fn(), + resolveOrgRegionMock: vi.fn(), + })); + +vi.mock("../../../src/lib/error-reporting.js", () => ({ + reportCliError: reportCliErrorMock, })); vi.mock("../../../src/lib/region.js", async (importOriginal) => { @@ -334,3 +341,58 @@ describe("widget sort resolution", () => { ); }); }); + +describe("widget query failures", () => { + const REGION = "https://us.sentry.io"; + const dashboard = makeDashboard([ + { + title: "Slow screens", + displayType: "table", + widgetType: "spans", + queries: [ + { + fields: ["p50(tags[view_ms,number])"], + columns: [], + aggregates: ["p50(tags[view_ms,number])"], + conditions: "", + orderby: "", + }, + ], + }, + ]); + + test("reports through the shared policy and keeps the API detail", async () => { + const error = new ApiError( + "API request failed: 400 Bad Request", + 400, + "tags[view_ms,number] is invalid for parameter 1 in p50.\nIts a string type field.", + "/organizations/test-org/events/", + ); + apiRequestToRegionMock.mockReset(); + apiRequestToRegionMock.mockRejectedValue(error); + reportCliErrorMock.mockReset(); + + const results = await queryAllWidgets(REGION, "test-org", dashboard); + + expect(reportCliErrorMock).toHaveBeenCalledWith(error); + expect(results.get(0)).toEqual({ + type: "error", + message: + "API request failed: 400 Bad Request: tags[view_ms,number] is invalid for parameter 1 in p50. Its a string type field.", + }); + }); + + test("does not repeat a status-text detail", async () => { + apiRequestToRegionMock.mockReset(); + apiRequestToRegionMock.mockRejectedValue( + new ApiError("API request failed: 404 Not Found", 404, "Not Found"), + ); + + const results = await queryAllWidgets(REGION, "test-org", dashboard); + + expect(results.get(0)).toEqual({ + type: "error", + message: "API request failed: 404 Not Found", + }); + }); +}); diff --git a/packages/cli/test/lib/formatters/dashboard.test.ts b/packages/cli/test/lib/formatters/dashboard.test.ts index f8d041502..8ba81ebb7 100644 --- a/packages/cli/test/lib/formatters/dashboard.test.ts +++ b/packages/cli/test/lib/formatters/dashboard.test.ts @@ -1096,6 +1096,26 @@ describe("formatDashboardWithData", () => { expect(output).toContain("Broken Widget"); expect(output).toContain("query failed: Query timeout exceeded"); }); + + test("wraps a long API detail instead of clipping it", () => { + const detail = + "tags[view_ms,number] is invalid for parameter 1 in p50. Its a string type field"; + const data = makeDashboardData({ + widgets: [ + makeWidget({ + title: "Broken Widget", + displayType: "line", + data: makeErrorData({ + message: `API request failed: 400 Bad Request: ${detail}`, + }), + }), + ], + }); + const output = formatDashboardWithData(data); + for (const word of detail.split(" ")) { + expect(output).toContain(word); + } + }); }); describe("text widget", () => { From 378d7bd5e5d904e987a3ba3b58fe358662c81c9b Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Wed, 4 Nov 2026 15:04:21 +0000 Subject: [PATCH 2/2] fix(cli): Route every Sentry capture through reportCliError Add the sentry-cli/no-raw-capture-exception lint rule so a raw captureException can't skip the 4xx/network silencing and api_error context again (CLI-10). reportCliError now takes optional tags, extra, and level, and the 18 existing raw call sites use it. --- .oxlintrc.json | 1 + packages/cli/AGENTS.md | 6 +++ packages/cli/lint-rules/cli-oxlint-plugin.js | 20 ++++++++++ packages/cli/src/commands/cli/setup.ts | 4 +- packages/cli/src/commands/log/list.ts | 5 +-- packages/cli/src/lib/agent-skills.ts | 6 +-- packages/cli/src/lib/api/teams.ts | 5 +-- packages/cli/src/lib/auth-hint.ts | 7 ++-- packages/cli/src/lib/delta-upgrade.ts | 3 +- packages/cli/src/lib/dsn/fs-utils.ts | 3 +- packages/cli/src/lib/error-reporting.ts | 17 +++++++-- .../lib/init/tools/create-sentry-project.ts | 4 +- packages/cli/src/lib/init/verify-setup.ts | 9 +++-- packages/cli/src/lib/init/wizard-runner.ts | 10 ++--- packages/cli/src/lib/interactive-login.ts | 7 ++-- packages/cli/src/lib/mcp/server.ts | 4 +- packages/cli/src/lib/telemetry.ts | 2 +- packages/cli/src/lib/version-check.ts | 7 ++-- packages/cli/test/lib/dsn/fs-utils.test.ts | 38 +++++++++---------- packages/cli/test/lib/error-reporting.test.ts | 27 ++++++++++++- .../init/verify-setup-windows.mocked.test.ts | 4 +- .../cli/test/lib/init/verify-setup.test.ts | 9 +++-- packages/cli/test/lib/telemetry.test.ts | 3 +- 23 files changed, 132 insertions(+), 69 deletions(-) diff --git a/.oxlintrc.json b/.oxlintrc.json index 18ac71c5a..d9f0bdbd3 100644 --- a/.oxlintrc.json +++ b/.oxlintrc.json @@ -82,6 +82,7 @@ ], "rules": { "sentry-cli/no-silent-catch": "error", + "sentry-cli/no-raw-capture-exception": "error", "sentry-cli/no-stdout-write-in-commands": "error", "sentry-cli/no-process-stdout-in-commands": "error", "sentry-cli/no-stderr-write-in-commands": "error", diff --git a/packages/cli/AGENTS.md b/packages/cli/AGENTS.md index afe0aad7e..fe5746722 100644 --- a/packages/cli/AGENTS.md +++ b/packages/cli/AGENTS.md @@ -988,6 +988,12 @@ separate script or baseline file to maintain. Fix a grandfathered catch by adding logging/re-throwing and deleting its suppression line; only add a new suppression for a genuinely intentional silent catch, with a real reason. +Report errors to Sentry with `reportCliError(error, { tags, extra, level })` +from `src/lib/error-reporting.ts`, never `captureException` directly. A raw +capture skips the 4xx/network silencing and the `api_error` context, so user +errors pile into one issue with no detail (CLI-10). The +`no-raw-capture-exception` CLI Oxlint rule enforces this. + ### Auto-Recovery for Wrong Entity Types When a user provides the wrong type of identifier (e.g., an issue short ID diff --git a/packages/cli/lint-rules/cli-oxlint-plugin.js b/packages/cli/lint-rules/cli-oxlint-plugin.js index 064357e65..e55678234 100644 --- a/packages/cli/lint-rules/cli-oxlint-plugin.js +++ b/packages/cli/lint-rules/cli-oxlint-plugin.js @@ -382,6 +382,26 @@ const rules = { } : {}, ), + // A raw capture skips the 4xx silencing and `api_error` context that + // reportCliError() applies, so user errors pile into one issue (CLI-10). + "no-raw-capture-exception": rule( + "Report errors with reportCliError() from lib/error-reporting.js. A raw captureException skips the 4xx silencing and API error context.", + (_context, file, report) => + file.includes("/src/") && !file.endsWith("/src/lib/error-reporting.ts") + ? { + ImportDeclaration(node) { + if (!String(node.source.value).startsWith("@sentry/")) return; + for (const specifier of node.specifiers) { + if (specifier.imported?.name === "captureException") + report(specifier); + } + }, + CallExpression(node) { + if (call(node, "captureException")) report(node); + }, + } + : {}, + ), "no-stdout-write-in-commands": rule( "Yield CommandOutput instead of writing to stdout.", (_context, file, report) => diff --git a/packages/cli/src/commands/cli/setup.ts b/packages/cli/src/commands/cli/setup.ts index 48481b1c3..c0935871d 100644 --- a/packages/cli/src/commands/cli/setup.ts +++ b/packages/cli/src/commands/cli/setup.ts @@ -10,7 +10,6 @@ import { existsSync, unlinkSync } from "node:fs"; import { chmod, copyFile, mkdir, rename, unlink } from "node:fs/promises"; import { dirname, join } from "node:path"; import { isatty } from "node:tty"; -import { captureException } from "@sentry/node"; import type { SentryContext } from "../../context.js"; import { installAgentSkills } from "../../lib/agent-skills.js"; import { assertAutoLoginHostTrusted } from "../../lib/auto-auth.js"; @@ -67,6 +66,7 @@ import { type ShellInfo, } from "../../lib/shell.js"; import { addToWindowsUserPath } from "../../lib/windows-path.js"; +import { reportCliError } from "../../lib/error-reporting.js"; type SetupFlags = { readonly install: boolean; @@ -507,7 +507,7 @@ async function bestEffort( await fn(); } catch (error) { warn(stepName, error); - captureException(error, { + reportCliError(error, { level: "warning", tags: { "setup.step": stepName }, }); diff --git a/packages/cli/src/commands/log/list.ts b/packages/cli/src/commands/log/list.ts index 75efa0d6f..94328a474 100644 --- a/packages/cli/src/commands/log/list.ts +++ b/packages/cli/src/commands/log/list.ts @@ -6,8 +6,6 @@ * Supports trace ID as a positional argument to filter logs by trace. */ -// oxlint-disable-next-line sentry-cli/no-namespace-import -- Sentry SDK recommends namespace import -import * as Sentry from "@sentry/node"; import type { SentryContext } from "../../context.js"; import { type LogSortDirection, @@ -80,6 +78,7 @@ import { } from "../../lib/trace-target.js"; import { getUpdateNotification } from "../../lib/version-check.js"; import { SentryLogSchema } from "../../generated/output-schemas/log.js"; +import { reportCliError } from "../../lib/error-reporting.js"; type ListFlags = { readonly limit: number; @@ -412,7 +411,7 @@ async function fetchPoll( if (error instanceof ValidationError) { throw error; } - Sentry.captureException(error); + reportCliError(error); const message = stringifyUnknown(error); config.onDiagnostic(`Error fetching logs: ${message}\n`); return; diff --git a/packages/cli/src/lib/agent-skills.ts b/packages/cli/src/lib/agent-skills.ts index 63d968fc8..09feaf3dd 100644 --- a/packages/cli/src/lib/agent-skills.ts +++ b/packages/cli/src/lib/agent-skills.ts @@ -12,8 +12,8 @@ import { accessSync, constants, existsSync, mkdirSync } from "node:fs"; import { rename, rm, writeFile } from "node:fs/promises"; import { basename, dirname, join } from "node:path"; -import { captureException } from "@sentry/node"; import { SKILL_FILES } from "../generated/skill-content.js"; +import { reportCliError } from "./error-reporting.js"; /** Where skills are installed */ export type AgentSkillLocation = { @@ -87,7 +87,7 @@ async function atomicWriteFile( await rename(tempPath, destPath); } catch (error) { await rm(tempPath, { force: true }).catch((cleanupError) => { - captureException(cleanupError, { + reportCliError(cleanupError, { level: "debug", tags: { "setup.step": "agent-skills-cleanup" }, }); @@ -128,7 +128,7 @@ async function writeSkillFiles( referenceCount, }; } catch (error) { - captureException(error, { + reportCliError(error, { level: "warning", tags: { "setup.step": "agent-skills" }, }); diff --git a/packages/cli/src/lib/api/teams.ts b/packages/cli/src/lib/api/teams.ts index 9f888176d..2e63ad5cd 100644 --- a/packages/cli/src/lib/api/teams.ts +++ b/packages/cli/src/lib/api/teams.ts @@ -10,8 +10,6 @@ import { listOrganizationTeams, listProjectTeams as sdkListProjectTeams, } from "@sentry/api"; -// oxlint-disable-next-line sentry-cli/no-namespace-import -- Sentry SDK recommends namespace import -import * as Sentry from "@sentry/node"; import type { SentryTeam } from "../../types/index.js"; @@ -23,6 +21,7 @@ import { unwrapPaginatedResult, unwrapResult, } from "./infrastructure.js"; +import { reportCliError } from "../error-reporting.js"; /** * List teams in an organization. @@ -120,7 +119,7 @@ export async function createTeam( try { await addMemberToTeam(orgSlug, team.slug, "me"); } catch (error) { - Sentry.captureException(error, { + reportCliError(error, { extra: { orgSlug, teamSlug: team.slug, context: "auto-add member" }, }); logger.warn( diff --git a/packages/cli/src/lib/auth-hint.ts b/packages/cli/src/lib/auth-hint.ts index b1ce75df6..bbad173e8 100644 --- a/packages/cli/src/lib/auth-hint.ts +++ b/packages/cli/src/lib/auth-hint.ts @@ -23,8 +23,6 @@ * is wrong, just a helpful breadcrumb). */ -// oxlint-disable-next-line sentry-cli/no-namespace-import -- Sentry SDK recommends namespace import -import * as Sentry from "@sentry/node"; import { getActiveEnvVarName, getRawEnvToken, @@ -34,6 +32,7 @@ import { import { getUserInfo } from "./db/user.js"; import { logger } from "./logger.js"; import { getRcInjectedTokenSource } from "./sentryclirc.js"; +import { reportCliError } from "./error-reporting.js"; const log = logger.withTag("auth"); @@ -73,7 +72,7 @@ export function maybeWarnEnvTokenIgnored(): void { try { hasStored = hasStoredAuthCredentials(); } catch (error) { - Sentry.captureException(error); + reportCliError(error); } if (!hasStored) { return; @@ -121,7 +120,7 @@ function resolveStoredUserLabel(): string { // DB read failure for the user-info cache is non-fatal — fall back // to the neutral label — but still surface the error to Sentry so // we can diagnose persistent cache-read failures. - Sentry.captureException(error); + reportCliError(error); return "stored OAuth user"; } } diff --git a/packages/cli/src/lib/delta-upgrade.ts b/packages/cli/src/lib/delta-upgrade.ts index 7ac8fbe0c..d2dbc5374 100644 --- a/packages/cli/src/lib/delta-upgrade.ts +++ b/packages/cli/src/lib/delta-upgrade.ts @@ -43,6 +43,7 @@ import { import { CLI_VERSION } from "./constants.js"; import { customFetch } from "./custom-ca.js"; import { getConfigDir } from "./db/index.js"; +import { reportCliError } from "./error-reporting.js"; import { extractInputUrl } from "./fetch-input.js"; import { formatBytes } from "./formatters/numbers.js"; import { logger } from "./logger.js"; @@ -669,7 +670,7 @@ export function attemptDeltaUpgrade( span.setStatus({ code: 1 }); return result; } catch (error) { - Sentry.captureException(error, { + reportCliError(error, { level: "warning", tags: { "delta.from_version": CLI_VERSION, diff --git a/packages/cli/src/lib/dsn/fs-utils.ts b/packages/cli/src/lib/dsn/fs-utils.ts index 9d568be99..49046e10d 100644 --- a/packages/cli/src/lib/dsn/fs-utils.ts +++ b/packages/cli/src/lib/dsn/fs-utils.ts @@ -7,6 +7,7 @@ import { stat } from "node:fs/promises"; // oxlint-disable-next-line sentry-cli/no-namespace-import -- Sentry SDK recommends namespace import import * as Sentry from "@sentry/node"; +import { reportCliError } from "../error-reporting.js"; /** * Check if an error is an expected file system error that should be silently ignored. @@ -67,7 +68,7 @@ export function handleFileError( context: { operation: string; path?: string }, ): void { if (!isIgnorableFileError(error)) { - Sentry.captureException(error, { + reportCliError(error, { tags: { operation: context.operation, }, diff --git a/packages/cli/src/lib/error-reporting.ts b/packages/cli/src/lib/error-reporting.ts index 9191430fc..fecd59d31 100644 --- a/packages/cli/src/lib/error-reporting.ts +++ b/packages/cli/src/lib/error-reporting.ts @@ -437,6 +437,13 @@ function setCliErrorContext(scope: Sentry.Scope, error: unknown): void { // Public API // --------------------------------------------------------------------------- +/** Call-site details added to a captured event. */ +export type ReportContext = { + level?: Sentry.SeverityLevel; + tags?: Record; + extra?: Record; +}; + /** * Report an error to Sentry without rethrowing. * @@ -445,11 +452,15 @@ function setCliErrorContext(scope: Sentry.Scope, error: unknown): void { * log is not a substitute: users do not run `--verbose`, so swallowed * unexpected failures must become Sentry issues. * + * This is the only place that calls `captureException`; the + * `sentry-cli/no-raw-capture-exception` lint rule enforces it. + * * - Silenced errors (network, expected auth, 4xx) emit a metric and return * without calling `captureException`. - * - Captured errors get grouping tags + structured context on a fresh scope. + * - Captured errors get grouping tags + structured context on a fresh scope, + * plus the caller's `context`. */ -export function reportCliError(error: unknown): void { +export function reportCliError(error: unknown, context?: ReportContext): void { const silenced = classifySilenced(error); if (silenced) { recordSilencedError(error, silenced); @@ -459,7 +470,7 @@ export function reportCliError(error: unknown): void { Sentry.withScope((scope) => { setGroupingTags(scope, error); setCliErrorContext(scope, error); - Sentry.captureException(error); + Sentry.captureException(error, context); }); } diff --git a/packages/cli/src/lib/init/tools/create-sentry-project.ts b/packages/cli/src/lib/init/tools/create-sentry-project.ts index af78a27ce..37680bae6 100644 --- a/packages/cli/src/lib/init/tools/create-sentry-project.ts +++ b/packages/cli/src/lib/init/tools/create-sentry-project.ts @@ -7,7 +7,6 @@ * that auto-creates a personal team for eligible members. */ -import { captureException } from "@sentry/node"; import { createProjectWithAutoTeam, createProjectWithDsn, @@ -26,6 +25,7 @@ import type { } from "../types.js"; import { formatToolError } from "./shared.js"; import type { InitToolDefinition, ToolContext } from "./types.js"; +import { reportCliError } from "../../error-reporting.js"; type ProjectData = { projectSlug: string; @@ -93,7 +93,7 @@ async function resolveProjectCreation(opts: { platform && err.detail?.includes("Invalid platform") ) { - captureException(err, { + reportCliError(err, { extra: { attemptedPlatform: platform, projectName: name, diff --git a/packages/cli/src/lib/init/verify-setup.ts b/packages/cli/src/lib/init/verify-setup.ts index aad64bded..d118026c5 100644 --- a/packages/cli/src/lib/init/verify-setup.ts +++ b/packages/cli/src/lib/init/verify-setup.ts @@ -14,7 +14,7 @@ import { type ChildProcess, spawn, spawnSync } from "node:child_process"; import { resolve } from "node:path"; -import { addBreadcrumb, captureException, setTag } from "@sentry/node"; +import { addBreadcrumb, setTag } from "@sentry/node"; import { createSpotlightBuffer } from "@spotlightjs/spotlight/sdk"; import { BUFFER_SIZE, shutdownServer } from "../../commands/local/run.js"; import { buildApp, tryListen } from "../../commands/local/server.js"; @@ -22,6 +22,7 @@ import { detectDevCommand } from "../dev-script.js"; import { logger } from "../logger.js"; import type { WorkflowRunResult } from "./types.js"; import type { WizardUI } from "./ui/types.js"; +import { reportCliError } from "../error-reporting.js"; /** Verification timeout in seconds. */ const VERIFY_TIMEOUT_S = 15; @@ -577,7 +578,7 @@ function reportOutcome(outcome: VerifyOutcome, ctx: ReportContext): void { if (outcome.kind === "errored") { const scrubbed = scrubOutputLine(outcome.errorLine).slice(0, 200); ui.log.warn(`Could not verify — startup error: ${scrubbed}`); - captureException(new Error("init verification: startup error"), { + reportCliError(new Error("init verification: startup error"), { tags: { ...telemetryTags, "wizard.verify": "startup_error" }, extra: { ...telemetryExtra, @@ -596,7 +597,7 @@ function reportOutcome(outcome: VerifyOutcome, ctx: ReportContext): void { `Could not verify — dev server exited with code ${outcome.code}`, ); logger.debug(`Last output: ${getLines().slice(-3).join(" | ")}`); - captureException(new Error("init verification failed"), { + reportCliError(new Error("init verification failed"), { tags: { ...telemetryTags, "wizard.verify": "child_exited" }, extra: { ...telemetryExtra, exitCode: outcome.code }, }); @@ -605,7 +606,7 @@ function reportOutcome(outcome: VerifyOutcome, ctx: ReportContext): void { const verifyTag = outcome.kind === "silent" ? "silent" : "timeout"; ui.log.warn(`Could not verify — no output within ${VERIFY_TIMEOUT_S}s`); - captureException(new Error("init verification failed"), { + reportCliError(new Error("init verification failed"), { tags: { ...telemetryTags, "wizard.verify": verifyTag }, extra: telemetryExtra, }); diff --git a/packages/cli/src/lib/init/wizard-runner.ts b/packages/cli/src/lib/init/wizard-runner.ts index 8f1e252ee..95401ef22 100644 --- a/packages/cli/src/lib/init/wizard-runner.ts +++ b/packages/cli/src/lib/init/wizard-runner.ts @@ -14,12 +14,7 @@ import { randomBytes } from "node:crypto"; import { MastraClient } from "@mastra/client-js"; -import { - addBreadcrumb, - captureException, - getTraceData, - setTag, -} from "@sentry/node"; +import { addBreadcrumb, getTraceData, setTag } from "@sentry/node"; import { formatAuthHeader } from "../auth-header.js"; import { formatBanner } from "../banner.js"; import { CLI_VERSION } from "../constants.js"; @@ -82,6 +77,7 @@ import { precomputeSentryDetection, preReadCommonFiles, } from "./workflow-inputs.js"; +import { reportCliError } from "../error-reporting.js"; type SpinState = { running: boolean }; @@ -923,7 +919,7 @@ async function resumeWithRecovery( }); return recovered; } - captureException(err, { + reportCliError(err, { level: "warning", tags: { "wizard.stale_step_recovery": "failed", diff --git a/packages/cli/src/lib/interactive-login.ts b/packages/cli/src/lib/interactive-login.ts index 948f27000..315c6d845 100644 --- a/packages/cli/src/lib/interactive-login.ts +++ b/packages/cli/src/lib/interactive-login.ts @@ -5,8 +5,6 @@ * Used by both the `auth login` command and auto-auth in bin.ts. */ -// oxlint-disable-next-line sentry-cli/no-namespace-import -- Sentry SDK recommends namespace import -import * as Sentry from "@sentry/node"; import { openBrowser } from "./browser.js"; import { setupCopyKeyListener } from "./clipboard.js"; import { getAccountRow } from "./db/accounts.js"; @@ -17,6 +15,7 @@ import { renderInlineMarkdown } from "./formatters/markdown.js"; import { logger } from "./logger.js"; import { completeOAuthFlow, performDeviceFlow } from "./oauth.js"; import { generateQRCode } from "./qrcode.js"; +import { reportCliError } from "./error-reporting.js"; const log = logger.withTag("auth.login"); @@ -288,7 +287,7 @@ export async function runInteractiveLogin( }); } catch (error) { // Report to Sentry but don't block auth - user info is not critical - Sentry.captureException(error); + reportCliError(error); } } else if (!options?.newAccount) { // The active account's credentials were replaced in place; without @@ -297,7 +296,7 @@ export async function runInteractiveLogin( try { clearUserInfo(); } catch (error) { - Sentry.captureException(error); + reportCliError(error); } } diff --git a/packages/cli/src/lib/mcp/server.ts b/packages/cli/src/lib/mcp/server.ts index 83324e79f..b4558aac4 100644 --- a/packages/cli/src/lib/mcp/server.ts +++ b/packages/cli/src/lib/mcp/server.ts @@ -20,7 +20,6 @@ import { sentryBeforeSend } from "@sentry/mcp-core/telem/sentry"; import { startStdio } from "@sentry/mcp-core/transports/stdio"; import type { CredentialRouter, ServerContext } from "@sentry/mcp-core/types"; import { - captureException, consoleLoggingIntegration, flush, init, @@ -34,6 +33,7 @@ import { logger } from "../logger.js"; import { isTelemetryEnabled } from "../telemetry.js"; import { redactTelemetryEnvelope } from "../telemetry/credential-redaction.js"; import type { McpServerConfig } from "./config.js"; +import { reportCliError } from "../error-reporting.js"; /** Credential wiring supplied by the CLI host. */ export type McpServeOptions = { @@ -318,7 +318,7 @@ export function installProcessHandlers(): ( } // oxlint-disable-next-line sentry-cli/no-silent-catch -- reporting must not block exit. try { - captureException(error); + reportCliError(error); } catch { // reporting must not block exit } diff --git a/packages/cli/src/lib/telemetry.ts b/packages/cli/src/lib/telemetry.ts index 9c5d323c9..2d02974cb 100644 --- a/packages/cli/src/lib/telemetry.ts +++ b/packages/cli/src/lib/telemetry.ts @@ -73,7 +73,7 @@ async function initTelemetryContext(): Promise { } catch (error) { // Context initialization is not critical - continue without it // But capture the error for debugging - Sentry.captureException(error); + reportCliError(error); } } diff --git a/packages/cli/src/lib/version-check.ts b/packages/cli/src/lib/version-check.ts index d3dd708d4..8dc1dc52a 100644 --- a/packages/cli/src/lib/version-check.ts +++ b/packages/cli/src/lib/version-check.ts @@ -32,6 +32,7 @@ import { fetchLatestFromGitHubWithSource, fetchLatestNightlyVersionWithSource, } from "./upgrade.js"; +import { reportCliError } from "./error-reporting.js"; /** Target check interval: ~24 hours */ const CHECK_INTERVAL_MS = 24 * 60 * 60 * 1000; @@ -268,7 +269,7 @@ function checkForUpdateInBackgroundImpl(): void { } } catch (error) { // DB access failed - report to Sentry but don't crash CLI - Sentry.captureException(error); + reportCliError(error); return; } @@ -399,14 +400,14 @@ function getUpdateNotificationWithCopy( try { markUpdateNotified(); } catch (error) { - Sentry.captureException(error); + reportCliError(error); } notifiedThisProcess = true; return notification; } catch (error) { // DB access failed - report to Sentry but don't crash CLI - Sentry.captureException(error); + reportCliError(error); return null; } } diff --git a/packages/cli/test/lib/dsn/fs-utils.test.ts b/packages/cli/test/lib/dsn/fs-utils.test.ts index 2f886c535..7ade8cd69 100644 --- a/packages/cli/test/lib/dsn/fs-utils.test.ts +++ b/packages/cli/test/lib/dsn/fs-utils.test.ts @@ -7,12 +7,12 @@ import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; -const { captureException } = vi.hoisted(() => ({ - captureException: vi.fn(), +const { reportCliError } = vi.hoisted(() => ({ + reportCliError: vi.fn(), })); +vi.mock("../../../src/lib/error-reporting.js", () => ({ reportCliError })); vi.mock("@sentry/node", () => ({ - captureException, startSpan: (_opts: unknown, fn: () => unknown) => fn(), })); @@ -27,11 +27,11 @@ function errnoError(code: string, message?: string): Error { describe("handleFileError", () => { beforeEach(() => { - captureException.mockClear(); + reportCliError.mockClear(); }); afterEach(() => { - captureException.mockClear(); + reportCliError.mockClear(); }); describe("ignorable errors (should NOT report to Sentry)", () => { @@ -40,7 +40,7 @@ describe("handleFileError", () => { operation: "test", path: "/missing", }); - expect(captureException).not.toHaveBeenCalled(); + expect(reportCliError).not.toHaveBeenCalled(); }); test("EACCES — permission denied", () => { @@ -48,7 +48,7 @@ describe("handleFileError", () => { operation: "test", path: "/secret", }); - expect(captureException).not.toHaveBeenCalled(); + expect(reportCliError).not.toHaveBeenCalled(); }); test("EPERM — operation not permitted", () => { @@ -56,7 +56,7 @@ describe("handleFileError", () => { operation: "test", path: "/locked", }); - expect(captureException).not.toHaveBeenCalled(); + expect(reportCliError).not.toHaveBeenCalled(); }); test("EISDIR — path is a directory, not a file", () => { @@ -67,7 +67,7 @@ describe("handleFileError", () => { path: "/project/.env", }, ); - expect(captureException).not.toHaveBeenCalled(); + expect(reportCliError).not.toHaveBeenCalled(); }); test("ENOTDIR — path component is not a directory", () => { @@ -75,7 +75,7 @@ describe("handleFileError", () => { operation: "test", path: "/file.txt/child", }); - expect(captureException).not.toHaveBeenCalled(); + expect(reportCliError).not.toHaveBeenCalled(); }); test("ETIMEDOUT — connection timed out on a network mount", () => { @@ -83,7 +83,7 @@ describe("handleFileError", () => { operation: "scandir", path: "/mnt/cloud-storage", }); - expect(captureException).not.toHaveBeenCalled(); + expect(reportCliError).not.toHaveBeenCalled(); }); test("ENOTCONN — scandir on a disconnected network or FUSE mount", () => { @@ -94,7 +94,7 @@ describe("handleFileError", () => { path: "/mnt/remote", }, ); - expect(captureException).not.toHaveBeenCalled(); + expect(reportCliError).not.toHaveBeenCalled(); }); test("UNKNOWN code — non-POSIX error with UNKNOWN code", () => { @@ -102,7 +102,7 @@ describe("handleFileError", () => { operation: "scandir", path: "/Users/austin/Documents/app-audit.trace", }); - expect(captureException).not.toHaveBeenCalled(); + expect(reportCliError).not.toHaveBeenCalled(); }); test("Unknown system error — macOS non-POSIX errno with no standard code", () => { @@ -114,7 +114,7 @@ describe("handleFileError", () => { operation: "scandir", path: "/Users/austin/Documents/app-audit.trace", }); - expect(captureException).not.toHaveBeenCalled(); + expect(reportCliError).not.toHaveBeenCalled(); }); }); @@ -124,22 +124,22 @@ describe("handleFileError", () => { operation: "test", path: "/disk-fail", }); - expect(captureException).toHaveBeenCalledTimes(1); + expect(reportCliError).toHaveBeenCalledTimes(1); }); test("ENOMEM — out of memory", () => { handleFileError(errnoError("ENOMEM"), { operation: "test" }); - expect(captureException).toHaveBeenCalledTimes(1); + expect(reportCliError).toHaveBeenCalledTimes(1); }); test("generic Error without code", () => { handleFileError(new Error("something broke"), { operation: "test" }); - expect(captureException).toHaveBeenCalledTimes(1); + expect(reportCliError).toHaveBeenCalledTimes(1); }); test("non-Error value", () => { handleFileError("string error", { operation: "test" }); - expect(captureException).toHaveBeenCalledTimes(1); + expect(reportCliError).toHaveBeenCalledTimes(1); }); }); @@ -149,7 +149,7 @@ describe("handleFileError", () => { operation: "checkEnvForDsn", path: "/project/.env", }); - expect(captureException).toHaveBeenCalledWith(error, { + expect(reportCliError).toHaveBeenCalledWith(error, { tags: { operation: "checkEnvForDsn" }, extra: { path: "/project/.env" }, }); diff --git a/packages/cli/test/lib/error-reporting.test.ts b/packages/cli/test/lib/error-reporting.test.ts index 8ae24f937..212588cc4 100644 --- a/packages/cli/test/lib/error-reporting.test.ts +++ b/packages/cli/test/lib/error-reporting.test.ts @@ -697,7 +697,7 @@ describe("reportCliError integration", () => { expect(error).toBeInstanceOf(MalformedAuthTokenError); const { tags, contexts } = capturedScopeTags(error); - expect(captureSpy).toHaveBeenCalledExactlyOnceWith(error); + expect(captureSpy).toHaveBeenCalledExactlyOnceWith(error, undefined); expect(metricSpy).not.toHaveBeenCalled(); expect(tags).toMatchObject({ "cli_error.class": "MalformedAuthTokenError", @@ -715,6 +715,31 @@ describe("reportCliError integration", () => { expect(diagnostics).not.toContain(secondPart); }); + test("passes the caller's tags and extras to the captured event", () => { + const error = new Error("disk failed"); + const context = { + level: "warning" as const, + tags: { "setup.step": "agent-skills" }, + extra: { path: "/tmp/skill" }, + }; + reportCliError(error, context); + expect(captureSpy).toHaveBeenCalledExactlyOnceWith(error, context); + }); + + test("silences a 4xx ApiError even when the caller passes context", () => { + reportCliError(new ApiError("forbidden", 403), { + tags: { "setup.step": "agent-skills" }, + }); + expect(captureSpy).not.toHaveBeenCalled(); + expect(metricSpy).toHaveBeenCalledWith( + "cli.error.silenced", + 1, + expect.objectContaining({ + attributes: expect.objectContaining({ reason: "api_user_error" }), + }), + ); + }); + test("captures ApiError(400) with normalized endpoint tag", () => { const err = new ApiError( "failed", diff --git a/packages/cli/test/lib/init/verify-setup-windows.mocked.test.ts b/packages/cli/test/lib/init/verify-setup-windows.mocked.test.ts index cc181fecb..181741469 100644 --- a/packages/cli/test/lib/init/verify-setup-windows.mocked.test.ts +++ b/packages/cli/test/lib/init/verify-setup-windows.mocked.test.ts @@ -71,9 +71,11 @@ vi.mock("node:child_process", async (importOriginal) => { vi.mock("@sentry/node", () => ({ addBreadcrumb: vi.fn(), - captureException: vi.fn(), setTag: vi.fn(), })); +vi.mock("../../../src/lib/error-reporting.js", () => ({ + reportCliError: vi.fn(), +})); import { verifySetup } from "../../../src/lib/init/verify-setup.js"; diff --git a/packages/cli/test/lib/init/verify-setup.test.ts b/packages/cli/test/lib/init/verify-setup.test.ts index 8dc0d200e..a60e86192 100644 --- a/packages/cli/test/lib/init/verify-setup.test.ts +++ b/packages/cli/test/lib/init/verify-setup.test.ts @@ -7,11 +7,12 @@ import { createMockUI } from "./ui/mock-ui.js"; const sentryMocks = vi.hoisted(() => ({ addBreadcrumb: vi.fn(), - captureException: vi.fn(), setTag: vi.fn(), })); +const { reportCliError } = vi.hoisted(() => ({ reportCliError: vi.fn() })); vi.mock("@sentry/node", () => sentryMocks); +vi.mock("../../../src/lib/error-reporting.js", () => ({ reportCliError })); type FixtureProcesses = { shellPid: number; @@ -24,7 +25,7 @@ let fixtureProcesses: FixtureProcesses | undefined; beforeEach(async () => { sentryMocks.addBreadcrumb.mockClear(); - sentryMocks.captureException.mockClear(); + reportCliError.mockClear(); sentryMocks.setTag.mockClear(); tmpDir = await mkdtemp(join(TEST_TMP_DIR, "verify-setup-test-")); }); @@ -154,7 +155,7 @@ describe("verifySetup", () => { kind: "log.info", message: "Skipping verification — could not detect a dev command", }); - expect(sentryMocks.captureException).not.toHaveBeenCalled(); + expect(reportCliError).not.toHaveBeenCalled(); expect(sentryMocks.setTag).toHaveBeenCalledWith("wizard.verify", "skipped"); expect(sentryMocks.addBreadcrumb).toHaveBeenCalledWith({ category: "wizard.verify", @@ -183,7 +184,7 @@ describe("verifySetup", () => { kind: "log.warn", message: "Skipping verification — could not start the dev command.", }); - expect(sentryMocks.captureException).not.toHaveBeenCalled(); + expect(reportCliError).not.toHaveBeenCalled(); expect(sentryMocks.setTag).toHaveBeenCalledWith("wizard.verify", "skipped"); expect(sentryMocks.addBreadcrumb).toHaveBeenCalledWith({ category: "wizard.verify", diff --git a/packages/cli/test/lib/telemetry.test.ts b/packages/cli/test/lib/telemetry.test.ts index f37aa558c..2b71ccb95 100644 --- a/packages/cli/test/lib/telemetry.test.ts +++ b/packages/cli/test/lib/telemetry.test.ts @@ -325,7 +325,7 @@ describe("withTelemetry", () => { ).rejects.toThrow(error); // Captured via reportCliError → Sentry.withScope expect(withScopeSpy).toHaveBeenCalled(); - expect(captureSpy).toHaveBeenCalledWith(error); + expect(captureSpy).toHaveBeenCalledWith(error, undefined); withScopeSpy.mockRestore(); captureSpy.mockRestore(); }); @@ -393,6 +393,7 @@ describe("withTelemetry", () => { ).rejects.toThrow(MalformedAuthTokenError); expect(captureSpy).toHaveBeenCalledExactlyOnceWith( expect.any(MalformedAuthTokenError), + undefined, ); expect( metricSpy.mock.calls.find((c) => c[0] === "cli.error.silenced"),