diff --git a/src/lib/error-reporting.ts b/src/lib/error-reporting.ts index 03cc87c28..f3004786b 100644 --- a/src/lib/error-reporting.ts +++ b/src/lib/error-reporting.ts @@ -3,9 +3,10 @@ * * Provides two things: * - * 1. **Silencing rules** — `OutputError`, expected-state `AuthError`, and - * 401–499 `ApiError` are not sent to Sentry as issues. A - * `cli.error.silenced` metric preserves volume + user/org context. + * 1. **Silencing rules** — `OutputError`, `ContextError` (a required value the + * user omitted), expected-state `AuthError`, and 401–499 `ApiError` are not + * sent to Sentry as issues. A `cli.error.silenced` metric preserves volume + + * user/org context. * * 2. **Grouping tags** — enriches every error event with `cli_error.*` tags * that Sentry's server-side fingerprint rules use for stable grouping. @@ -42,7 +43,11 @@ import { * Reasons an error may be silenced (not sent to Sentry as an issue). * Exposed as the `reason` attribute on the `cli.error.silenced` metric. */ -type SilenceReason = "output_error" | "auth_expected" | "api_user_error"; +type SilenceReason = + | "output_error" + | "context_missing" + | "auth_expected" + | "api_user_error"; /** * Classify whether an error should be silenced. @@ -54,6 +59,16 @@ export function classifySilenced(error: unknown): SilenceReason | null { if (error instanceof OutputError) { return "output_error"; } + // A ContextError always means the user omitted a required value (no + // org/project could be resolved, a required ID was not provided, etc.). It is + // never a CLI bug — unlike ResolutionError, where a *provided* value that + // can't be matched may signal a product/access issue worth observing. There + // is nothing per-instance to investigate, so silence the whole class; the + // `cli.error.silenced` metric (keyed by `error_class` + `resource`) preserves + // the volume and which value was missing. (CLI-3B: ~2000 users.) + if (error instanceof ContextError) { + return "context_missing"; + } if ( error instanceof AuthError && (error.reason === "not_authenticated" || error.reason === "expired") @@ -78,6 +93,12 @@ function recordSilencedError(error: unknown, reason: SilenceReason): void { if (error instanceof AuthError) { attributes.auth_reason = error.reason; } + // Preserve which required value was missing so the silenced-volume metric + // keeps sub-grouping context (e.g. "Organization and project" vs "Event ID"). + // ContextError resources are a small fixed set, so cardinality stays low. + if (error instanceof ContextError) { + attributes.resource = error.resource; + } try { Sentry.metrics.distribution("cli.error.silenced", 1, { attributes }); diff --git a/src/lib/telemetry.ts b/src/lib/telemetry.ts index 5876fa2d7..9bdd1529b 100644 --- a/src/lib/telemetry.ts +++ b/src/lib/telemetry.ts @@ -226,14 +226,15 @@ export async function withTelemetry( } ); } catch (e) { - // Route through reportCliError so silencing (OutputError, expected-auth - // AuthError, 401–499 ApiError) and fingerprint normalization are applied - // consistently. Silenced errors emit a `cli.error.silenced` metric + - // optional structured log instead of creating a Sentry issue. + // Route through reportCliError so silencing (OutputError, ContextError, + // expected-auth AuthError, 401–499 ApiError) and fingerprint normalization + // are applied consistently. Silenced errors emit a `cli.error.silenced` + // metric + optional structured log instead of creating a Sentry issue. reportCliError(e); // Only mark session crashed for errors that weren't silenced. - // Silenced errors (OutputError, expected AuthError, user 4xx ApiError) - // are expected states — marking them crashed would skew release-health. + // Silenced errors (OutputError, ContextError, expected AuthError, user 4xx + // ApiError) are expected states — marking them crashed would skew + // release-health. if (!classifySilenced(e)) { markSessionCrashed(); } diff --git a/test/lib/error-reporting.test.ts b/test/lib/error-reporting.test.ts index 731d3c072..95db79506 100644 --- a/test/lib/error-reporting.test.ts +++ b/test/lib/error-reporting.test.ts @@ -245,7 +245,13 @@ describe("classifySilenced", () => { }); test.each([ - ["ContextError", new ContextError("Organization", "sentry org view ")], + ["auto-detect failure", new ContextError("Organization and project", "x")], + ["missing ID", new ContextError("Event ID", "sentry event view ", [])], + ])("silences ContextError (%s)", (_label, err) => { + expect(classifySilenced(err)).toBe("context_missing"); + }); + + test.each([ [ "ResolutionError", new ResolutionError("Project 'x'", "not found", "sentry issue list"), @@ -402,12 +408,22 @@ describe("reportCliError integration", () => { return { tags, contexts }; } - test("captures ContextError with scope (tags applied)", () => { - const err = new ContextError("Organization", "sentry org view "); - reportCliError(err); - expect(captureSpy).toHaveBeenCalledWith(err); - expect(withScopeSpy).toHaveBeenCalled(); - expect(metricSpy).not.toHaveBeenCalled(); + test("silences ContextError and emits metric with resource", () => { + reportCliError( + new ContextError("Organization and project", "sentry org view ") + ); + expect(captureSpy).not.toHaveBeenCalled(); + expect(metricSpy).toHaveBeenCalledWith( + "cli.error.silenced", + 1, + expect.objectContaining({ + attributes: expect.objectContaining({ + error_class: "ContextError", + reason: "context_missing", + resource: "Organization and project", + }), + }) + ); }); test("ValidationError with field uses field as kind", () => { diff --git a/test/lib/telemetry.test.ts b/test/lib/telemetry.test.ts index 8d1321361..c32271342 100644 --- a/test/lib/telemetry.test.ts +++ b/test/lib/telemetry.test.ts @@ -289,7 +289,7 @@ describe("withTelemetry", () => { captureSpy.mockRestore(); }); - test("captures ContextError with fingerprint (no silencing)", async () => { + test("silences ContextError (missing user input, not a crash)", async () => { const captureSpy = vi.spyOn(Sentry, "captureException"); const metricSpy = vi.spyOn(Sentry.metrics, "distribution"); const { ContextError } = await import("../../src/lib/errors.js"); @@ -302,12 +302,13 @@ describe("withTelemetry", () => { throw error; }) ).rejects.toThrow(error); - expect(captureSpy).toHaveBeenCalledWith(error); - // No silencing metric for captured errors + // ContextError is an expected "missing input" error — not reported as a + // Sentry issue; volume is preserved via the cli.error.silenced metric. + expect(captureSpy).not.toHaveBeenCalled(); const silencedCall = metricSpy.mock.calls.find( (c) => c[0] === "cli.error.silenced" ); - expect(silencedCall).toBeUndefined(); + expect(silencedCall).toBeDefined(); captureSpy.mockRestore(); metricSpy.mockRestore(); });