Skip to content

Commit f459b89

Browse files
committed
fix(telemetry): exclude OutputError from Sentry exception capture (CLI-PK)
OutputError is an intentional non-zero exit mechanism, not a CLI bug. It was being captured by Sentry.captureException() in withTelemetry, creating 1,335 noise events across 160 users. - Add OutputError to the exclusion list in withTelemetry's catch block - Record api_error.* span attributes in sentry api command before throwing OutputError, preserving Discover queryability - Add tests for OutputError exclusion
1 parent 7ce9bdf commit f459b89

File tree

3 files changed

+67
-3
lines changed

3 files changed

+67
-3
lines changed

src/commands/api.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
* Similar to 'gh api' for GitHub.
66
*/
77

8+
// biome-ignore lint/performance/noNamespaceImport: Sentry SDK recommends namespace import
9+
import * as Sentry from "@sentry/node-core/light";
810
import type { SentryContext } from "../context.js";
911
import { buildSearchParams, rawApiRequest } from "../lib/api-client.js";
1012
import { buildCommand } from "../lib/command.js";
@@ -1037,6 +1039,28 @@ export async function resolveBody(
10371039
// Command Definition
10381040

10391041
/** Log outgoing request details in `> ` curl-verbose style. */
1042+
/**
1043+
* Record API error attributes on the active telemetry span.
1044+
*
1045+
* Since `OutputError` is excluded from `Sentry.captureException` (it signals
1046+
* an intentional non-zero exit, not a CLI bug), these span attributes are the
1047+
* only telemetry signal for `sentry api` HTTP errors. They're queryable in
1048+
* Discover via `api_error.status`, matching the pattern used by
1049+
* `recordApiErrorOnSpan` for `ApiError`-based errors in other commands.
1050+
*/
1051+
function recordApiErrorAttributes(
1052+
status: number,
1053+
endpoint: string,
1054+
method: string
1055+
): void {
1056+
const span = Sentry.getActiveSpan();
1057+
if (span) {
1058+
span.setAttribute("api_error.status", status);
1059+
span.setAttribute("api_error.endpoint", endpoint);
1060+
span.setAttribute("api_error.method", method);
1061+
}
1062+
}
1063+
10401064
function logRequest(
10411065
method: string,
10421066
endpoint: string,
@@ -1217,6 +1241,17 @@ export const apiCommand = buildCommand({
12171241
logResponse(response);
12181242
}
12191243

1244+
// Record API error attributes on the active span for Discover queryability.
1245+
// OutputError is excluded from Sentry.captureException (it's an intentional
1246+
// non-zero exit, not a bug), so span attributes are the only telemetry signal.
1247+
if (isError) {
1248+
recordApiErrorAttributes(
1249+
response.status,
1250+
normalizedEndpoint,
1251+
flags.method
1252+
);
1253+
}
1254+
12201255
// Silent mode — no output, just exit code
12211256
if (flags.silent) {
12221257
if (isError) {

src/lib/telemetry.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import {
2020
} from "./constants.js";
2121
import { isReadonlyError, tryRepairAndRetry } from "./db/schema.js";
2222
import { getEnv } from "./env.js";
23-
import { ApiError, AuthError } from "./errors.js";
23+
import { ApiError, AuthError, OutputError } from "./errors.js";
2424
import { attachSentryReporter } from "./logger.js";
2525
import { getSentryBaseUrl, isSentrySaasUrl } from "./sentry-urls.js";
2626
import { getRealUsername } from "./utils.js";
@@ -145,7 +145,12 @@ export async function withTelemetry<T>(
145145
(e.reason === "not_authenticated" || e.reason === "expired");
146146
// 4xx API errors are user errors (wrong ID, no access), not CLI bugs.
147147
// They're recorded as span attributes above for volume-spike detection.
148-
if (!(isExpectedAuthState || isClientApiError(e))) {
148+
// OutputError is an intentional non-zero exit (e.g., `sentry api` got a
149+
// 4xx/5xx response) — not a CLI bug. Error details are recorded as span
150+
// attributes by the command itself.
151+
if (
152+
!(isExpectedAuthState || isClientApiError(e) || e instanceof OutputError)
153+
) {
149154
Sentry.captureException(e);
150155
markSessionCrashed();
151156
}

test/lib/telemetry.test.ts

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import {
1717
import { chmodSync, mkdirSync, rmSync } from "node:fs";
1818
// biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking
1919
import * as Sentry from "@sentry/node-core/light";
20-
import { ApiError, AuthError } from "../../src/lib/errors.js";
20+
import { ApiError, AuthError, OutputError } from "../../src/lib/errors.js";
2121
import {
2222
createTracedDatabase,
2323
getSentryTracePropagationTargets,
@@ -208,6 +208,30 @@ describe("withTelemetry", () => {
208208
const result = await withTelemetry(() => 42);
209209
expect(result).toBe(42);
210210
});
211+
212+
test("does not capture OutputError (intentional exit-code mechanism)", async () => {
213+
const captureSpy = spyOn(Sentry, "captureException");
214+
const error = new OutputError(null);
215+
await expect(
216+
withTelemetry(() => {
217+
throw error;
218+
})
219+
).rejects.toThrow(error);
220+
expect(captureSpy).not.toHaveBeenCalled();
221+
captureSpy.mockRestore();
222+
});
223+
224+
test("does not capture OutputError with data", async () => {
225+
const captureSpy = spyOn(Sentry, "captureException");
226+
const error = new OutputError({ error: "not found" });
227+
await expect(
228+
withTelemetry(() => {
229+
throw error;
230+
})
231+
).rejects.toThrow(error);
232+
expect(captureSpy).not.toHaveBeenCalled();
233+
captureSpy.mockRestore();
234+
});
211235
});
212236
});
213237

0 commit comments

Comments
 (0)