Skip to content

Commit bfc1739

Browse files
committed
fix(issue-list): propagate ApiError instead of plain Error on fetch failures
When all project fetches fail, re-throw the original ApiError (with status, detail, endpoint) instead of a plain Error. This lets the telemetry layer in PR #251 correctly classify 4xx errors as client errors and suppress them from Sentry exceptions. Also: - Classify more HTTP status codes (404, 429, other 4xx) instead of lumping everything into "unknown" - Include partial failure info in JSON output when some projects fail - Warn on stderr about partial failures in human output - Add tests for error propagation and partial failure handling
1 parent 618671d commit bfc1739

File tree

2 files changed

+427
-32
lines changed

2 files changed

+427
-32
lines changed

src/commands/issue/list.ts

Lines changed: 87 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,36 @@ const VALID_SORT_VALUES: SortValue[] = ["date", "new", "freq", "user"];
5353
const USAGE_HINT = "sentry issue list <org>/<project>";
5454

5555
/** Error type classification for fetch failures */
56-
type FetchErrorType = "permission" | "network" | "unknown";
56+
type FetchErrorType =
57+
| "permission"
58+
| "not_found"
59+
| "rate_limit"
60+
| "bad_request"
61+
| "network"
62+
| "unknown";
63+
64+
/**
65+
* Classify an API error by HTTP status code for user-facing messaging.
66+
*
67+
* @param error - The ApiError to classify
68+
* @returns Error type classification
69+
*/
70+
function classifyApiError(error: ApiError): FetchErrorType {
71+
const { status } = error;
72+
if (status === 401 || status === 403) {
73+
return "permission";
74+
}
75+
if (status === 404) {
76+
return "not_found";
77+
}
78+
if (status === 429) {
79+
return "rate_limit";
80+
}
81+
if (status >= 400 && status < 500) {
82+
return "bad_request";
83+
}
84+
return "unknown";
85+
}
5786

5887
function parseSort(value: string): SortValue {
5988
if (!VALID_SORT_VALUES.includes(value as SortValue)) {
@@ -241,7 +270,7 @@ function getComparator(
241270

242271
type FetchResult =
243272
| { success: true; data: IssueListResult }
244-
| { success: false; errorType: FetchErrorType };
273+
| { success: false; errorType: FetchErrorType; error: Error };
245274

246275
/** Result of resolving targets from parsed argument */
247276
type TargetResolutionResult = {
@@ -364,19 +393,24 @@ async function fetchIssuesForTarget(
364393
if (error instanceof AuthError) {
365394
throw error;
366395
}
367-
// Classify error type for better user messaging
368-
// 401/403 are permission errors
369-
if (
370-
error instanceof ApiError &&
371-
(error.status === 401 || error.status === 403)
372-
) {
373-
return { success: false, errorType: "permission" };
396+
397+
const wrapped = error instanceof Error ? error : new Error(String(error));
398+
399+
// Classify API errors by status code for targeted user messaging
400+
if (error instanceof ApiError) {
401+
return {
402+
success: false,
403+
errorType: classifyApiError(error),
404+
error: wrapped,
405+
};
374406
}
407+
375408
// Network errors (fetch failures, timeouts)
376409
if (error instanceof TypeError && error.message.includes("fetch")) {
377-
return { success: false, errorType: "network" };
410+
return { success: false, errorType: "network", error: wrapped };
378411
}
379-
return { success: false, errorType: "unknown" };
412+
413+
return { success: false, errorType: "unknown", error: wrapped };
380414
}
381415
}
382416

@@ -438,7 +472,7 @@ export const listCommand = buildCommand({
438472
flags: ListFlags,
439473
target?: string
440474
): Promise<void> {
441-
const { stdout, cwd, setContext } = this;
475+
const { stdout, stderr, cwd, setContext } = this;
442476

443477
// Parse positional argument to determine resolution strategy
444478
const parsed = parseOrgProjectArg(target);
@@ -477,34 +511,36 @@ export const listCommand = buildCommand({
477511

478512
// Separate successful fetches from failures
479513
const validResults: IssueListResult[] = [];
480-
const errorTypes = new Set<FetchErrorType>();
514+
const failures: { errorType: FetchErrorType; error: Error }[] = [];
481515

482516
for (const result of results) {
483517
if (result.success) {
484518
validResults.push(result.data);
485519
} else {
486-
errorTypes.add(result.errorType);
520+
failures.push({ errorType: result.errorType, error: result.error });
487521
}
488522
}
489523

490-
if (validResults.length === 0) {
491-
// Build error message based on what types of errors we saw
492-
if (errorTypes.has("permission")) {
493-
throw new Error(
494-
`Failed to fetch issues from ${targets.length} project(s).\n` +
495-
"You don't have permission to access these projects.\n\n" +
496-
"Try running 'sentry auth status' to verify your authentication."
524+
if (validResults.length === 0 && failures.length > 0) {
525+
// Re-throw the first underlying error so telemetry can classify it
526+
// correctly (e.g., ApiError → isClientApiError → suppressed from exceptions).
527+
// Add context about how many projects failed.
528+
// biome-ignore lint/style/noNonNullAssertion: guarded by failures.length > 0
529+
const first = failures[0]!;
530+
const prefix = `Failed to fetch issues from ${targets.length} project(s)`;
531+
532+
// For ApiError, propagate the original so telemetry sees the status code
533+
if (first.error instanceof ApiError) {
534+
throw new ApiError(
535+
`${prefix}: ${first.error.message}`,
536+
first.error.status,
537+
first.error.detail,
538+
first.error.endpoint
497539
);
498540
}
499-
if (errorTypes.has("network")) {
500-
throw new Error(
501-
`Failed to fetch issues from ${targets.length} project(s).\n` +
502-
"Network connection failed. Check your internet connection."
503-
);
504-
}
505-
throw new Error(
506-
`Failed to fetch issues from ${targets.length} project(s).`
507-
);
541+
542+
// For other errors, add context to the message
543+
throw new Error(`${prefix}.\n${first.error.message}`);
508544
}
509545

510546
// Determine display mode
@@ -539,13 +575,32 @@ export const listCommand = buildCommand({
539575
getComparator(flags.sort)(a.issue, b.issue)
540576
);
541577

542-
// JSON output
578+
// JSON output — include partial failure info when some projects failed
543579
if (flags.json) {
544580
const allIssues = issuesWithOptions.map((i) => i.issue);
545-
writeJson(stdout, allIssues);
581+
if (failures.length > 0) {
582+
writeJson(stdout, {
583+
issues: allIssues,
584+
errors: failures.map((f) => ({
585+
type: f.errorType,
586+
message: f.error.message,
587+
})),
588+
});
589+
} else {
590+
writeJson(stdout, allIssues);
591+
}
546592
return;
547593
}
548594

595+
// Warn on stderr about partial failures (human output only)
596+
if (failures.length > 0) {
597+
stderr.write(
598+
muted(
599+
`\nNote: Failed to fetch issues from ${failures.length} project(s). Showing results from ${validResults.length} project(s).\n`
600+
)
601+
);
602+
}
603+
549604
if (issuesWithOptions.length === 0) {
550605
stdout.write("No issues found.\n");
551606
if (footer) {

0 commit comments

Comments
 (0)