Skip to content

Commit 0725a14

Browse files
authored
fix(errors): surface user search-query 400s as ValidationError, keep CLI-built 400s reported (#1154)
## Why Follow-up to #1151 based on review feedback: **silencing every search-query 400 papers over a real bug class.** HTTP 400 means the server rejected our request as malformed. That has two distinct causes, only distinguishable at the command boundary (where `flags.query` is in scope): 1. **The user typed an invalid `--query`** → a user input mistake. 2. **The CLI built an invalid query itself** (no user `--query`) → a genuine CLI defect. PR #1151 special-cased `isSearchQueryParseError()` in all three classifiers (`classifySilenced`, `isUserError`, `isUserApiError`), which silenced **both** — so CLI-authored bad queries became invisible. ## What changed - **New `toSearchQueryError(error, userQuery, extraSuggestions?)`** in `src/lib/errors.ts`: converts a parse-400 into a clean, actionable `ValidationError` **only when the user supplied `--query`**. When there's no user `--query`, the original `ApiError(400)` is returned untouched so it stays reported to Sentry as the CLI bug it is. - Wired in at the three command boundaries that accept `--query`: `issue list` (both the single- and multi-project 400 paths), `explore`, and `trace list`. - **Removed** the `isSearchQueryParseError` special-case from `classifySilenced` (+ dropped the `api_query_error` `SilenceReason`), `isUserError`, and `isUserApiError`. A raw `ApiError(400)` is now always treated as a reported CLI bug. ## Result - User query typos → clean `ValidationError` with the server detail + search-syntax help (correct exit code, no "CLI bug" upgrade nudge). - CLI-constructed bad queries → still reported, so we can actually find and fix them. ## Note A user-query `ValidationError` is still surfaced to the user and (like other `ValidationError`s) reaches Sentry — that volume is now a useful signal for client-side query-linting UX rather than noise mislabeled as a 400 crash. Happy to silence query `ValidationError`s separately if preferred. ## Tests - `toSearchQueryError` unit tests (convert vs passthrough, suggestions, non-ApiError). - `issue list` integration tests: user `--query` → `ValidationError`; no `--query` → reported `ApiError(400)`. - Updated `classifySilenced` / `isUserError` / `isUserApiError` tests to assert search-query 400s are no longer silenced. - Full unit suite for touched files green; lint + typecheck clean.
1 parent 03e657b commit 0725a14

20 files changed

Lines changed: 568 additions & 123 deletions

File tree

.lore.md

Lines changed: 80 additions & 71 deletions
Large diffs are not rendered by default.

src/commands/event/list.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import {
1515
hasPreviousPage,
1616
resolveCursor,
1717
} from "../../lib/db/pagination.js";
18-
import { ContextError } from "../../lib/errors.js";
18+
import { ContextError, toSearchQueryError } from "../../lib/errors.js";
1919
import { CommandOutput } from "../../lib/formatters/output.js";
2020
import { buildListCommand, paginationHint } from "../../lib/list-command.js";
2121
import { withProgress } from "../../lib/polling.js";
@@ -155,6 +155,9 @@ export const listCommand = buildListCommand("event", {
155155
full: flags.full,
156156
cursor,
157157
...timeRangeToApiParams(timeRange),
158+
}).catch((error: unknown): never => {
159+
// An unparseable user --query is a user input mistake, not a CLI bug.
160+
throw toSearchQueryError(error, flags.query);
158161
})
159162
);
160163

src/commands/explore.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import {
2020
hasPreviousPage,
2121
resolveCursor,
2222
} from "../lib/db/pagination.js";
23-
import { ValidationError } from "../lib/errors.js";
23+
import { toSearchQueryError, ValidationError } from "../lib/errors.js";
2424
import { filterFields } from "../lib/formatters/json.js";
2525
import { buildMetaColumns } from "../lib/formatters/meta-table.js";
2626
import { CommandOutput } from "../lib/formatters/output.js";
@@ -792,7 +792,13 @@ export const exploreCommand = buildListCommand("explore", {
792792
message: `Querying ${dataset} in ${project ? `${org}/${project}` : org}...`,
793793
json: flags.json,
794794
},
795-
() => config.fetch({ cursor, limit: flags.limit, timeRange })
795+
() =>
796+
config
797+
.fetch({ cursor, limit: flags.limit, timeRange })
798+
.catch((error: unknown): never => {
799+
// An unparseable user --query is a user input mistake, not a CLI bug.
800+
throw toSearchQueryError(error, flags.query);
801+
})
796802
);
797803

798804
advancePaginationState(PAGINATION_KEY, contextKey, direction, nextCursor);

