Skip to content

Commit eec7396

Browse files
committed
fix: improve Sentry issue grouping to eliminate duplicate issues
- Add cli_error.kind tags for CliError, HostScopeError, WizardError (previously had no grouping key, every unique message = separate issue) - Extract deriveErrorKind() from setGroupingTags() to reduce complexity - Add normalizeEndpoint() for ApiError sub-grouping by endpoint shape - Expand extractResourceKind() to strip 'in <slug>' and entity name slugs - Enrich non-CliError exceptions (TypeError, Error) with kind from message - Drop EBADF errors in beforeSend (same class of OS noise as EPIPE) - Add 34 new tests (unit + property) covering all new grouping logic
1 parent d7440ff commit eec7396

6 files changed

Lines changed: 502 additions & 39 deletions

File tree

.lore.md

Lines changed: 51 additions & 0 deletions
Large diffs are not rendered by default.

src/lib/error-reporting.ts

Lines changed: 124 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,17 @@ import * as Sentry from "@sentry/node-core/light";
2121
import {
2222
ApiError,
2323
AuthError,
24+
CliError,
2425
ContextError,
2526
DeviceFlowError,
27+
HostScopeError,
2628
OutputError,
2729
ResolutionError,
2830
SeerError,
2931
TimeoutError,
3032
UpgradeError,
3133
ValidationError,
34+
WizardError,
3235
} from "./errors.js";
3336

3437
// ---------------------------------------------------------------------------
@@ -101,6 +104,43 @@ function recordSilencedError(error: unknown, reason: SilenceReason): void {
101104
// Grouping tags
102105
// ---------------------------------------------------------------------------
103106

107+
/** Endpoint normalization patterns — compiled once at module scope. */
108+
const ENDPOINT_PATTERNS: [RegExp, string][] = [
109+
[/\/organizations\/[^/]+/, "/organizations/{org}"],
110+
[/\/projects\/[^/]+\/[^/]+/, "/projects/{org}/{project}"],
111+
[/\/issues\/[^/]+/, "/issues/{id}"],
112+
[/\/events\/[^/]+/, "/events/{id}"],
113+
[/\/groups\/[^/]+/, "/groups/{id}"],
114+
[/\/releases\/[^/]+/, "/releases/{version}"],
115+
[/\/teams\/[^/]+\/[^/]+/, "/teams/{org}/{team}"],
116+
[/\/dashboards\/[^/]+/, "/dashboards/{id}"],
117+
[/\/customers\/[^/]+/, "/customers/{org}"],
118+
];
119+
120+
/**
121+
* Strip remaining bare numeric segments (e.g. /12345/) but preserve
122+
* the API version prefix /0/ which is always the second segment.
123+
*/
124+
const BARE_NUMERIC_SEGMENT_RE = /(?<=\/api\/0\/.*)\/\d+(?=\/|$)/g;
125+
126+
/**
127+
* Normalize an API endpoint path by parameterizing variable segments.
128+
*
129+
* Replaces org slugs, project slugs, issue IDs, event IDs, and other
130+
* entity identifiers with placeholders so that server-side fingerprint
131+
* rules can sub-group `ApiError` by endpoint shape rather than exact path.
132+
*
133+
* `"/api/0/projects/my-org/my-project/events/abc123/"` →
134+
* `"/api/0/projects/{org}/{project}/events/{id}/"`
135+
*/
136+
export function normalizeEndpoint(endpoint: string): string {
137+
let result = endpoint;
138+
for (const [pattern, replacement] of ENDPOINT_PATTERNS) {
139+
result = result.replace(pattern, replacement);
140+
}
141+
return result.replace(BARE_NUMERIC_SEGMENT_RE, "/{id}");
142+
}
143+
104144
/**
105145
* Strip quoted substrings, numeric/hex IDs, and org/project paths from a
106146
* resource string to produce a stable "kind" for grouping.
@@ -115,7 +155,15 @@ export function extractResourceKind(resource: string): string {
115155
.replace(/'[^']*'/g, "")
116156
.replace(/"[^"]*"/g, "")
117157
.replace(/\b[0-9a-f]{16,32}\b/gi, "")
118-
.replace(/\bin\s+[\w-]+\/[\w-]+/g, "")
158+
.replace(/\bin\s+[\w-]+(?:\/[\w-]+)*/g, "")
159+
// Strip hyphenated slugs after known entity names (e.g., "Organization my-company").
160+
// Requires at least one hyphen to avoid stripping English words ("Project not found").
161+
// Safe for current callers: resource values with slugs use quotes (stripped above),
162+
// and headline values don't start with entity names.
163+
.replace(
164+
/\b(Organization|Dashboard|Dashboards|Project|Team)\s+[\w][\w-]*-[\w-]*/gi,
165+
"$1"
166+
)
119167
.replace(/\b\d+\b/g, "")
120168
.replace(/\s+/g, " ")
121169
.trim();
@@ -140,6 +188,64 @@ export function extractMessagePrefix(message: string, maxWords = 3): string {
140188
.join(" ");
141189
}
142190

