diff --git a/src/commands/issue/list.ts b/src/commands/issue/list.ts index e1bba9b1..01691742 100644 --- a/src/commands/issue/list.ts +++ b/src/commands/issue/list.ts @@ -52,9 +52,6 @@ const VALID_SORT_VALUES: SortValue[] = ["date", "new", "freq", "user"]; /** Usage hint for ContextError messages */ const USAGE_HINT = "sentry issue list /"; -/** Error type classification for fetch failures */ -type FetchErrorType = "permission" | "network" | "unknown"; - function parseSort(value: string): SortValue { if (!VALID_SORT_VALUES.includes(value as SortValue)) { throw new Error( @@ -241,7 +238,7 @@ function getComparator( type FetchResult = | { success: true; data: IssueListResult } - | { success: false; errorType: FetchErrorType }; + | { success: false; error: Error }; /** Result of resolving targets from parsed argument */ type TargetResolutionResult = { @@ -349,7 +346,7 @@ async function resolveTargetsFromParsedArg( * * @param target - Resolved org/project target * @param options - Query options (query, limit, sort) - * @returns Success with issues, or failure with error type classification + * @returns Success with issues, or failure with the original error preserved * @throws {AuthError} When user is not authenticated */ async function fetchIssuesForTarget( @@ -364,19 +361,11 @@ async function fetchIssuesForTarget( if (error instanceof AuthError) { throw error; } - // Classify error type for better user messaging - // 401/403 are permission errors - if ( - error instanceof ApiError && - (error.status === 401 || error.status === 403) - ) { - return { success: false, errorType: "permission" }; - } - // Network errors (fetch failures, timeouts) - if (error instanceof TypeError && error.message.includes("fetch")) { - return { success: false, errorType: "network" }; - } - return { success: false, errorType: "unknown" }; + + return { + success: false, + error: error instanceof Error ? error : new Error(String(error)), + }; } } @@ -438,7 +427,7 @@ export const listCommand = buildCommand({ flags: ListFlags, target?: string ): Promise { - const { stdout, cwd, setContext } = this; + const { stdout, stderr, cwd, setContext } = this; // Parse positional argument to determine resolution strategy const parsed = parseOrgProjectArg(target); @@ -477,34 +466,36 @@ export const listCommand = buildCommand({ // Separate successful fetches from failures const validResults: IssueListResult[] = []; - const errorTypes = new Set(); + const failures: Error[] = []; for (const result of results) { if (result.success) { validResults.push(result.data); } else { - errorTypes.add(result.errorType); + failures.push(result.error); } } - if (validResults.length === 0) { - // Build error message based on what types of errors we saw - if (errorTypes.has("permission")) { - throw new Error( - `Failed to fetch issues from ${targets.length} project(s).\n` + - "You don't have permission to access these projects.\n\n" + - "Try running 'sentry auth status' to verify your authentication." + if (validResults.length === 0 && failures.length > 0) { + // Re-throw the first underlying error so telemetry can classify it + // correctly (e.g., ApiError → isClientApiError → suppressed from exceptions). + // Add context about how many projects failed. + // biome-ignore lint/style/noNonNullAssertion: guarded by failures.length > 0 + const first = failures[0]!; + const prefix = `Failed to fetch issues from ${targets.length} project(s)`; + + // For ApiError, propagate the original so telemetry sees the status code + if (first instanceof ApiError) { + throw new ApiError( + `${prefix}: ${first.message}`, + first.status, + first.detail, + first.endpoint ); } - if (errorTypes.has("network")) { - throw new Error( - `Failed to fetch issues from ${targets.length} project(s).\n` + - "Network connection failed. Check your internet connection." - ); - } - throw new Error( - `Failed to fetch issues from ${targets.length} project(s).` - ); + + // For other errors, add context to the message + throw new Error(`${prefix}.\n${first.message}`); } // Determine display mode @@ -539,13 +530,33 @@ export const listCommand = buildCommand({ getComparator(flags.sort)(a.issue, b.issue) ); - // JSON output + // JSON output — include partial failure info when some projects failed if (flags.json) { const allIssues = issuesWithOptions.map((i) => i.issue); - writeJson(stdout, allIssues); + if (failures.length > 0) { + writeJson(stdout, { + issues: allIssues, + errors: failures.map((e) => + e instanceof ApiError + ? { status: e.status, message: e.message } + : { message: e.message } + ), + }); + } else { + writeJson(stdout, allIssues); + } return; } + // Warn on stderr about partial failures (human output only) + if (failures.length > 0) { + stderr.write( + muted( + `\nNote: Failed to fetch issues from ${failures.length} project(s). Showing results from ${validResults.length} project(s).\n` + ) + ); + } + if (issuesWithOptions.length === 0) { stdout.write("No issues found.\n"); if (footer) { diff --git a/test/commands/issue/list.test.ts b/test/commands/issue/list.test.ts new file mode 100644 index 00000000..30a71606 --- /dev/null +++ b/test/commands/issue/list.test.ts @@ -0,0 +1,340 @@ +/** + * Issue List Command Tests + * + * Tests for error propagation and partial failure handling + * in src/commands/issue/list.ts + */ + +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { listCommand } from "../../../src/commands/issue/list.js"; +import { DEFAULT_SENTRY_URL } from "../../../src/lib/constants.js"; +import { setAuthToken } from "../../../src/lib/db/auth.js"; +import { setDefaults } from "../../../src/lib/db/defaults.js"; +import { setOrgRegion } from "../../../src/lib/db/regions.js"; +import { ApiError } from "../../../src/lib/errors.js"; +import { mockFetch, useTestConfigDir } from "../../helpers.js"; + +type ListFlags = { + readonly query?: string; + readonly limit: number; + readonly sort: "date" | "new" | "freq" | "user"; + readonly json: boolean; +}; + +/** Command function type extracted from loader result */ +type ListFunc = ( + this: unknown, + flags: ListFlags, + target?: string +) => Promise; + +const getConfigDir = useTestConfigDir("test-issue-list-", { + isolateProjectRoot: true, +}); + +let originalFetch: typeof globalThis.fetch; +let func: ListFunc; + +beforeEach(async () => { + originalFetch = globalThis.fetch; + func = (await listCommand.loader()) as unknown as ListFunc; + await setAuthToken("test-token"); + await setOrgRegion("test-org", DEFAULT_SENTRY_URL); + await setDefaults("test-org", "test-project"); +}); + +afterEach(() => { + globalThis.fetch = originalFetch; +}); + +/** Create a minimal context mock for testing */ +function createContext() { + const stdout = { + output: "", + write(s: string) { + stdout.output += s; + }, + }; + const stderr = { + output: "", + write(s: string) { + stderr.output += s; + }, + }; + + const context = { + process, + stdout, + stderr, + cwd: getConfigDir(), + setContext: () => { + // Intentionally empty — telemetry context not needed in tests + }, + }; + + return { context, stdout, stderr }; +} + +/** Build a mock issue response */ +function mockIssue(overrides?: Record) { + return { + id: "123", + shortId: "TEST-PROJECT-1", + title: "Test Error", + status: "unresolved", + platform: "javascript", + type: "error", + count: "10", + userCount: 5, + lastSeen: "2025-01-01T00:00:00Z", + firstSeen: "2025-01-01T00:00:00Z", + level: "error", + ...overrides, + }; +} + +describe("issue list: error propagation", () => { + test("throws ApiError (not plain Error) when all fetches fail with 400", async () => { + // Uses default org/project from setDefaults("test-org", "test-project") + // listIssues hits: /api/0/organizations/test-org/issues/?query=project:test-project + globalThis.fetch = mockFetch(async (input, init) => { + const req = new Request(input, init); + if (req.url.includes("/issues/")) { + return new Response( + JSON.stringify({ detail: "Invalid query: unknown field" }), + { status: 400 } + ); + } + return new Response(JSON.stringify({ detail: "Not found" }), { + status: 404, + }); + }); + + const { context } = createContext(); + + try { + await func.call(context, { limit: 10, sort: "date", json: false }); + expect.unreachable("Should have thrown"); + } catch (error) { + expect(error).toBeInstanceOf(ApiError); + expect((error as ApiError).status).toBe(400); + expect((error as Error).message).toContain("Failed to fetch issues"); + } + }); + + test("throws ApiError with 404 status when project not found", async () => { + globalThis.fetch = mockFetch(async (input, init) => { + const req = new Request(input, init); + if (req.url.includes("/issues/")) { + return new Response(JSON.stringify({ detail: "Project not found" }), { + status: 404, + }); + } + return new Response(JSON.stringify([]), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + }); + + const { context } = createContext(); + + try { + await func.call(context, { limit: 10, sort: "date", json: false }); + expect.unreachable("Should have thrown"); + } catch (error) { + expect(error).toBeInstanceOf(ApiError); + expect((error as ApiError).status).toBe(404); + } + }); + + test("throws ApiError with 429 status on rate limiting", async () => { + globalThis.fetch = mockFetch(async (input, init) => { + const req = new Request(input, init); + if (req.url.includes("/issues/")) { + return new Response(JSON.stringify({ detail: "Too many requests" }), { + status: 429, + }); + } + return new Response(JSON.stringify([]), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + }); + + const { context } = createContext(); + + try { + await func.call(context, { limit: 10, sort: "date", json: false }); + expect.unreachable("Should have thrown"); + } catch (error) { + expect(error).toBeInstanceOf(ApiError); + expect((error as ApiError).status).toBe(429); + } + }); + + test("preserves ApiError detail from original error", async () => { + globalThis.fetch = mockFetch(async (input, init) => { + const req = new Request(input, init); + if (req.url.includes("/issues/")) { + return new Response( + JSON.stringify({ detail: "Invalid search query: bad syntax" }), + { status: 400 } + ); + } + return new Response(JSON.stringify([]), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + }); + + const { context } = createContext(); + + try { + await func.call(context, { limit: 10, sort: "date", json: false }); + expect.unreachable("Should have thrown"); + } catch (error) { + expect(error).toBeInstanceOf(ApiError); + const apiErr = error as ApiError; + expect(apiErr.detail).toBeDefined(); + } + }); +}); + +describe("issue list: partial failure handling", () => { + test("JSON output includes error info on partial failures", async () => { + await setOrgRegion("multi-org", DEFAULT_SENTRY_URL); + + globalThis.fetch = mockFetch(async (input, init) => { + const req = new Request(input, init); + const url = req.url; + + // listProjects: /api/0/organizations/multi-org/projects/ + if (url.includes("/organizations/multi-org/projects/")) { + return new Response( + JSON.stringify([ + { id: "1", slug: "proj-a", name: "Project A" }, + { id: "2", slug: "proj-b", name: "Project B" }, + ]), + { + status: 200, + headers: { "Content-Type": "application/json", Link: "" }, + } + ); + } + + // listIssues: /api/0/organizations/multi-org/issues/?query=project:proj-a... + if (url.includes("/organizations/multi-org/issues/")) { + const queryParam = new URL(url).searchParams.get("query") ?? ""; + if (queryParam.includes("project:proj-a")) { + return new Response(JSON.stringify([mockIssue({ id: "1" })]), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + } + if (queryParam.includes("project:proj-b")) { + return new Response( + JSON.stringify({ detail: "Invalid query syntax" }), + { status: 400 } + ); + } + } + + return new Response(JSON.stringify([]), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + }); + + const { context, stdout } = createContext(); + + await func.call( + context, + { limit: 10, sort: "date", json: true }, + "multi-org/" + ); + + const output = JSON.parse(stdout.output); + expect(output).toHaveProperty("issues"); + expect(output).toHaveProperty("errors"); + expect(output.issues.length).toBe(1); + expect(output.errors.length).toBe(1); + expect(output.errors[0].status).toBe(400); + }); + + test("stderr warning on partial failures in human output", async () => { + await setOrgRegion("multi-org", DEFAULT_SENTRY_URL); + + globalThis.fetch = mockFetch(async (input, init) => { + const req = new Request(input, init); + const url = req.url; + + if (url.includes("/organizations/multi-org/projects/")) { + return new Response( + JSON.stringify([ + { id: "1", slug: "proj-a", name: "Project A" }, + { id: "2", slug: "proj-b", name: "Project B" }, + ]), + { + status: 200, + headers: { "Content-Type": "application/json", Link: "" }, + } + ); + } + + if (url.includes("/organizations/multi-org/issues/")) { + const queryParam = new URL(url).searchParams.get("query") ?? ""; + if (queryParam.includes("project:proj-a")) { + return new Response(JSON.stringify([mockIssue({ id: "1" })]), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + } + if (queryParam.includes("project:proj-b")) { + return new Response(JSON.stringify({ detail: "Permission denied" }), { + status: 403, + }); + } + } + + return new Response(JSON.stringify([]), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + }); + + const { context, stderr } = createContext(); + + await func.call( + context, + { limit: 10, sort: "date", json: false }, + "multi-org/" + ); + + expect(stderr.output).toContain("Failed to fetch issues from 1 project(s)"); + expect(stderr.output).toContain("Showing results from 1 project(s)"); + }); + + test("JSON output is plain array when no failures", async () => { + globalThis.fetch = mockFetch(async (input, init) => { + const req = new Request(input, init); + if (req.url.includes("/issues/")) { + return new Response(JSON.stringify([mockIssue()]), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + } + return new Response(JSON.stringify([]), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + }); + + const { context, stdout } = createContext(); + + await func.call(context, { limit: 10, sort: "date", json: true }); + + const output = JSON.parse(stdout.output); + // Should be a plain array, not an object with issues/errors keys + expect(Array.isArray(output)).toBe(true); + }); +});