Ignore HTTP 403 when fetching workflow runs#22670
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Agent-Logs-Url: https://github.com/github/gh-aw/sessions/3d8df93a-6beb-4e84-a19e-7c6f10c1404e
There was a problem hiding this comment.
Pull request overview
Adjusts workflow-run fetching to tolerate GitHub CLI failures caused by HTTP 403 responses (e.g., rate limiting) so health checks don’t fail hard when the API is temporarily inaccessible.
Changes:
- Treat
gh run listfailures containingHTTP 403as “no results” and emit a warning to stderr instead of returning an error. - Extend error-classification tests to include
HTTP 403cases and ensure classifications are mutually exclusive.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/cli/logs_github_api.go | Adds logic to detect HTTP 403 in combined output and return empty results without error. |
| pkg/cli/logs_github_api_test.go | Adds want403 and new cases asserting 403 detection is distinct from auth/invalid-field paths. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Ignore 403 errors (e.g. rate limit exceeded) – treat as no results. | ||
| if strings.Contains(combinedMsgLower, "http 403") { | ||
| fmt.Fprintln(os.Stderr, console.FormatWarningMessage("GitHub API returned 403, skipping: "+strings.TrimSpace(outputMsg))) |
There was a problem hiding this comment.
Ignoring any HTTP 403 here changes the semantics of listWorkflowRunsWithPagination from “couldn’t fetch runs” to “no runs exist”, which can silently mask real permission/configuration problems (403 Forbidden / resource not accessible) in other call sites (e.g. pkg/cli/logs_orchestrator.go and logs_download flows). Consider scoping this behavior to rate-limit cases only (match on "rate limit" / "secondary rate limit") or making it opt-in via a ListWorkflowRunsOptions flag so only the health check suppresses 403s, while other commands still surface an actionable error.
| // Ignore 403 errors (e.g. rate limit exceeded) – treat as no results. | |
| if strings.Contains(combinedMsgLower, "http 403") { | |
| fmt.Fprintln(os.Stderr, console.FormatWarningMessage("GitHub API returned 403, skipping: "+strings.TrimSpace(outputMsg))) | |
| // Ignore 403 errors only when they clearly indicate a rate limit condition | |
| // (e.g. "rate limit" / "secondary rate limit"). Other 403s (e.g. missing | |
| // permissions, misconfigured token) should surface as errors instead of | |
| // being treated as "no results". | |
| if strings.Contains(combinedMsgLower, "http 403") && | |
| (strings.Contains(combinedMsgLower, "rate limit") || | |
| strings.Contains(combinedMsgLower, "secondary rate limit") || | |
| strings.Contains(combinedMsgLower, "abuse detection")) { | |
| fmt.Fprintln(os.Stderr, console.FormatWarningMessage("GitHub API returned 403 (rate limited), skipping: "+strings.TrimSpace(outputMsg))) |
The
health-smoke-copilotCI job was failing when the GitHub API returned a 403 (rate limit exceeded), causinggh run listto exit non-zero and the health command to propagate the error.Changes
pkg/cli/logs_github_api.go: InlistWorkflowRunsWithPagination, detectHTTP 403in the combined error output and return empty results (nil, 0, nil) with a stderr warning instead of propagating an error. The health command then outputs a valid zero-run JSON structure and exits cleanly.pkg/cli/logs_github_api_test.go: ExtendedTestListWorkflowRunsErrorHandlingwith awant403field and two 403 test cases to verify the classification is mutually exclusive with auth and invalid-field errors.💬 Send tasks to Copilot coding agent from Slack and Teams to turn conversations into code. Copilot posts an update in your thread when it's finished.