From 4e6256674d0fd9d22d87315131a86fa7d9ae899a Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Tue, 21 Apr 2026 09:45:26 +0000 Subject: [PATCH] fix(arg-parsing): throw ValidationError (not raw Error) for format errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 9 sites in src/lib/arg-parsing.ts used `throw new Error(...)` for user-input validation errors (invalid issue format, invalid sort value, --limit out of range). These escape the error-reporting pipeline as plain `Error` instances, so: 1. They lack the `cli_error.class:"ValidationError"` tag, so the server-side fingerprint rule does not fire and they group as generic errors in Sentry. 2. They are not silenced by the 401-499 user-error filter (not that the filter would apply here — but the semantics are wrong). 3. The `CliError` formatting path does not render them the way ValidationErrors are rendered. Observed as CLI-1C9 today: a user-input 'Invalid issue format: "XQUIK-"' event created a new error group tagged `class=Error` instead of joining the canonical ValidationError parent. Fix: replace all 9 raw `throw new Error(...)` calls with `throw new ValidationError(...)` carrying a `field` label so the fingerprint rule's `{{ tags.cli_error.kind }}` slot is populated. Added a regression test in arg-parsing.test.ts that asserts `parseIssueArg` failure modes throw `ValidationError` instances (not just the message text). --- src/lib/arg-parsing.ts | 42 ++++++++++++++++++++++-------------- test/lib/arg-parsing.test.ts | 14 ++++++++++++ 2 files changed, 40 insertions(+), 16 deletions(-) diff --git a/src/lib/arg-parsing.ts b/src/lib/arg-parsing.ts index 18e81a0b0..f2f626137 100644 --- a/src/lib/arg-parsing.ts +++ b/src/lib/arg-parsing.ts @@ -237,7 +237,10 @@ export function detectSwappedTrialArgs( export function validateLimit(value: string, min = 1, max = 1000): number { const num = Number.parseInt(value, 10); if (Number.isNaN(num) || num < min || num > max) { - throw new Error(`--limit must be between ${min} and ${max}`); + throw new ValidationError( + `--limit must be between ${min} and ${max}`, + "limit" + ); } return num; } @@ -257,8 +260,9 @@ const VALID_LOG_SORT_DIRECTIONS: readonly LogSortDirection[] = [ */ export function parseLogSort(value: string): LogSortDirection { if (!VALID_LOG_SORT_DIRECTIONS.includes(value as LogSortDirection)) { - throw new Error( - `Invalid sort value. Must be one of: ${VALID_LOG_SORT_DIRECTIONS.join(", ")}` + throw new ValidationError( + `Invalid sort value. Must be one of: ${VALID_LOG_SORT_DIRECTIONS.join(", ")}`, + "sort" ); } return value as LogSortDirection; @@ -495,7 +499,7 @@ function parseSlashOrgProject(input: string): ParsedOrgProject { if (!rawOrg) { // "/cli" → search for project across all orgs if (!rawProject) { - throw new Error( + throw new ValidationError( 'Invalid format: "/" requires a project slug (e.g., "/cli")' ); } @@ -705,8 +709,9 @@ function parseMultiSlashIssueArg( const remainder = rest.slice(slashIdx + 1); if (!(project && remainder)) { - throw new Error( - `Invalid issue format: "${arg}". Missing project or issue ID segment.` + throw new ValidationError( + `Invalid issue format: "${arg}". Missing project or issue ID segment.`, + "issue" ); } @@ -781,14 +786,16 @@ function parseAfterSlash( const suffix = rest.slice(lastDash + 1).toUpperCase(); if (!project) { - throw new Error( - `Invalid issue format: "${arg}". Cannot use trailing slash before suffix.` + throw new ValidationError( + `Invalid issue format: "${arg}". Cannot use trailing slash before suffix.`, + "issue" ); } if (!suffix) { - throw new Error( - `Invalid issue format: "${arg}". Missing suffix after dash.` + throw new ValidationError( + `Invalid issue format: "${arg}". Missing suffix after dash.`, + "issue" ); } @@ -810,8 +817,9 @@ function parseWithSlash(arg: string): ParsedIssueArg { const rest = arg.slice(slashIdx + 1); if (!rest) { - throw new Error( - `Invalid issue format: "${arg}". Missing issue ID after slash.` + throw new ValidationError( + `Invalid issue format: "${arg}". Missing issue ID after slash.`, + "issue" ); } @@ -836,14 +844,16 @@ function parseWithDash(arg: string): ParsedIssueArg { const suffix = arg.slice(lastDash + 1).toUpperCase(); if (!projectSlug) { - throw new Error( - `Invalid issue format: "${arg}". Missing project before suffix.` + throw new ValidationError( + `Invalid issue format: "${arg}". Missing project before suffix.`, + "issue" ); } if (!suffix) { - throw new Error( - `Invalid issue format: "${arg}". Missing suffix after dash.` + throw new ValidationError( + `Invalid issue format: "${arg}". Missing suffix after dash.`, + "issue" ); } diff --git a/test/lib/arg-parsing.test.ts b/test/lib/arg-parsing.test.ts index fde7a3261..6d9361ad0 100644 --- a/test/lib/arg-parsing.test.ts +++ b/test/lib/arg-parsing.test.ts @@ -347,6 +347,20 @@ describe("parseIssueArg", () => { test("just slash throws error", () => { expect(() => parseIssueArg("/")).toThrow("Missing issue ID after slash"); }); + + test("invalid issue format throws ValidationError (not raw Error)", () => { + // Ensures the fingerprint rule's `cli_error.class:"ValidationError"` tag + // fires for these user-input errors, so they group under the canonical + // ValidationError parent instead of as generic `Error` issues in Sentry. + // Regression: CLI-1C9 was created because `parseIssueArg` used raw + // `throw new Error(...)` instead of `ValidationError` for format errors. + expect(() => parseIssueArg("XQUIK-")).toThrow(ValidationError); + expect(() => parseIssueArg("cli-")).toThrow(ValidationError); + expect(() => parseIssueArg("sentry/")).toThrow(ValidationError); + expect(() => parseIssueArg("sentry/-G")).toThrow(ValidationError); + expect(() => parseIssueArg("-G")).toThrow(ValidationError); + expect(() => parseIssueArg("/")).toThrow(ValidationError); + }); }); // URL integration tests — applySentryUrlContext may set SENTRY_HOST/SENTRY_URL as a side effect