Skip to content

Commit b14c22b

Browse files
authored
fix(api): throw ValidationError for user-input failures in api command (CLI-1GC) (#855)
## Summary Fixes [CLI-1GC](https://sentry.sentry.io/issues/7444602558/): user runs `sentry api ... --input /tmp/missing.json`, sees `Unexpected error: Error: File not found: ...` with a full stack trace, and the error gets captured to Sentry as a CLI bug. ## Root cause `buildBodyFromInput()` in `src/commands/api.ts` throws a plain `new Error()` when the `--input` file doesn't exist. Because this runs inside the command's `func()` body (not a Stricli `parse` callback), it bypasses the `CliError` handling chain in `app.ts`: 1. `classifySilenced()` doesn't recognize plain `Error` → `Sentry.captureException` fires → reported as a CLI bug. 2. `exc instanceof CliError` is `false` → user sees `Unexpected error:` + stack trace instead of a clean message. This is the same pattern documented in the lore entry _"api.ts: plain Error throws inside func() bypass CliError handling"_ — same file, same kind of bug across 7 call sites. ## Fix Convert all 7 `func()`-path `throw new Error(...)` sites in `src/commands/api.ts` to `throw new ValidationError(..., field)`: | Function | `field` | |----------|---------| | `buildBodyFromInput` (file not found) | `"input"` | | `parseHeaders` (bad header format) | `"header"` | | `parseFieldKey` (bad key format) | `"field"` | | `validatePathSegments` (dangerous/invalid keys) | `"field"` | | `validateTypeCompatibility` (type conflicts) | `"field"` | `parseMethod` (line 74) keeps `new Error()` — it runs in a Stricli `parse` callback where Stricli catches and formats it. After the fix: - User sees a clean `Error: File not found: /tmp/missing.json` (no stack trace). - The error is silenced from Sentry telemetry as an expected user error. - The `field` metadata enables stable Sentry grouping if any of these surfaces do legitimately need attention. ## Test plan - Updated existing `buildBodyFromInput` "throws for non-existent file" test to assert `instanceof ValidationError`. - `bun test test/commands/api.test.ts` — 203 pass. - `bun test test/commands/api.property.test.ts` — 50 pass. - `bun run lint`, `bun run typecheck` — clean.
1 parent 16e853b commit b14c22b

2 files changed

Lines changed: 21 additions & 12 deletions

File tree

src/commands/api.ts

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ const BRACKET_CONTENTS_REGEX = /\[([^[\]]*)\]/g;
164164
export function parseFieldKey(key: string): string[] {
165165
const match = key.match(FIELD_KEY_REGEX);
166166
if (!match?.[1]) {
167-
throw new Error(`Invalid field key format: ${key}`);
167+
throw new ValidationError(`Invalid field key format: ${key}`, "field");
168168
}
169169

170170
const baseKey = match[1];
@@ -190,14 +190,18 @@ function validatePathSegments(path: string[]): void {
190190

191191
// Check for prototype pollution
192192
if (DANGEROUS_KEYS.has(segment)) {
193-
throw new Error(`Invalid field key: "${segment}" is not allowed`);
193+
throw new ValidationError(
194+
`Invalid field key: "${segment}" is not allowed`,
195+
"field"
196+
);
194197
}
195198

196199
// Empty brackets ("") are only valid at the end of the path (array push syntax)
197200
// Reject patterns like a[][b] which would silently lose data
198201
if (segment === "" && i < path.length - 1) {
199-
throw new Error(
200-
"Invalid field key: empty brackets [] can only appear at the end of a key"
202+
throw new ValidationError(
203+
"Invalid field key: empty brackets [] can only appear at the end of a key",
204+
"field"
201205
);
202206
}
203207
}
@@ -249,14 +253,16 @@ function validateTypeCompatibility(
249253
const pathStr = formatPathForError(path, index);
250254

251255
if (expectsArray && !Array.isArray(existing)) {
252-
throw new Error(
253-
`expected array type under "${pathStr}", got ${getTypeName(existing)}`
256+
throw new ValidationError(
257+
`expected array type under "${pathStr}", got ${getTypeName(existing)}`,
258+
"field"
254259
);
255260
}
256261

257262
if (!(expectsArray || isTraversableObject(existing))) {
258-
throw new Error(
259-
`expected map type under "${pathStr}", got ${getTypeName(existing)}`
263+
throw new ValidationError(
264+
`expected map type under "${pathStr}", got ${getTypeName(existing)}`,
265+
"field"
260266
);
261267
}
262268
}
@@ -628,7 +634,10 @@ export function parseHeaders(headers: string[]): Record<string, string> {
628634
for (const header of headers) {
629635
const colonIndex = header.indexOf(":");
630636
if (colonIndex === -1) {
631-
throw new Error(`Invalid header format: ${header}. Expected Key: Value`);
637+
throw new ValidationError(
638+
`Invalid header format: ${header}. Expected Key: Value`,
639+
"header"
640+
);
632641
}
633642

634643
const key = header.substring(0, colonIndex).trim();
@@ -821,7 +830,7 @@ export async function buildBodyFromInput(
821830
} else {
822831
const file = Bun.file(inputPath);
823832
if (!(await file.exists())) {
824-
throw new Error(`File not found: ${inputPath}`);
833+
throw new ValidationError(`File not found: ${inputPath}`, "input");
825834
}
826835
content = await file.text();
827836
}

test/commands/api.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1039,12 +1039,12 @@ describe("buildBodyFromInput", () => {
10391039
}
10401040
});
10411041

1042-
test("throws for non-existent file", async () => {
1042+
test("throws ValidationError for non-existent file", async () => {
10431043
const mockStdin = createMockStdin("");
10441044

10451045
await expect(
10461046
buildBodyFromInput("/nonexistent/path/file.json", mockStdin)
1047-
).rejects.toThrow(/File not found/);
1047+
).rejects.toBeInstanceOf(ValidationError);
10481048
});
10491049
});
10501050

0 commit comments

Comments
 (0)