Skip to content

Commit bcb9beb

Browse files
authored
fix(telemetry): exclude OutputError from Sentry exception capture (CLI-PK) (#629)
## Summary - **Exclude `OutputError` from `Sentry.captureException()`** in `withTelemetry`'s outer catch block. `OutputError` is an intentional non-zero exit mechanism (e.g., `sentry api` got a 4xx/5xx response), not a CLI bug. This eliminates ~1,335 noise events across 160 users ([CLI-PK](https://sentry.sentry.io/issues/7367976863/)). - **Add structured `api_error.*` span attributes** (`status`, `endpoint`, `method`) on the active telemetry span before throwing `OutputError` in the `sentry api` command, preserving Discover queryability for API error rates. - **Add tests** verifying `OutputError` is not captured as a Sentry exception. ## Changes | File | Change | |------|--------| | `src/lib/telemetry.ts` | Add `OutputError` to exclusion check alongside `AuthError` and client `ApiError` | | `src/commands/api.ts` | Add `recordApiErrorAttributes()` helper; record span attributes on 4xx/5xx | | `test/lib/telemetry.test.ts` | Add 2 tests for `OutputError` exclusion (null data + object data) | Fixes CLI-PK
1 parent 42c6e5e commit bcb9beb

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";
@@ -1036,6 +1038,28 @@ export async function resolveBody(
10361038

10371039
// Command Definition
10381040

1041+
/**
1042+
* Record API error attributes on the active telemetry span.
1043+
*
1044+
* Since `OutputError` is excluded from `Sentry.captureException` (it signals
1045+
* an intentional non-zero exit, not a CLI bug), these span attributes are the
1046+
* only telemetry signal for `sentry api` HTTP errors. They're queryable in
1047+
* Discover via `api_error.status`, matching the pattern used by
1048+
* `recordApiErrorOnSpan` for `ApiError`-based errors in other commands.
1049+
*/
1050+
function recordApiErrorAttributes(
1051+
status: number,
1052+
endpoint: string,
1053+
method: string
1054+
): void {
1055+
const span = Sentry.getActiveSpan();
1056+
if (span) {
1057+
span.setAttribute("api_error.status", status);
1058+
span.setAttribute("api_error.endpoint", endpoint);
1059+
span.setAttribute("api_error.method", method);
1060+
}
1061+
}
1062+
10391063
/** Log outgoing request details in `> ` curl-verbose style. */
10401064
function logRequest(
10411065
method: 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";
@@ -150,7 +150,12 @@ export async function withTelemetry<T>(
150150
(e.reason === "not_authenticated" || e.reason === "expired");
151151
// 4xx API errors are user errors (wrong ID, no access), not CLI bugs.
152152
// They're recorded as span attributes above for volume-spike detection.
153-
if (!(isExpectedAuthState || isClientApiError(e))) {
153+
// OutputError is an intentional non-zero exit (e.g., `sentry api` got a
154+
// 4xx/5xx response) — not a CLI bug. Error details are recorded as span
155+
// attributes by the command itself.
156+
if (
157+
!(isExpectedAuthState || isClientApiError(e) || e instanceof OutputError)
158+
) {
154159
Sentry.captureException(e);
155160
markSessionCrashed();
156161
}

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)