Improve GitHub authentication errors - #24502
Conversation
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3694bac61a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if exc.response.status_code in GITHUB_AUTHENTICATION_STATUS_CODES: | ||
| raise GitHubAuthenticationError.from_http_status_error(exc) from exc |
There was a problem hiding this comment.
When a side-effectful async call fails with 401/403, this conversion now raises GitHubAuthenticationError, which is not an httpx.HTTPError. Existing call sites such as CreatePullRequestStep.execute still catch (httpx.HTTPError, ValidationError) after self.pr_url is set so they can report “PR created but labeling failed”; with the real client, a label-permission 403 now bypasses that branch and the user loses the created PR URL/manual-label guidance. Update those call sites to catch the new type or otherwise preserve the partial-success handling.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Updated in bd54df1 after follow-up. GitHubAuthenticationError now bypasses the contextual manual-recovery catches in CreatePullRequestStep and the release branch create/tag workflows, so the centralized CLI handler tells the user to refresh the token. Non-authentication HTTP and validation failures still retain the existing partial-success/manual-recovery guidance. This deliberately treats authentication failures as recoverable configuration errors rather than sending the user directly to a manual workflow.
| if e.response.status_code in GITHUB_AUTHENTICATION_STATUS_CODES: | ||
| raise GitHubAuthenticationError.from_http_status_error(e) from e |
There was a problem hiding this comment.
This now maps every sync-client 403 that is not a primary-limit response (X-RateLimit-Remaining: 0) to a token/permission error. GitHub's REST rate-limit docs (https://docs.github.com/rest/using-the-rest-api/rate-limits-for-the-rest-api#exceeding-the-rate-limit) also describe secondary rate limits as 403/429 responses where retry-after or the error body can be the discriminator, so commands using GitHubManager can tell users to refresh github.token when they should wait/back off instead. Please exclude secondary-limit responses before raising GitHubAuthenticationError.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f5574ff. Secondary limits are now identified from either Retry-After or GitHub’s secondary-limit response message before authentication conversion. The classifier is shared by the sync and async clients; invalid delays fall back to 60 seconds, and synchronous secondary retries are bounded so persistent limits propagate as HTTPStatusError rather than token guidance.
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: e3a1660 | Docs | Datadog PR Page | Give us feedback! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3317a0c6f9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Review from steveny91 is dismissed. Related teams and files:
- agent-integrations
- ddev/src/ddev/cli/init.py
Validation ReportAll 21 validations passed. Show details
|
3 participants
What does this PR do?
Adds an internal CLI exception-handler registry and uses it to render actionable GitHub authentication and permission errors through ddev's normal styled output. GitHub 401 and non-rate-limit 403 responses now recommend refreshing
github.tokenwithout interfering with rate-limit retries.Motivation
Expired or under-permissioned GitHub tokens currently surface low-level HTTP errors, which makes users think ddev itself is broken. This gives them a direct remediation command while preserving existing command context.
Review checklist (to be filled by reviewers)
qa/requiredif this PR needs QA validation, orqa/skip-qaif it does not. Exactly one of the two is required.backport/<branch-name>label to the PR and it will automatically open a backport PR once this one is merged