Skip to content

Commit e4e79b9

Browse files
committed
fix: add 403 scope guidance to issue list error handling (CLI-97)
When issue list gets 403 Forbidden, the error now includes suggestions about token scopes, re-authentication, and project membership — matching the 403 guidance added to org listing in PR #498. Applied to both code paths (handleResolvedTargets and org-all mode). Extracted enrichIssueListError() helper to DRY the 400/403 enrichment used by both paths. Affects 41 users (CLI-97).
1 parent eb0c003 commit e4e79b9

File tree

2 files changed

+71
-16
lines changed

2 files changed

+71
-16
lines changed

AGENTS.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -830,6 +830,9 @@ mock.module("./some-module", () => ({
830830
<!-- lore:019cb8c2-d7b5-780c-8a9f-d20001bc198f -->
831831
* **Install script: BSD sed and awk JSON parsing breaks OCI digest extraction**: The install script parses OCI manifests with awk (no jq). Key trap: BSD sed \`\n\` is literal, not newline. Fix: single awk pass tracking last-seen \`"digest"\`, printing when \`"org.opencontainers.image.title"\` matches target. The config digest (\`sha256:44136fa...\`) is a 2-byte \`{}\` blob — downloading it instead of the real binary causes \`gunzip: unexpected end of file\`.
832832
833+
<!-- lore:019d0b04-ccec-7bd2-a5ca-732e7064cc1a -->
834+
* **Multi-region fan-out: distinguish all-403 from empty orgs with hasSuccessfulRegion flag**: The multi-region org listing fan-out (\`listOrganizationsUncached\` in \`src/lib/api/organizations.ts\`) uses \`Promise.allSettled\` to collect results from all regions. When checking whether to propagate a 403 error, don't use \`flatResults.length === 0\` alone — a region returning 200 OK with zero orgs pushes nothing into \`flatResults\`, making it look like all regions failed. Track a separate \`hasSuccessfulRegion\` boolean set on any \`"fulfilled"\` settlement. Only re-throw the 403 \`ApiError\` when \`!hasSuccessfulRegion && lastScopeError\` — this correctly distinguishes "all regions returned 403" from "some regions had no orgs."
835+
833836
<!-- lore:019c969a-1c90-7041-88a8-4e4d9a51ebed -->
834837
* **Multiple mockFetch calls replace each other — use unified mocks for multi-endpoint tests**: Bun test mocking gotchas: (1) \`mockFetch()\` replaces \`globalThis.fetch\` — calling it twice replaces the first mock. Use a single unified fetch mock dispatching by URL pattern. (2) \`mock.module()\` pollutes the module registry for ALL subsequent test files. Tests using it must live in \`test/isolated/\` and run via \`test:isolated\`. This also causes \`delta-upgrade.test.ts\` to fail when run alongside \`test/isolated/delta-upgrade.test.ts\` — the isolated test's \`mock.module()\` replaces \`CLI\_VERSION\` for all subsequent files. (3) For \`Bun.spawn\`, use direct property assignment in \`beforeEach\`/\`afterEach\`.
835838
@@ -838,6 +841,9 @@ mock.module("./some-module", () => ({
838841
839842
### Pattern
840843
844+
<!-- lore:019d0b04-ccf7-7e74-a049-2ca6b8faa85f -->
845+
* **Cursor Bugbot review comments: fix valid bugs before merging**: Cursor Bugbot sometimes catches real logic bugs in PRs (not just style). In CLI-89, Bugbot identified that \`flatResults.length === 0\` was an incorrect proxy for "all regions failed" since fulfilled-but-empty regions are indistinguishable from rejected ones. Always read Bugbot's full comment body (via \`gh api repos/OWNER/REPO/pulls/NUM/comments\`) and fix valid findings before merging. Bugbot comments include a \`BUGBOT\_BUG\_ID\` HTML comment for tracking.
846+
841847
<!-- lore:019c972c-9f11-7c0d-96ce-3f8cc2641175 -->
842848
* **Org-scoped SDK calls follow getOrgSdkConfig + unwrapResult pattern**: All org-scoped API calls in src/lib/api-client.ts: (1) call \`getOrgSdkConfig(orgSlug)\` for regional URL + SDK config, (2) spread into SDK function: \`{ ...config, path: { organization\_id\_or\_slug: orgSlug, ... } }\`, (3) pass to \`unwrapResult(result, errorContext)\`. Shared helpers \`resolveAllTargets\`/\`resolveOrgAndProject\` must NOT call \`fetchProjectId\` — commands that need it enrich targets themselves.
843849

src/commands/issue/list.ts

Lines changed: 65 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -841,17 +841,7 @@ async function handleOrgAllIssues(
841841
)
842842
);
843843
} catch (error) {
844-
// Enrich 400 errors with the same suggestions as handleResolvedTargets
845-
// (CLI-BY, 23 users). The org-all path was missing these hints.
846-
if (error instanceof ApiError && error.status === 400) {
847-
throw new ApiError(
848-
error.message,
849-
error.status,
850-
build400Detail(error.detail, flags),
851-
error.endpoint
852-
);
853-
}
854-
throw error;
844+
throw enrichIssueListError(error, flags);
855845
}
856846
const { issues, nextCursor } = issuesResult;
857847

@@ -966,6 +956,63 @@ function build400Detail(
966956
return lines.join("\n ");
967957
}
968958

959+
/**
960+
* Enrich an API error from issue listing with actionable suggestions.
961+
*
962+
* Handles both 400 (query/parameter) and 403 (permission) errors.
963+
* Re-throws non-ApiError and unhandled statuses unchanged.
964+
*/
965+
function enrichIssueListError(
966+
error: unknown,
967+
flags: Pick<ListFlags, "query" | "period">
968+
): never {
969+
if (error instanceof ApiError) {
970+
if (error.status === 400) {
971+
throw new ApiError(
972+
error.message,
973+
error.status,
974+
build400Detail(error.detail, flags),
975+
error.endpoint
976+
);
977+
}
978+
if (error.status === 403) {
979+
throw new ApiError(
980+
error.message,
981+
error.status,
982+
build403Detail(error.detail),
983+
error.endpoint
984+
);
985+
}
986+
}
987+
throw error;
988+
}
989+
990+
/**
991+
* Build an enriched error detail for 403 Forbidden responses.
992+
*
993+
* Suggests checking token scopes and project membership — the most common
994+
* causes of 403 on the issues endpoint (CLI-97, 41 users).
995+
*
996+
* @param originalDetail - The API response detail (may be undefined)
997+
* @returns Enhanced detail string with suggestions
998+
*/
999+
function build403Detail(originalDetail: string | undefined): string {
1000+
const lines: string[] = [];
1001+
1002+
if (originalDetail) {
1003+
lines.push(originalDetail, "");
1004+
}
1005+
1006+
lines.push(
1007+
"Suggestions:",
1008+
" • Your auth token may lack the required scopes (org:read, project:read)",
1009+
" • Re-authenticate with: sentry auth login",
1010+
" • Verify project membership: sentry project list <org>/"
1011+
);
1012+
1013+
return lines.join("\n ");
1014+
}
1015+
9691016
/**
9701017
* Handle auto-detect, explicit, and project-search modes.
9711018
*
@@ -1118,10 +1165,12 @@ async function handleResolvedTargets(
11181165
// or parameters are likely malformed. Common causes: invalid Sentry
11191166
// search syntax, unsupported period for the org's data retention.
11201167
if (first instanceof ApiError) {
1121-
const detail =
1122-
first.status === 400
1123-
? build400Detail(first.detail, flags)
1124-
: first.detail;
1168+
let detail = first.detail;
1169+
if (first.status === 400) {
1170+
detail = build400Detail(first.detail, flags);
1171+
} else if (first.status === 403) {
1172+
detail = build403Detail(first.detail);
1173+
}
11251174
throw new ApiError(
11261175
`${prefix}: ${first.message}`,
11271176
first.status,
@@ -1130,7 +1179,7 @@ async function handleResolvedTargets(
11301179
);
11311180
}
11321181

1133-
throw new Error(`${prefix}.\n${first.message}`);
1182+
throw new Error(`${prefix}: ${first.message}`);
11341183
}
11351184

11361185
const isMultiProject = validResults.length > 1;

0 commit comments

Comments
 (0)