src/commands/issue/events.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import {
1212
hasPreviousPage,
1313
resolveCursor,
1414
} from "../../lib/db/pagination.js";
15-
import { ContextError } from "../../lib/errors.js";
15+
import { ContextError, toSearchQueryError } from "../../lib/errors.js";
1616
import { CommandOutput } from "../../lib/formatters/output.js";
1717
import { buildListCommand, paginationHint } from "../../lib/list-command.js";
1818
import { withProgress } from "../../lib/polling.js";
@@ -137,6 +137,9 @@ export const eventsCommand = buildListCommand("issue", {
137137
full: flags.full,
138138
cursor,
139139
...timeRangeToApiParams(timeRange),
140+
}).catch((error: unknown): never => {
141+
// An unparseable user --query is a user input mistake, not a CLI bug.
142+
throw toSearchQueryError(error, flags.query);
140143
})
141144
);
142145

src/commands/issue/list.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import { createDsnFingerprint } from "../../lib/dsn/index.js";
4040
import {
4141
ApiError,
4242
ContextError,
43+
toSearchQueryError,
4344
ValidationError,
4445
withAuthGuard,
4546
} from "../../lib/errors.js";
@@ -906,6 +907,13 @@ function enrichIssueListError(
906907
error: unknown,
907908
flags: Pick<ListFlags, "query" | "period" | "sort">
908909
): never {
910+
// A user-supplied --query the server cannot parse is a user input mistake,
911+
// not a CLI bug: surface it as a ValidationError. A 400 with no user --query
912+
// (the CLI built a bad query) falls through and stays a reported ApiError.
913+
const queryError = toSearchQueryError(error, flags.query);
914+
if (queryError !== error) {
915+
throw queryError;
916+
}
909917
if (error instanceof ApiError) {
910918
if (error.status === 400) {
911919
throw new ApiError(
@@ -1124,6 +1132,14 @@ async function handleResolvedTargets(
11241132
const { error: first } = failures[0]!;
11251133
const prefix = `Failed to fetch issues from ${targets.length} project(s)`;
11261134

1135+
// A user-supplied --query the server cannot parse is a user input mistake,
1136+
// not a CLI bug — surface it as a ValidationError before wrapping as a
1137+
// multi-project ApiError. CLI-authored 400s fall through and stay reported.
1138+
const queryError = toSearchQueryError(first, flags.query);
1139+
if (queryError !== first) {
1140+
throw queryError;
1141+
}
1142+
11271143
// Propagate ApiError so telemetry sees the original status code.
11281144
// For 400 errors, append actionable suggestions since the user's query
11291145
// or parameters are likely malformed. Common causes: invalid Sentry

src/commands/log/list.ts

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import {
2222
import {
2323
AuthError,
2424
stringifyUnknown,
25+
toSearchQueryError,
2526
ValidationError,
2627
} from "../../lib/errors.js";
2728
import {
@@ -180,6 +181,9 @@ async function executeSingleFetch(
180181
...timeRangeToApiParams(timeRange),
181182
sort: flags.sort,
182183
extraFields: flags.fields,
184+
}).catch((error: unknown): never => {
185+
// An unparseable user --query is a user input mistake, not a CLI bug.
186+
throw toSearchQueryError(error, flags.query);
183187
});
184188

185189
const periodLabel =
@@ -317,7 +321,8 @@ function renderLogRows(
317321
* Execute a single poll iteration in follow mode.
318322
*
319323
* Returns the new logs, or `undefined` if a transient error occurred
320-
* (reported via `onDiagnostic`). Re-throws {@link AuthError}.
324+
* (reported via `onDiagnostic`). Re-throws {@link AuthError} and
325+
* {@link ValidationError} (a converted bad-`--query` 400).
321326
*/
322327
async function fetchPoll<T extends LogLike>(
323328
config: FollowGeneratorConfig<T>,
@@ -330,6 +335,12 @@ async function fetchPoll<T extends LogLike>(
330335
if (error instanceof AuthError) {
331336
throw error;
332337
}
338+
// A ValidationError here is a converted search-query 400 (bad user
339+
// --query). It is a user input mistake, not a transient fetch failure, so
340+
// surface it and stop the stream rather than capturing it as a CLI bug.
341+
if (error instanceof ValidationError) {
342+
throw error;
343+
}
333344
Sentry.captureException(error);
334345
const message = stringifyUnknown(error);
335346
config.onDiagnostic(`Error fetching logs: ${message}\n`);
@@ -485,6 +496,9 @@ async function executeTraceSingleFetch(
485496
limit: flags.limit,
486497
...timeRangeToApiParams(timeRange),
487498
sort: flags.sort,
499+
}).catch((error: unknown): never => {
500+
// An unparseable user --query is a user input mistake, not a CLI bug.
501+
throw toSearchQueryError(error, flags.query);
488502
});
489503

490504
const periodLabel =
@@ -825,6 +839,10 @@ export const listCommand = buildListCommand(
825839
query: traceQuery,
826840
limit: flags.limit,
827841
statsPeriod,
842+
}).catch((error: unknown): never => {
843+
// An unparseable user --query is a user input mistake, not a
844+
// CLI bug — surface it as an actionable ValidationError.
845+
throw toSearchQueryError(error, flags.query);
828846
}),
829847
extractNew: (logs, lastTs) =>
830848
logs.filter((l) => {
@@ -896,6 +914,10 @@ export const listCommand = buildListCommand(
896914
statsPeriod,
897915
afterTimestamp,
898916
extraFields: flags.fields,
917+
}).catch((error: unknown): never => {
918+
// An unparseable user --query is a user input mistake, not a
919+
// CLI bug — surface it as an actionable ValidationError.
920+
throw toSearchQueryError(error, flags.query);
899921
}),
900922
extractNew: (logs) => logs,
901923
});

src/commands/replay/list.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
hasPreviousPage,
1818
resolveCursor,
1919
} from "../../lib/db/pagination.js";
20+
import { toSearchQueryError } from "../../lib/errors.js";
2021
import {
2122
escapeMarkdownCell,
2223
formatRelativeTime,
@@ -364,6 +365,9 @@ export const listCommand = buildListCommand("replay", {
364365
sort: flags.sort,
365366
cursor,
366367
...timeRangeToApiParams(timeRange),
368+
}).catch((error: unknown): never => {
369+
// An unparseable user --query is a user input mistake, not a CLI bug.
370+
throw toSearchQueryError(error, flags.query);
367371
})
368372
);
369373

src/commands/span/list.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
hasPreviousPage,
2121
resolveCursor,
2222
} from "../../lib/db/pagination.js";
23+
import { toSearchQueryError } from "../../lib/errors.js";
2324
import {
2425
type FlatSpan,
2526
formatSpanTable,
@@ -390,6 +391,9 @@ async function handleTraceMode(
390391
...timeRangeToApiParams(timeRange),
391392
extraFields: extraApiFields,
392393
allProjects: true,
394+
}).catch((error: unknown): never => {
395+
// An unparseable user --query is a user input mistake, not a CLI bug.
396+
throw toSearchQueryError(error, flags.query);
393397
})
394398
);
395399

@@ -471,6 +475,9 @@ async function handleProjectMode(
471475
cursor,
472476
...timeRangeToApiParams(timeRange),
473477
extraFields: extraApiFields,
478+
}).catch((error: unknown): never => {
479+
// An unparseable user --query is a user input mistake, not a CLI bug.
480+
throw toSearchQueryError(error, flags.query);
474481
})
475482
);
476483

src/commands/trace/list.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
hasPreviousPage,
1414
resolveCursor,
1515
} from "../../lib/db/pagination.js";
16+
import { toSearchQueryError } from "../../lib/errors.js";
1617
import { formatTraceTable } from "../../lib/formatters/index.js";
1718
import { filterFields } from "../../lib/formatters/json.js";
1819
import { CommandOutput } from "../../lib/formatters/output.js";
@@ -289,6 +290,9 @@ export const listCommand = buildListCommand("trace", {
289290
sort: flags.sort,
290291
cursor,
291292
...timeRangeToApiParams(timeRange),
293+
}).catch((error: unknown): never => {
294+
// An unparseable user --query is a user input mistake, not a CLI bug.
295+
throw toSearchQueryError(error, flags.query);
292296
})
293297
);
294298

src/commands/trace/logs.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
} from "../../lib/arg-parsing.js";
1414
import { openInBrowser } from "../../lib/browser.js";
1515
import { buildCommand } from "../../lib/command.js";
16+
import { toSearchQueryError } from "../../lib/errors.js";
1617
import { filterFields } from "../../lib/formatters/json.js";
1718
import { formatLogTable } from "../../lib/formatters/log.js";
1819
import { CommandOutput, formatFooter } from "../../lib/formatters/output.js";
@@ -207,6 +208,9 @@ export const logsCommand = buildCommand({
207208
limit: flags.limit,
208209
query,
209210
sort: flags.sort,
211+
}).catch((error: unknown): never => {
212+
// An unparseable user --query is a user input mistake, not a CLI bug.
213+
throw toSearchQueryError(error, flags.query);
210214
})
211215
);
212216

0 commit comments

Comments
 (0)