191+
/**
192+
* Derive a stable `cli_error.kind` grouping key from an error instance.
193+
*
194+
* Returns `undefined` when the error is not a recognized CLI error class
195+
* (the caller should still set `cli_error.class` for basic grouping).
196+
*/
197+
function deriveErrorKind(error: Error): string | undefined {
198+
if (error instanceof ContextError) {
199+
return error.resource;
200+
}
201+
if (error instanceof ResolutionError) {
202+
return (
203+
extractResourceKind(error.resource) +
204+
" " +
205+
extractResourceKind(error.headline)
206+
);
207+
}
208+
// Fall back to the first few words of the message when no field is set
209+
// (e.g. validateHexId throws with no `field`, so using field would
210+
// collapse every unfielded ValidationError into one group).
211+
if (error instanceof ValidationError) {
212+
return error.field ?? extractMessagePrefix(error.message);
213+
}
214+
if (error instanceof ApiError) {
215+
return String(error.status);
216+
}
217+
if (error instanceof SeerError) {
218+
return error.reason;
219+
}
220+
if (error instanceof AuthError) {
221+
return error.reason;
222+
}
223+
if (error instanceof UpgradeError) {
224+
return error.reason;
225+
}
226+
if (error instanceof DeviceFlowError) {
227+
return error.code;
228+
}
229+
if (error instanceof TimeoutError) {
230+
return "timeout";
231+
}
232+
if (error instanceof HostScopeError) {
233+
return "host_scope";
234+
}
235+
if (error instanceof WizardError) {
236+
return "wizard";
237+
}
238+
// Catch-all for bare CliError — must be checked AFTER all subclasses
239+
// because instanceof matches the entire prototype chain.
240+
// ConfigError and OutputError intentionally fall through here:
241+
// ConfigError has no structured field beyond message; OutputError is
242+
// silenced by classifySilenced() before reaching deriveErrorKind().
243+
if (error instanceof CliError) {
244+
return extractMessagePrefix(error.message, 4);
245+
}
246+
return;
247+
}
248+
143249
/**
144250
* Set `cli_error.*` tags on a Sentry scope for an error that will be
145251
* captured. These tags are matched by server-side fingerprint rules to
@@ -149,6 +255,7 @@ export function extractMessagePrefix(message: string, maxWords = 3): string {
149255
* - `cli_error.class` — error class name (e.g. `"ContextError"`)
150256
* - `cli_error.kind` — stable grouping key derived from structured fields
151257
* - `cli_error.api_status` — HTTP status (ApiError only)
258+
* - `cli_error.api_endpoint` — normalized API path (ApiError only)
152259
*/
153260
function setGroupingTags(scope: Sentry.Scope, error: unknown): void {
154261
if (!(error instanceof Error)) {
@@ -157,36 +264,16 @@ function setGroupingTags(scope: Sentry.Scope, error: unknown): void {
157264

158265
scope.setTag("cli_error.class", error.name);
159266

160-
if (error instanceof ContextError) {
161-
scope.setTag("cli_error.kind", error.resource);
162-
} else if (error instanceof ResolutionError) {
163-
scope.setTag(
164-
"cli_error.kind",
165-
extractResourceKind(error.resource) +
166-
" " +
167-
extractResourceKind(error.headline)
168-
);
169-
} else if (error instanceof ValidationError) {
170-
// Fall back to the first few words of the message when no field is set
171-
// (e.g. validateHexId throws with no `field`, so using field would
172-
// collapse every unfielded ValidationError into one group).
173-
scope.setTag(
174-
"cli_error.kind",
175-
error.field ?? extractMessagePrefix(error.message)
176-
);
177-
} else if (error instanceof ApiError) {
267+
const kind = deriveErrorKind(error);
268+
if (kind !== undefined) {
269+
scope.setTag("cli_error.kind", kind);
270+
}
271+
272+
if (error instanceof ApiError) {
178273
scope.setTag("cli_error.api_status", String(error.status));
179-
scope.setTag("cli_error.kind", String(error.status));
180-
} else if (error instanceof SeerError) {
181-
scope.setTag("cli_error.kind", error.reason);
182-
} else if (error instanceof AuthError) {
183-
scope.setTag("cli_error.kind", error.reason);
184-
} else if (error instanceof UpgradeError) {
185-
scope.setTag("cli_error.kind", error.reason);
186-
} else if (error instanceof DeviceFlowError) {
187-
scope.setTag("cli_error.kind", error.code);
188-
} else if (error instanceof TimeoutError) {
189-
scope.setTag("cli_error.kind", "timeout");
274+
if (error.endpoint) {
275+
scope.setTag("cli_error.api_endpoint", normalizeEndpoint(error.endpoint));
276+
}
190277
}
191278
}
192279

@@ -273,5 +360,12 @@ export function enrichEventWithGroupingTags(
273360
event.tags = event.tags ?? {};
274361
event.tags["cli_error.class"] = exc.type;
275362

363+
// Set kind from exception message prefix so server-side rules can group
364+
// non-CliError exceptions (TypeError, Error, WizardCancelledError, etc.)
365+
// that bypass reportCliError (uncaught exceptions, unhandled rejections).
366+
if (exc.value) {
367+
event.tags["cli_error.kind"] = extractMessagePrefix(exc.value, 4);
368+
}
369+
276370
return event;
277371
}

src/lib/telemetry.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,35 @@ export function isEpipeError(event: Sentry.ErrorEvent): boolean {
313313
return false;
314314
}
315315

316+
/**
317+
* Detect EBADF (bad file descriptor) errors in Sentry events.
318+
*
319+
* These occur when the init wizard's stdin reopen (`stdin-reopen.ts`) queues
320+
* reads on a destroyed file descriptor. Same class of OS-level noise as EPIPE
321+
* — not actionable, just different fd numbers producing duplicate issues.
322+
*
323+
* @internal Exported for testing
324+
*/
325+
export function isEbadfError(event: Sentry.ErrorEvent): boolean {
326+
const exceptions = event.exception?.values;
327+
if (exceptions) {
328+
for (const ex of exceptions) {
329+
if (ex.value?.includes("EBADF")) {
330+
return true;
331+
}
332+
}
333+
}
334+
335+
const systemError = event.contexts?.node_system_error as
336+
| { code?: string }
337+
| undefined;
338+
if (systemError?.code === "EBADF") {
339+
return true;
340+
}
341+
342+
return false;
343+
}
344+
316345
/**
317346
* Check if an error is a user-caused (401–499) API error.
318347
*
@@ -612,6 +641,13 @@ export function initSentry(
612641
return null;
613642
}
614643

644+
// EBADF errors come from the init wizard's stdin reopen queuing reads
645+
// on a destroyed fd. Same class of OS-level noise as EPIPE — different
646+
// fd numbers just produce duplicate issues. Not actionable — drop them.
647+
if (isEbadfError(event)) {
648+
return null;
649+
}
650+
615651
// Normalize relative frame paths to absolute. Bun's compiled binaries
616652
// with sourcemap: "linked" produce relative paths like "dist-bin/bin.js"
617653
// in Error.stack. Sentry's symbolicator only matches absolute paths

test/lib/error-reporting.property.test.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ const slugArb = array(
2525
.map((chars) => chars.join(""))
2626
.filter((s) => !(s.startsWith("-") || s.endsWith("-")) && s.length > 0);
2727

28+
/** Slug that contains at least one hyphen (matches the entity-name strip regex). */
29+
const hyphenatedSlugArb = slugArb.filter((s) => s.includes("-"));
30+
2831
/** 32-character lowercase hex id (trace/event/log id). */
2932
const hexIdArb = stringMatching(/^[0-9a-f]{32}$/).filter(
3033
(s) => s.length === 32
@@ -130,4 +133,45 @@ describe("extractResourceKind — property tests", () => {
130133
{ numRuns: DEFAULT_NUM_RUNS }
131134
);
132135
});
136+
137+
test("bare slug after 'in' (no slash) is stripped for any slug", () => {
138+
fcAssert(
139+
property(slugArb, slugArb, (a, b) => {
140+
expect(extractResourceKind(`not found in ${a}`)).toBe(
141+
extractResourceKind(`not found in ${b}`)
142+
);
143+
expect(extractResourceKind(`not found in ${a}`)).toBe("not found");
144+
}),
145+
{ numRuns: DEFAULT_NUM_RUNS }
146+
);
147+
});
148+
149+
test("hyphenated slug after entity name is stripped for any slug", () => {
150+
fcAssert(
151+
property(hyphenatedSlugArb, hyphenatedSlugArb, (a, b) => {
152+
expect(extractResourceKind(`Organization ${a}`)).toBe(
153+
extractResourceKind(`Organization ${b}`)
154+
);
155+
expect(extractResourceKind(`Organization ${a}`)).toBe("Organization");
156+
}),
157+
{ numRuns: DEFAULT_NUM_RUNS }
158+
);
159+
});
160+
161+
test("Dashboard with numeric ID and slug produces same kind", () => {
162+
fcAssert(
163+
property(
164+
numericIdArb,
165+
slugArb,
166+
numericIdArb,
167+
slugArb,
168+
(n1, s1, n2, s2) => {
169+
expect(extractResourceKind(`Dashboard ${n1} in ${s1}`)).toBe(
170+
extractResourceKind(`Dashboard ${n2} in ${s2}`)
171+
);
172+
}
173+
),
174+
{ numRuns: DEFAULT_NUM_RUNS }
175+
);
176+
});
133177
});

0 commit comments

Comments
 (0)