diff --git a/ddev/changelog.d/24963.added b/ddev/changelog.d/24963.added
new file mode 100644
index 0000000000000..6a5ed6b2a7567
--- /dev/null
+++ b/ddev/changelog.d/24963.added
@@ -0,0 +1 @@
+Add a retry strategy to the async GitHub client for failures that are not rate limiting.
diff --git a/ddev/src/ddev/cli/ci/tests/dispatcher_config.py b/ddev/src/ddev/cli/ci/tests/dispatcher_config.py
index 51842ee01b024..8dc97b507e2e9 100644
--- a/ddev/src/ddev/cli/ci/tests/dispatcher_config.py
+++ b/ddev/src/ddev/cli/ci/tests/dispatcher_config.py
@@ -10,6 +10,7 @@
from pydantic import BaseModel, ConfigDict, Field
from ddev.cli.ci.tests.rate_limiting import RateLimiterFactoryConfig
+from ddev.utils.github_async.retry import MUTATION_RETRY, SAFE_RETRY, RetryPolicies, RetryPolicy
if TYPE_CHECKING:
from ddev.repo.config import RepositoryConfig
@@ -26,6 +27,49 @@ class BatchingConfig(BaseModel):
allow_integration_splitting: bool = False
+class RetryLimitsConfig(BaseModel):
+ """Attempt and backoff limits for one class of GitHub request."""
+
+ model_config = ConfigDict(frozen=True, extra="forbid")
+
+ attempts: int = Field(default=3, ge=1)
+ # Bounds the whole ladder including backoff. None leaves attempts as the only stop condition.
+ timeout_seconds: float | None = Field(default=60.0, gt=0)
+ wait_initial_seconds: float = Field(default=0.5, gt=0)
+ wait_max_seconds: float = Field(default=10.0, gt=0)
+ wait_jitter_seconds: float = Field(default=1.0, ge=0)
+
+ def apply_to(self, policy: RetryPolicy) -> RetryPolicy:
+ """`policy` with these limits, keeping the conditions it retries on."""
+ return policy.replace(
+ attempts=self.attempts,
+ timeout=self.timeout_seconds,
+ wait_initial=self.wait_initial_seconds,
+ wait_max=self.wait_max_seconds,
+ wait_jitter=self.wait_jitter_seconds,
+ )
+
+
+class GitHubRetryConfig(BaseModel):
+ """Retry limits for the GitHub client, read from `[dispatcher.github_retries]`.
+
+ Only how hard to try is configurable. What may be retried follows from whether an endpoint can be
+ replayed, so widening it from a config file would make a duplicate side effect a setting.
+ """
+
+ model_config = ConfigDict(frozen=True, extra="forbid")
+
+ safe: RetryLimitsConfig = RetryLimitsConfig()
+ mutating: RetryLimitsConfig = RetryLimitsConfig(attempts=2)
+
+ def to_policies(self) -> RetryPolicies:
+ """Build the client's policies: these limits over the built-in conditions."""
+ return RetryPolicies(
+ safe=self.safe.apply_to(SAFE_RETRY),
+ mutating=self.mutating.apply_to(MUTATION_RETRY),
+ )
+
+
class DispatcherConfig(BaseModel):
"""Per-repository Dispatcher configuration."""
@@ -67,6 +111,9 @@ class DispatcherConfig(BaseModel):
github_rate_limits: RateLimiterFactoryConfig = RateLimiterFactoryConfig()
"""Rate limiter tiers shared by every task, from `[dispatcher.github_rate_limits]`."""
+ github_retries: GitHubRetryConfig = GitHubRetryConfig()
+ """How hard a failed GitHub request is retried, from `[dispatcher.github_retries]`."""
+
@classmethod
def from_repo_config(cls, repo_config: RepositoryConfig) -> DispatcherConfig:
"""Build a DispatcherConfig from the `/dispatcher` table of `.ddev/config.toml`."""
diff --git a/ddev/src/ddev/utils/github_async/AGENTS.md b/ddev/src/ddev/utils/github_async/AGENTS.md
index 3d839f2fc69d0..b7f04b1341add 100644
--- a/ddev/src/ddev/utils/github_async/AGENTS.md
+++ b/ddev/src/ddev/utils/github_async/AGENTS.md
@@ -103,6 +103,47 @@ Follow the block with an `Args:` section that describes each argument, and a
`Returns:` section describing the wrapped response. This keeps every method
traceable to its source contract and makes the public surface self-explanatory.
+## Two retry layers, and which one owns a failure
+
+Failures are retried in two places, and adding a retry to the wrong one is the mistake this section
+exists to prevent.
+
+- **Rate limiting** (`_rate_limited_request`, with `ddev.utils.rate_limiting`) owns 403 and 429
+ responses that the headers confirm are rate limiting. Re-acquiring the limiter *is* the backoff, so
+ there is no sleeping or arithmetic there. Never add a sleep or a wait to this layer.
+- **The retry strategy** (`_request`, with `retry.py`) owns everything else: transport failures and
+ 5xx. stamina executes it, so there is no backoff arithmetic here either.
+
+The strategy wraps the rate-limit layer, never the reverse, so every attempt re-acquires the limiter
+and waits out any pause the governor holds.
+
+When adding an endpoint method, decide its default by whether the request can be **replayed**, not by
+its verb:
+
+- A GET takes the default, `retry` forwarded straight through.
+- A mutation that is idempotent (sets given fields to given values, or is a no-op when repeated)
+ passes `retry=retry if retry is not None else self._retry_policies.safe` and says why in the
+ docstring. `update_check_run`, `update_issue_comment` and `add_labels_to_issue` are the examples.
+- Any other mutation takes the default, which only retries failures that prove the request never
+ reached GitHub. Widening one of these is how you get a duplicate workflow run or a second comment.
+
+What may be retried is not configurable; `[dispatcher.github_retries]` tunes attempts and backoff
+only. A config file that could widen the conditions would make a duplicate side effect a setting.
+
+The client reports its own retries through the logger passed to it, and stays quiet when there is
+none. Separately, stamina installs a process-wide `on_retry` hook that logs every scheduled retry to
+the `stamina` logger, so retries are visible even with no logger injected. That hook is global, and
+disabling it would also silence the unrelated `stamina.retry` in `ddev/e2e/agent/docker.py`, so it is
+left alone here rather than reached into from this package.
+
+## Never follow a redirect
+
+The client does not follow redirects, because the `Authorization` header would travel to whatever
+host `Location` names. An unexpected 3xx raises `GitHubUnexpectedRedirectError` naming the endpoint,
+and no policy may retry one. `download_artifact` is the single endpoint whose contract is a redirect;
+it opts in with `expect_redirect=True` and reads `Location` itself. Do not enable `follow_redirects`
+to make a failing request work.
+
## One method per API endpoint
Always keep exactly one method per API endpoint. Do not create convenience
diff --git a/ddev/src/ddev/utils/github_async/__init__.py b/ddev/src/ddev/utils/github_async/__init__.py
index edc0007d4a7d6..72f3674161e53 100644
--- a/ddev/src/ddev/utils/github_async/__init__.py
+++ b/ddev/src/ddev/utils/github_async/__init__.py
@@ -31,6 +31,12 @@
from .client import GitHubResponse as GitHubResponse
from .client import PaginationData as PaginationData
from .client import async_github_client as async_github_client
+ from .retry import DEFAULT_RETRY_POLICIES as DEFAULT_RETRY_POLICIES
+ from .retry import MUTATION_RETRY as MUTATION_RETRY
+ from .retry import NO_RETRY as NO_RETRY
+ from .retry import SAFE_RETRY as SAFE_RETRY
+ from .retry import RetryPolicies as RetryPolicies
+ from .retry import RetryPolicy as RetryPolicy
# Map of exported name -> submodule (relative to this package) that defines it.
MODULE_BY_NAME: dict[str, str] = {
@@ -41,6 +47,12 @@
'COMMENT_BODY_LIMIT': 'client',
'GitHubResponse': 'client',
'PaginationData': 'client',
+ 'RetryPolicy': 'retry',
+ 'RetryPolicies': 'retry',
+ 'NO_RETRY': 'retry',
+ 'SAFE_RETRY': 'retry',
+ 'MUTATION_RETRY': 'retry',
+ 'DEFAULT_RETRY_POLICIES': 'retry',
}
diff --git a/ddev/src/ddev/utils/github_async/client.py b/ddev/src/ddev/utils/github_async/client.py
index aac236ac31809..6cc607e06df63 100644
--- a/ddev/src/ddev/utils/github_async/client.py
+++ b/ddev/src/ddev/utils/github_async/client.py
@@ -6,6 +6,7 @@
from __future__ import annotations
import io
+import logging
import re
import zipfile
from collections.abc import AsyncIterator, Callable
@@ -15,18 +16,25 @@
from typing import Any, Literal, Self, overload
import httpx
+import stamina
from pydantic import BaseModel, ConfigDict, Field
from ddev.utils.github_errors import (
GITHUB_AUTHENTICATION_STATUS_CODES,
GitHubAuthenticationError,
GitHubBodyTooLongError,
+ GitHubUnexpectedRedirectError,
github_body_too_long_message,
github_secondary_rate_limit_wait,
)
-from ddev.utils.rate_limiting import NULL_SNAPSHOT, BudgetSnapshot, InstrumentedAsyncLimiter
+from ddev.utils.rate_limiting import (
+ NULL_SNAPSHOT,
+ BudgetSnapshot,
+ InstrumentedAsyncLimiter,
+ RateLimitWaitAbandoned,
+)
-from .defaults import default_github_rate_limiter
+from .defaults import default_github_rate_limiter, log_rate_limit_events
from .models import (
ArtifactsList,
CheckRun,
@@ -39,6 +47,15 @@
WorkflowJobsList,
WorkflowRun,
)
+from .retry import (
+ DEFAULT_RETRY_POLICIES,
+ NO_RETRY,
+ RetryPolicies,
+ RetryPolicy,
+ RetryPredicate,
+ on_status,
+ retry_attempts,
+)
GITHUB_API_VERSION = "2022-11-28"
DEFAULT_BASE_URL = "https://api.github.com"
@@ -48,6 +65,12 @@
# never below the character count GitHub means, so it errs only towards refusing a body it might take.
COMMENT_BODY_LIMIT = 65_536
+# How an expired signed URL presents from the artifact storage host.
+SIGNED_URL_EXPIRED_STATUS = 403
+
+# Stands in for a query string, which must not be logged.
+QUERY_MASK = "***" # noqa: S105
+
_LINK_RE = re.compile(r'<([^>]+)>;\s*rel="([^"]+)"')
@@ -111,6 +134,44 @@ def parse_header[T](headers: httpx.Headers, key: str, cast: Callable[[str], T])
return None
+def with_query_masked(text: str) -> str:
+ """`text` with everything from its first `?` onwards replaced.
+
+ Applied to a URL and to any message that might quote one. Nothing in a signed URL's query is worth
+ keeping, so none of it is parsed: no encoding, delimiter or parameter name has to be guessed right.
+ """
+ head, separator, _ = text.partition("?")
+ return f"{head}?{QUERY_MASK}" if separator else head
+
+
+def failure_reason(exc: httpx.HTTPError) -> str:
+ """Why a request failed, in a form that carries no query string.
+
+ A status error's message is rebuilt from the response, because httpx writes that one around the
+ full URL. A transport error's is its OS-level reason, masked in case it ever quotes one too.
+ """
+ if isinstance(exc, httpx.HTTPStatusError):
+ return f"HTTP {exc.response.status_code} {exc.response.reason_phrase}"
+ return with_query_masked(str(exc))
+
+
+class RetryCause:
+ """Carries the failure from the predicate, which sees it, to the log line, which counts attempts.
+
+ stamina exposes the attempt number but not the exception, so there is nowhere else to read it.
+ """
+
+ def __init__(self, should_retry: RetryPredicate) -> None:
+ self._should_retry = should_retry
+ self.error: Exception | None = None
+
+ def __call__(self, exc: Exception) -> bool:
+ if not self._should_retry(exc):
+ return False
+ self.error = exc
+ return True
+
+
def github_rate_limit_snapshot(headers: httpx.Headers) -> BudgetSnapshot | None:
"""Parse GitHub's `x-ratelimit-*` / `retry-after` response headers into a BudgetSnapshot."""
snapshot = BudgetSnapshot(
@@ -131,8 +192,12 @@ class AsyncGitHubClient:
Rate-limit protection is on by default: requests are paced and, when GitHub signals a
rate-limit rejection, retried in reaction to the response headers. The governor supplies the
- backoff, so there is no sleeping or backoff arithmetic in this client. The default protection
- logs through the ``ddev.utils.github_async.defaults`` logger.
+ backoff, so there is no sleeping or backoff arithmetic in this client.
+
+ Failures that are *not* rate limiting, a dropped connection or a 502, are handled separately by
+ the retry strategy in `retry.py`. The two layers answer different questions, "GitHub told us to
+ wait" against "that request did not land, ask again", and each endpoint method takes a `retry`
+ argument to override its default for a single call.
Args:
token: GitHub token; must be non-empty.
@@ -150,9 +215,14 @@ class AsyncGitHubClient:
max_rate_limit_retries: Extra attempts for a header-confirmed rate-limit response (403/429).
Each retry is a full fresh acquisition (governor wait plus bucket token); the default of
2 covers the common "hit a secondary limit once, wait, succeed" case plus one repeat.
- Only rate-limit responses are retried: transport errors and non-rate-limit statuses
- propagate immediately, and RateLimitWaitAbandoned (the governor's ``max_wait_seconds``
- killswitch) propagates to the caller.
+ Only rate-limit responses are retried here; every other failure belongs to the retry
+ strategy, and RateLimitWaitAbandoned (the governor's `max_wait_seconds` killswitch)
+ reaches the caller untouched by either layer.
+ retry_policies: The retry strategies to use for failures that are not rate limiting, one per
+ replay-safety class. Defaults to `DEFAULT_RETRY_POLICIES`.
+ logger: Logger this client writes to. None keeps it silent. It also receives the events of the
+ default rate limiter, while a caller-supplied `rate_limiter` keeps whatever logging it was
+ built with, since that choice belongs to whoever built it.
transport: Optional custom HTTPX transport (useful for testing with MockTransport).
"""
@@ -163,18 +233,29 @@ def __init__(
rate_limiter: InstrumentedAsyncLimiter | None = None,
default_timeout: float = 30.0,
max_rate_limit_retries: int = 2,
+ retry_policies: RetryPolicies | None = None,
+ logger: logging.Logger | None = None,
transport: httpx.AsyncBaseTransport | None = None,
) -> None:
if not token:
raise ValueError("GitHub token must not be empty.")
+ self._logger = logger
# A None limiter means "use the default protection," not "no protection." The local bucket
# is deliberately permissive because the governor is the protection; with a healthy budget
# and no secondary limits the governor adds zero wait, so this default is invisible to
# well-behaved callers and engages only once GitHub has already signaled backpressure.
- self._rate_limiter = rate_limiter if rate_limiter is not None else default_github_rate_limiter()
+ self._rate_limiter = (
+ rate_limiter
+ if rate_limiter is not None
+ else default_github_rate_limiter(on_event=log_rate_limit_events(logger) if logger is not None else None)
+ )
self._default_timeout = default_timeout
self._max_rate_limit_retries = max_rate_limit_retries
+ self._retry_policies = retry_policies if retry_policies is not None else DEFAULT_RETRY_POLICIES
+ # A 403 from GitHub itself arrives as GitHubAuthenticationError, which the guard refuses, so
+ # adding this one cannot turn a permission denial into a retry loop.
+ self._artifact_retry = self._retry_policies.safe.also_on(on_status(SIGNED_URL_EXPIRED_STATUS))
self._headers = {
"Authorization": f"Bearer {token}",
"X-GitHub-Api-Version": GITHUB_API_VERSION,
@@ -198,6 +279,38 @@ async def aclose(self) -> None:
def _effective_timeout(self, timeout: float | None) -> float:
return timeout if timeout is not None else self._default_timeout
+ def _retry_cause(self, policy: RetryPolicy) -> RetryCause:
+ """The predicate for one operation: what `policy` accepts, minus what this client refuses."""
+
+ def should_retry(exc: Exception) -> bool:
+ return not self._refuses_retry(exc) and policy.should_retry(exc)
+
+ return RetryCause(should_retry)
+
+ def _refuses_retry(self, exc: Exception) -> bool:
+ """Failures no policy may retry.
+
+ Auth does not improve by asking again, rate limiting belongs to the limiter whose pause is the
+ correct wait, and a redirect is an answer rather than a failure.
+ """
+ if isinstance(exc, (GitHubAuthenticationError, RateLimitWaitAbandoned)):
+ return True
+ if isinstance(exc, httpx.HTTPStatusError):
+ return self._is_rate_limit_response(exc.response) or exc.response.has_redirect_location
+ return False
+
+ def _log_retry(self, description: str, cause: RetryCause, attempt: stamina.Attempt) -> None:
+ """Report a retry that is about to run. Silent on the first attempt, and with no logger."""
+ if attempt.num == 1 or self._logger is None:
+ return
+ self._logger.warning(
+ "Retrying %s after %r (attempt %s)",
+ description,
+ cause.error,
+ attempt.num,
+ extra={"attempt": attempt.num, "error": repr(cause.error)},
+ )
+
@staticmethod
def _is_rate_limit_response(response: httpx.Response) -> bool:
"""Whether *response* is a retryable rate-limit rejection, by GitHub's own discrimination rule.
@@ -218,12 +331,17 @@ async def _execute_request(
method: str,
endpoint: str,
timeout: float,
+ *,
+ expect_redirect: bool = False,
**kwargs: Any,
) -> httpx.Response:
try:
response = await self._client.request(method, endpoint, timeout=timeout, **kwargs)
except httpx.TransportError as exc:
- raise type(exc)(f"{method} {endpoint}: {exc}") from exc
+ # Rewritten in place rather than replaced by a copy, which would drop the request httpx
+ # attached and leave `exc.request` raising RuntimeError for the caller.
+ exc.args = (f"{method} {endpoint}: {exc}",)
+ raise
# Observe before raise_for_status, never after: learning must not be gated on success. A
# failed response's rate-limit headers arm the shared pause even if the caller swallows the
# exception, so one request's 403 protects every other in-flight and future request in this
@@ -234,14 +352,22 @@ async def _execute_request(
snapshot = replace(snapshot or NULL_SNAPSHOT, retry_after=secondary_rate_limit_wait)
if snapshot is not None:
self._rate_limiter.observe(snapshot)
+ # The artifact endpoint checks the redirect itself, and reports a bad one more precisely.
+ if expect_redirect and response.is_redirect:
+ return response
+ # Not `is_redirect`, which spans the whole 3xx range: a 304 carries no Location to refuse.
+ if response.has_redirect_location:
+ raise GitHubUnexpectedRedirectError.from_response(method, endpoint, response)
response.raise_for_status()
return response
- async def _request(
+ async def _rate_limited_request(
self,
method: str,
endpoint: str,
timeout: float | None = None,
+ *,
+ expect_redirect: bool = False,
**kwargs: Any,
) -> httpx.Response:
effective_timeout = self._effective_timeout(timeout)
@@ -258,7 +384,9 @@ async def _request(
for attempt in range(self._max_rate_limit_retries + 1):
async with self._rate_limiter:
try:
- return await self._execute_request(method, endpoint, effective_timeout, **kwargs)
+ return await self._execute_request(
+ method, endpoint, effective_timeout, expect_redirect=expect_redirect, **kwargs
+ )
except httpx.HTTPStatusError as exc:
# A rate-limit 403/429 is safe to retry for every endpoint, including
# non-idempotent POSTs, precisely because GitHub rejected it without performing
@@ -273,6 +401,31 @@ async def _request(
if exc.response.status_code in GITHUB_AUTHENTICATION_STATUS_CODES:
raise GitHubAuthenticationError.from_http_status_error(exc) from exc
raise
+ raise RuntimeError("unreachable: the rate-limit loop always returns or raises") # pragma: no cover
+
+ async def _request(
+ self,
+ method: str,
+ endpoint: str,
+ timeout: float | None = None,
+ *,
+ retry: RetryPolicy | None = None,
+ expect_redirect: bool = False,
+ **kwargs: Any,
+ ) -> httpx.Response:
+ """Send one request, retrying the failures its policy accepts.
+
+ Wraps the rate-limit layer rather than living inside it, so every attempt re-acquires the
+ limiter and waits out any pause the governor holds.
+ """
+ policy = retry if retry is not None else self._retry_policies.for_method(method)
+ cause = self._retry_cause(policy)
+ async for attempt in retry_attempts(policy, cause):
+ with attempt:
+ self._log_retry(f"{method} {endpoint}", cause, attempt)
+ return await self._rate_limited_request(
+ method, endpoint, timeout, expect_redirect=expect_redirect, **kwargs
+ )
raise RuntimeError("unreachable: the retry loop always returns or raises") # pragma: no cover
async def _paginated_request(
@@ -280,18 +433,24 @@ async def _paginated_request(
method: str,
endpoint: str,
timeout: float | None = None,
+ *,
+ retry: RetryPolicy | None = None,
**kwargs: Any,
) -> AsyncIterator[httpx.Response]:
- """Yield one httpx.Response per page, following Link headers."""
+ """Yield one httpx.Response per page, following Link headers.
+
+ The policy applies per page, so a flaky page is retried on its own without refetching the
+ pages already yielded.
+ """
url: str | None = endpoint
first = True
while url is not None:
if first:
- response = await self._request(method, url, timeout=timeout, **kwargs)
+ response = await self._request(method, url, timeout=timeout, retry=retry, **kwargs)
first = False
else:
# Subsequent pages: use the absolute next URL, no extra kwargs
- response = await self._request(method, url, timeout=timeout)
+ response = await self._request(method, url, timeout=timeout, retry=retry)
yield response
pagination = PaginationData.from_header(response.headers.get("link"))
url = pagination.next
@@ -317,6 +476,7 @@ async def create_workflow_dispatch(
inputs: dict[str, str] | None = None,
timeout: float | None = None,
*,
+ retry: RetryPolicy | None = None,
return_run_details: Literal[True],
) -> GitHubResponse[WorkflowDispatchResult]: ...
@@ -330,6 +490,7 @@ async def create_workflow_dispatch(
inputs: dict[str, str] | None = None,
timeout: float | None = None,
*,
+ retry: RetryPolicy | None = None,
return_run_details: Literal[False] = False,
) -> GitHubResponse[None]: ...
@@ -342,6 +503,7 @@ async def create_workflow_dispatch(
inputs: dict[str, str] | None = None,
timeout: float | None = None,
*,
+ retry: RetryPolicy | None = None,
return_run_details: bool = False,
) -> GitHubResponse[WorkflowDispatchResult] | GitHubResponse[None]:
"""
@@ -357,6 +519,7 @@ async def create_workflow_dispatch(
ref: Branch or tag name to run the workflow on.
inputs: Optional key/value inputs forwarded to the workflow.
timeout: Optional timeout for this specific request. Defaults to the client's default_timeout.
+ retry: Defaults to the mutating policy; a replayed dispatch could start a duplicate run.
return_run_details: When True, requests a 200 response with the new run's metadata
(workflow_run_id, run_url, html_url) instead of the default 204 No Content.
See https://github.blog/changelog/2026-02-19-workflow-dispatch-api-now-returns-run-ids/.
@@ -375,6 +538,7 @@ async def create_workflow_dispatch(
"POST",
f"/repos/{owner}/{repo}/actions/workflows/{workflow_id}/dispatches",
timeout=timeout,
+ retry=retry,
json=body,
)
if return_run_details:
@@ -387,6 +551,8 @@ async def get_workflow_run(
repo: str,
run_id: int,
timeout: float | None = None,
+ *,
+ retry: RetryPolicy | None = None,
) -> GitHubResponse[WorkflowRun]:
"""
Calls the GitHub API to get a single workflow run.
@@ -399,11 +565,14 @@ async def get_workflow_run(
repo: Repository name.
run_id: Numeric ID of the workflow run.
timeout: Optional timeout for this specific request. Defaults to the client's default_timeout.
+ retry: Defaults to the client's policy for replayable requests.
Returns:
GitHubResponse[WorkflowRun]: The validated workflow run data and headers.
"""
- response = await self._request("GET", f"/repos/{owner}/{repo}/actions/runs/{run_id}", timeout=timeout)
+ response = await self._request(
+ "GET", f"/repos/{owner}/{repo}/actions/runs/{run_id}", timeout=timeout, retry=retry
+ )
return self._parse_response(response, WorkflowRun)
async def list_workflow_run_artifacts(
@@ -413,6 +582,8 @@ async def list_workflow_run_artifacts(
run_id: int,
per_page: int = 30,
timeout: float | None = None,
+ *,
+ retry: RetryPolicy | None = None,
) -> AsyncIterator[GitHubResponse[ArtifactsList]]:
"""
Calls the GitHub API to list artifacts for a workflow run (paginated).
@@ -426,12 +597,15 @@ async def list_workflow_run_artifacts(
run_id: Numeric ID of the workflow run.
per_page: Number of artifacts per page (default 30, max 100).
timeout: Optional timeout for this specific request. Defaults to the client's default_timeout.
+ retry: Applies per page. Defaults to the client's policy for replayable requests.
Returns:
AsyncIterator[GitHubResponse[ArtifactsList]]: One page of artifacts per iteration.
"""
endpoint = f"/repos/{owner}/{repo}/actions/runs/{run_id}/artifacts"
- async for response in self._paginated_request("GET", endpoint, timeout=timeout, params={"per_page": per_page}):
+ async for response in self._paginated_request(
+ "GET", endpoint, timeout=timeout, retry=retry, params={"per_page": per_page}
+ ):
yield self._parse_response(response, ArtifactsList)
async def list_workflow_jobs(
@@ -441,6 +615,8 @@ async def list_workflow_jobs(
run_id: int,
per_page: int = 30,
timeout: float | None = None,
+ *,
+ retry: RetryPolicy | None = None,
) -> AsyncIterator[GitHubResponse[WorkflowJobsList]]:
"""
Calls the GitHub API to list jobs for a workflow run (paginated).
@@ -454,12 +630,15 @@ async def list_workflow_jobs(
run_id: Numeric ID of the workflow run.
per_page: Number of jobs per page (default 30, max 100).
timeout: Optional timeout for this specific request. Defaults to the client's default_timeout.
+ retry: Applies per page. Defaults to the client's policy for replayable requests.
Returns:
AsyncIterator[GitHubResponse[WorkflowJobsList]]: One page of jobs per iteration.
"""
endpoint = f"/repos/{owner}/{repo}/actions/runs/{run_id}/jobs"
- async for response in self._paginated_request("GET", endpoint, timeout=timeout, params={"per_page": per_page}):
+ async for response in self._paginated_request(
+ "GET", endpoint, timeout=timeout, retry=retry, params={"per_page": per_page}
+ ):
yield self._parse_response(response, WorkflowJobsList)
async def create_issue_comment(
@@ -469,6 +648,8 @@ async def create_issue_comment(
issue_number: int,
body: str,
timeout: float | None = None,
+ *,
+ retry: RetryPolicy | None = None,
) -> GitHubResponse[IssueComment]:
"""
Calls the GitHub API to create a comment on an issue or pull request.
@@ -482,6 +663,7 @@ async def create_issue_comment(
issue_number: Issue or pull request number.
body: Markdown body text of the comment. At most `COMMENT_BODY_LIMIT` UTF-8 bytes.
timeout: Optional timeout for this specific request. Defaults to the client's default_timeout.
+ retry: Defaults to the mutating policy; a replayed create leaves a second comment.
Returns:
GitHubResponse[IssueComment]: The validated comment data and headers.
@@ -490,7 +672,7 @@ async def create_issue_comment(
GitHubBodyTooLongError: If `body` is too long, measured here or refused by GitHub.
"""
response = await self._comment_request(
- "POST", f"/repos/{owner}/{repo}/issues/{issue_number}/comments", body=body, timeout=timeout
+ "POST", f"/repos/{owner}/{repo}/issues/{issue_number}/comments", body=body, timeout=timeout, retry=retry
)
return self._parse_response(response, IssueComment)
@@ -501,6 +683,8 @@ async def update_issue_comment(
comment_id: int,
body: str,
timeout: float | None = None,
+ *,
+ retry: RetryPolicy | None = None,
) -> GitHubResponse[IssueComment]:
"""
Calls the GitHub API to update an existing comment on an issue or pull request.
@@ -514,6 +698,7 @@ async def update_issue_comment(
comment_id: Numeric ID of the comment to update.
body: New markdown body text of the comment. At most `COMMENT_BODY_LIMIT` UTF-8 bytes.
timeout: Optional timeout for this specific request. Defaults to the client's default_timeout.
+ retry: Defaults to the replayable policy; setting one comment to one body is idempotent.
Returns:
GitHubResponse[IssueComment]: The validated comment data and headers.
@@ -522,11 +707,23 @@ async def update_issue_comment(
GitHubBodyTooLongError: If `body` is too long, measured here or refused by GitHub.
"""
response = await self._comment_request(
- "PATCH", f"/repos/{owner}/{repo}/issues/comments/{comment_id}", body=body, timeout=timeout
+ "PATCH",
+ f"/repos/{owner}/{repo}/issues/comments/{comment_id}",
+ body=body,
+ timeout=timeout,
+ retry=retry if retry is not None else self._retry_policies.safe,
)
return self._parse_response(response, IssueComment)
- async def _comment_request(self, method: str, endpoint: str, *, body: str, timeout: float | None) -> httpx.Response:
+ async def _comment_request(
+ self,
+ method: str,
+ endpoint: str,
+ *,
+ body: str,
+ timeout: float | None,
+ retry: RetryPolicy | None = None,
+ ) -> httpx.Response:
"""Send a comment *body*, enforcing the length limit from both sides.
Scoped to the comment endpoints rather than `_request`, because a 422 elsewhere has nothing to do
@@ -534,7 +731,7 @@ async def _comment_request(self, method: str, endpoint: str, *, body: str, timeo
"""
_ensure_body_fits(body)
try:
- return await self._request(method, endpoint, timeout=timeout, json={"body": body})
+ return await self._request(method, endpoint, timeout=timeout, retry=retry, json={"body": body})
except httpx.HTTPStatusError as exc:
if (message := github_body_too_long_message(exc.response)) is not None:
raise GitHubBodyTooLongError.from_response(message, limit=COMMENT_BODY_LIMIT) from exc
@@ -547,6 +744,8 @@ async def list_issue_comments(
issue_number: int,
per_page: int = 100,
timeout: float | None = None,
+ *,
+ retry: RetryPolicy | None = None,
) -> AsyncIterator[GitHubResponse[list[IssueComment]]]:
"""
Calls the GitHub API to list comments on an issue or pull request (paginated).
@@ -560,6 +759,7 @@ async def list_issue_comments(
issue_number: Issue or pull request number.
per_page: Number of comments per page (default 100, GitHub's maximum).
timeout: Optional timeout for this specific request. Defaults to the client's default_timeout.
+ retry: Applies per page. Defaults to the client's policy for replayable requests.
Returns:
AsyncIterator[GitHubResponse[list[IssueComment]]]: One page of comments per iteration,
@@ -568,7 +768,9 @@ async def list_issue_comments(
# The response body is a bare JSON array, so there is no wrapper model to validate against
# (unlike ``WorkflowJobsList``); each item is validated individually.
endpoint = f"/repos/{owner}/{repo}/issues/{issue_number}/comments"
- async for response in self._paginated_request("GET", endpoint, timeout=timeout, params={"per_page": per_page}):
+ async for response in self._paginated_request(
+ "GET", endpoint, timeout=timeout, retry=retry, params={"per_page": per_page}
+ ):
comments = [IssueComment.model_validate(item) for item in response.json()]
yield GitHubResponse[list[IssueComment]].model_validate(
{"data": comments, "headers": dict(response.headers)}
@@ -580,6 +782,8 @@ async def get_pull_request(
repo: str,
pull_number: int,
timeout: float | None = None,
+ *,
+ retry: RetryPolicy | None = None,
) -> GitHubResponse[PullRequest]:
"""
Calls the GitHub API to get a single pull request.
@@ -592,11 +796,14 @@ async def get_pull_request(
repo: Repository name.
pull_number: Pull request number.
timeout: Optional timeout for this specific request. Defaults to the client's default_timeout.
+ retry: Defaults to the replayable policy, which does not retry a 404.
Returns:
GitHubResponse[PullRequest]: The validated pull request data and headers.
"""
- response = await self._request("GET", f"/repos/{owner}/{repo}/pulls/{pull_number}", timeout=timeout)
+ response = await self._request(
+ "GET", f"/repos/{owner}/{repo}/pulls/{pull_number}", timeout=timeout, retry=retry
+ )
return self._parse_response(response, PullRequest)
async def list_pull_requests(
@@ -608,6 +815,8 @@ async def list_pull_requests(
base: str | None = None,
per_page: int = 100,
timeout: float | None = None,
+ *,
+ retry: RetryPolicy | None = None,
) -> GitHubResponse[list[PullRequest]]:
"""
Calls the GitHub API to list pull requests in a repository.
@@ -625,6 +834,7 @@ async def list_pull_requests(
base: Filter by base branch name.
per_page: Number of results per page (max 100). Only the first page is fetched.
timeout: Optional timeout for this specific request. Defaults to the client's default_timeout.
+ retry: Defaults to the client's policy for replayable requests.
Returns:
GitHubResponse[list[PullRequest]]: The validated pull requests on the first result page.
@@ -634,7 +844,9 @@ async def list_pull_requests(
params["head"] = head
if base is not None:
params["base"] = base
- response = await self._request("GET", f"/repos/{owner}/{repo}/pulls", timeout=timeout, params=params)
+ response = await self._request(
+ "GET", f"/repos/{owner}/{repo}/pulls", timeout=timeout, retry=retry, params=params
+ )
pulls = [PullRequest.model_validate(item) for item in response.json()]
return GitHubResponse[list[PullRequest]].model_validate({"data": pulls, "headers": dict(response.headers)})
@@ -648,6 +860,8 @@ async def create_pull_request(
body: str = "",
draft: bool = False,
timeout: float | None = None,
+ *,
+ retry: RetryPolicy | None = None,
) -> GitHubResponse[PullRequest]:
"""
Calls the GitHub API to create a pull request.
@@ -664,6 +878,7 @@ async def create_pull_request(
body: Pull request body.
draft: Whether to open the pull request as a draft.
timeout: Optional timeout for this specific request. Defaults to the client's default_timeout.
+ retry: Defaults to the mutating policy; a replayed create opens a second pull request.
Returns:
GitHubResponse[PullRequest]: The validated pull request data and headers.
@@ -672,6 +887,7 @@ async def create_pull_request(
"POST",
f"/repos/{owner}/{repo}/pulls",
timeout=timeout,
+ retry=retry,
json={"title": title, "head": head, "base": base, "body": body, "draft": draft},
)
return self._parse_response(response, PullRequest)
@@ -683,6 +899,8 @@ async def add_labels_to_issue(
issue_number: int,
labels: list[str],
timeout: float | None = None,
+ *,
+ retry: RetryPolicy | None = None,
) -> GitHubResponse[list[Label]]:
"""
Calls the GitHub API to add one or more labels to an issue or pull request.
@@ -696,6 +914,7 @@ async def add_labels_to_issue(
issue_number: Issue or pull request number.
labels: Labels to add. Existing labels on the issue are preserved.
timeout: Optional timeout for this specific request. Defaults to the client's default_timeout.
+ retry: Defaults to the replayable policy; adding a label twice is a no-op.
Returns:
GitHubResponse[list[Label]]: The full label list resulting from the operation (preserves
@@ -705,6 +924,7 @@ async def add_labels_to_issue(
"POST",
f"/repos/{owner}/{repo}/issues/{issue_number}/labels",
timeout=timeout,
+ retry=retry if retry is not None else self._retry_policies.safe,
json={"labels": labels},
)
labels_out = [Label.model_validate(item) for item in response.json()]
@@ -722,6 +942,8 @@ async def create_pr_review_comment(
line: int | None = None,
side: Literal["LEFT", "RIGHT"] | None = None,
timeout: float | None = None,
+ *,
+ retry: RetryPolicy | None = None,
) -> GitHubResponse[PullRequestReviewComment]:
"""
Calls the GitHub API to create an inline review comment on a pull request diff.
@@ -740,6 +962,7 @@ async def create_pr_review_comment(
line: Line number in the file (newer style, paired with side).
side: 'LEFT' or 'RIGHT' (newer style, paired with line).
timeout: Optional timeout for this specific request. Defaults to the client's default_timeout.
+ retry: Defaults to the mutating policy; a replayed create leaves a second review comment.
Returns:
GitHubResponse[PullRequestReviewComment]: The validated comment data and headers.
@@ -759,6 +982,7 @@ async def create_pr_review_comment(
"POST",
f"/repos/{owner}/{repo}/pulls/{pull_number}/comments",
timeout=timeout,
+ retry=retry,
json=payload,
)
return self._parse_response(response, PullRequestReviewComment)
@@ -773,6 +997,8 @@ async def create_check_run(
details_url: str | None = None,
output: dict[str, Any] | None = None,
timeout: float | None = None,
+ *,
+ retry: RetryPolicy | None = None,
) -> GitHubResponse[CheckRun]:
"""
Calls the GitHub API to create a check run on a commit.
@@ -789,6 +1015,7 @@ async def create_check_run(
details_url: Optional URL the check title links to.
output: Optional structured output (title, summary, ...).
timeout: Optional timeout for this specific request. Defaults to the client's default_timeout.
+ retry: Defaults to the mutating policy; a replayed create leaves a second check run.
Returns:
GitHubResponse[CheckRun]: The validated check run data and headers.
@@ -802,6 +1029,7 @@ async def create_check_run(
"POST",
f"/repos/{owner}/{repo}/check-runs",
timeout=timeout,
+ retry=retry,
json=payload,
)
return self._parse_response(response, CheckRun)
@@ -816,6 +1044,8 @@ async def update_check_run(
details_url: str | None = None,
output: dict[str, Any] | None = None,
timeout: float | None = None,
+ *,
+ retry: RetryPolicy | None = None,
) -> GitHubResponse[CheckRun]:
"""
Calls the GitHub API to update an existing check run.
@@ -832,6 +1062,7 @@ async def update_check_run(
details_url: Optional URL the check title links to.
output: Optional structured output (title, summary, ...).
timeout: Optional timeout for this specific request. Defaults to the client's default_timeout.
+ retry: Defaults to the replayable policy; setting fields to given values is idempotent.
Returns:
GitHubResponse[CheckRun]: The validated check run data and headers.
@@ -851,6 +1082,7 @@ async def update_check_run(
"PATCH",
f"/repos/{owner}/{repo}/check-runs/{check_run_id}",
timeout=timeout,
+ retry=retry if retry is not None else self._retry_policies.safe,
json=payload,
)
return self._parse_response(response, CheckRun)
@@ -859,23 +1091,23 @@ async def _resolve_artifact_redirect(
self,
archive_download_url: str,
timeout: float | None = None,
+ *,
+ retry: RetryPolicy | None = None,
) -> str:
- """Authenticated GET; return the unauthenticated signed URL from the 302 Location header."""
- try:
- redirect_response = await self._request(
- "GET", archive_download_url, timeout=timeout, follow_redirects=False
- )
- except GitHubAuthenticationError:
- raise
- except httpx.HTTPStatusError as exc:
- # httpx.raise_for_status() treats the expected 302 as an error since it isn't a 2xx;
- # recover the response from the exception so the redirect can still be inspected below.
- # The retry layer in _request is deliberately transparent here: a 302 is not a rate-limit
- # response by _is_rate_limit_response, so it is never retried and surfaces on the first
- # attempt exactly as before.
- redirect_response = exc.response
+ """Authenticated GET; return the unauthenticated signed URL from the 302 Location header.
+
+ The one endpoint whose contract is a redirect, so it asks for the response instead of having
+ it reported as an unexpected one.
+ """
+ redirect_response = await self._request(
+ "GET",
+ archive_download_url,
+ timeout=timeout,
+ retry=retry,
+ expect_redirect=True,
+ follow_redirects=False,
+ )
if redirect_response.status_code != 302:
- redirect_response.raise_for_status()
raise httpx.HTTPError(
f"Expected 302 redirect from {archive_download_url}, got {redirect_response.status_code}"
)
@@ -890,11 +1122,20 @@ async def _download_and_extract_zip(
dest_path: Path,
timeout: float | None = None,
) -> None:
- """Anonymous fetch (no bearer token to S3) + zip-slip-validated extractall."""
+ """Anonymous fetch (no bearer token to S3) + zip-slip-validated extractall.
+
+ A failure here reports without the query string, which is where the signed URL keeps its
+ signature, because the message reaches logs, stamina's retry hook and any traceback.
+ """
effective_timeout = self._effective_timeout(timeout)
async with httpx.AsyncClient(timeout=effective_timeout) as anonymous_client:
- download_response = await anonymous_client.get(signed_url)
- download_response.raise_for_status()
+ try:
+ download_response = await anonymous_client.get(signed_url)
+ download_response.raise_for_status()
+ except httpx.HTTPError as exc:
+ # Rewritten in place so the type, the request and the frames survive.
+ exc.args = (f"artifact download from {with_query_masked(signed_url)}: {failure_reason(exc)}",)
+ raise
dest_path.mkdir(parents=True, exist_ok=True)
with zipfile.ZipFile(io.BytesIO(download_response.content)) as zf:
@@ -913,6 +1154,8 @@ async def download_artifact(
archive_download_url: str,
dest_path: Path,
timeout: float | None = None,
+ *,
+ retry: RetryPolicy | None = None,
) -> None:
"""
Downloads and extracts a workflow run artifact zip into ``dest_path``.
@@ -927,16 +1170,24 @@ async def download_artifact(
bearer token is not leaked to the redirect target. Each zip member is
validated against ``dest_path`` before extraction (zip-slip protection).
- This performs a single attempt with no retries; any failure propagates to the
- caller.
+ Both requests are retried as one unit, so a retry resolves a fresh signed URL rather than
+ refetching an expired one. Nothing is written until the whole zip is in memory.
Args:
archive_download_url: The artifact's ``archive_download_url`` (absolute or relative to the API base).
dest_path: Directory where the zip contents will be extracted. Created if missing.
timeout: Optional timeout for both HTTP requests.
+ retry: Defaults to the replayable policy plus the 403 an expired signed URL produces.
"""
- location = await self._resolve_artifact_redirect(archive_download_url, timeout)
- await self._download_and_extract_zip(location, dest_path, timeout)
+ policy = retry if retry is not None else self._artifact_retry
+ cause = self._retry_cause(policy)
+ async for attempt in retry_attempts(policy, cause):
+ with attempt:
+ self._log_retry(f"artifact download {with_query_masked(archive_download_url)}", cause, attempt)
+ # NO_RETRY on the inner call: this loop is the only ladder, or the two would multiply.
+ location = await self._resolve_artifact_redirect(archive_download_url, timeout, retry=NO_RETRY)
+ await self._download_and_extract_zip(location, dest_path, timeout)
+ return
# ---------------------------------------------------------------------------
@@ -951,16 +1202,17 @@ async def async_github_client(
rate_limiter: InstrumentedAsyncLimiter | None = None,
default_timeout: float = 30.0,
max_rate_limit_retries: int = 2,
+ retry_policies: RetryPolicies | None = None,
+ logger: logging.Logger | None = None,
transport: httpx.AsyncBaseTransport | None = None,
) -> AsyncIterator[AsyncGitHubClient]:
"""
Async context manager that creates an AsyncGitHubClient and ensures it is closed on exit.
Rate-limit protection is on by default; the governor paces requests and supplies the backoff for
- retries. Header-confirmed rate-limit responses (403/429) are retried, transport errors and
- non-rate-limit statuses are not, and RateLimitWaitAbandoned propagates to the caller when the
- governor is configured with a wait budget. The default protection logs through the
- ``ddev.utils.github_async.defaults`` logger.
+ those retries. Header-confirmed rate-limit responses (403/429) are retried there, and
+ RateLimitWaitAbandoned propagates to the caller when the governor is configured with a wait
+ budget. Other failures are handled by the retry strategy in `retry.py`.
Args:
token: GitHub personal access token or app token.
@@ -970,6 +1222,8 @@ async def async_github_client(
default_timeout: Default per-request HTTP timeout in seconds. Bounds individual HTTP
requests only, not governor waits.
max_rate_limit_retries: Extra attempts for a header-confirmed rate-limit response.
+ retry_policies: Overrides the per-endpoint defaults for failures that are not rate limiting.
+ logger: Where retries are reported; None keeps the client silent.
transport: Optional custom HTTPX transport (useful for testing with MockTransport).
Yields:
@@ -980,6 +1234,8 @@ async def async_github_client(
rate_limiter=rate_limiter,
default_timeout=default_timeout,
max_rate_limit_retries=max_rate_limit_retries,
+ retry_policies=retry_policies,
+ logger=logger,
transport=transport,
)
try:
diff --git a/ddev/src/ddev/utils/github_async/retry.py b/ddev/src/ddev/utils/github_async/retry.py
new file mode 100644
index 0000000000000..714566f7b89f5
--- /dev/null
+++ b/ddev/src/ddev/utils/github_async/retry.py
@@ -0,0 +1,190 @@
+# (C) Datadog, Inc. 2026-present
+# All rights reserved
+# Licensed under a 3-clause BSD style license (see LICENSE)
+"""Retry strategy for requests that failed for a reason other than rate limiting.
+
+Rate limiting is handled elsewhere (`ddev.utils.rate_limiting`), where re-acquiring the limiter is
+itself the backoff. This module covers the rest: a policy says what to retry and how hard to try,
+and stamina executes it.
+"""
+
+from __future__ import annotations
+
+from dataclasses import dataclass
+from enum import Enum, auto
+from typing import TYPE_CHECKING
+
+import httpx
+import stamina
+
+if TYPE_CHECKING:
+ from collections.abc import AsyncIterator, Callable
+
+type RetryPredicate = Callable[[Exception], bool]
+
+# Transport failures raised before any byte of the request reached the server, so a replay cannot
+# repeat a side effect. Read and write failures are absent on purpose.
+PRE_SEND_TRANSPORT_ERRORS = (httpx.ConnectError, httpx.ConnectTimeout, httpx.PoolTimeout)
+
+RETRYABLE_SERVER_STATUSES = frozenset((500, 502, 503, 504))
+
+REPLAYABLE_HTTP_METHODS = frozenset({"GET", "HEAD", "OPTIONS"})
+
+DEFAULT_ATTEMPTS = 3
+DEFAULT_TIMEOUT_SECONDS = 60.0
+DEFAULT_WAIT_INITIAL_SECONDS = 0.5
+DEFAULT_WAIT_MAX_SECONDS = 10.0
+DEFAULT_WAIT_JITTER_SECONDS = 1.0
+
+# A mutation retries only what provably never left, which a second attempt either fixes at once or
+# is unlikely to fix at all.
+MUTATION_ATTEMPTS = 2
+
+
+class Unset(Enum):
+ """Sentinel for `RetryPolicy.replace`, where `timeout=None` means no timeout, not unchanged."""
+
+ TOKEN = auto()
+
+
+UNSET = Unset.TOKEN
+
+
+# ---------------------------------------------------------------------------
+# Predicates
+# ---------------------------------------------------------------------------
+
+
+def never(exc: Exception) -> bool:
+ """Refuse everything."""
+ return False
+
+
+def on_transport_error(exc: Exception) -> bool:
+ """Any transport failure, whether or not the request reached the server."""
+ return isinstance(exc, httpx.TransportError)
+
+
+def on_pre_send_transport_error(exc: Exception) -> bool:
+ """Only the transport failures that prove the request never left."""
+ return isinstance(exc, PRE_SEND_TRANSPORT_ERRORS)
+
+
+def on_status(*status_codes: int) -> RetryPredicate:
+ """Responses whose status is one of `status_codes`."""
+ wanted = frozenset(status_codes)
+
+ def matches(exc: Exception) -> bool:
+ return isinstance(exc, httpx.HTTPStatusError) and exc.response.status_code in wanted
+
+ return matches
+
+
+def any_of(*predicates: RetryPredicate) -> RetryPredicate:
+ """Accept what any of `predicates` accepts."""
+
+ def matches(exc: Exception) -> bool:
+ return any(predicate(exc) for predicate in predicates)
+
+ return matches
+
+
+# ---------------------------------------------------------------------------
+# Policies
+# ---------------------------------------------------------------------------
+
+
+@dataclass(frozen=True, slots=True)
+class RetryPolicy:
+ """What to retry, and how hard to try.
+
+ Frozen because the defaults below are shared for the life of the process; `replace`, `also_on` and
+ `unless` return new policies. `timeout` bounds the whole ladder, backoff included.
+ """
+
+ should_retry: RetryPredicate = never
+ attempts: int = DEFAULT_ATTEMPTS
+ timeout: float | None = DEFAULT_TIMEOUT_SECONDS
+ wait_initial: float = DEFAULT_WAIT_INITIAL_SECONDS
+ wait_max: float = DEFAULT_WAIT_MAX_SECONDS
+ wait_jitter: float = DEFAULT_WAIT_JITTER_SECONDS
+
+ def __post_init__(self) -> None:
+ if self.attempts < 1:
+ raise ValueError(f"attempts must be at least 1, got {self.attempts}")
+ if self.timeout is not None and self.timeout <= 0:
+ # stamina reads timeout=0 as "no retries", a confusing way to spell attempts=1.
+ raise ValueError(f"timeout must be positive or None, got {self.timeout}")
+
+ def replace(
+ self,
+ *,
+ should_retry: RetryPredicate | None = None,
+ attempts: int | None = None,
+ timeout: float | None | Unset = UNSET,
+ wait_initial: float | None = None,
+ wait_max: float | None = None,
+ wait_jitter: float | None = None,
+ ) -> RetryPolicy:
+ """A copy with the given fields changed."""
+ return RetryPolicy(
+ should_retry=self.should_retry if should_retry is None else should_retry,
+ attempts=self.attempts if attempts is None else attempts,
+ timeout=self.timeout if isinstance(timeout, Unset) else timeout,
+ wait_initial=self.wait_initial if wait_initial is None else wait_initial,
+ wait_max=self.wait_max if wait_max is None else wait_max,
+ wait_jitter=self.wait_jitter if wait_jitter is None else wait_jitter,
+ )
+
+ def also_on(self, predicate: RetryPredicate) -> RetryPolicy:
+ """A copy that retries what `predicate` accepts as well."""
+ return self.replace(should_retry=any_of(self.should_retry, predicate))
+
+ def unless(self, predicate: RetryPredicate) -> RetryPolicy:
+ """A copy that refuses what `predicate` accepts."""
+ accepted = self.should_retry
+
+ def narrowed(exc: Exception) -> bool:
+ return accepted(exc) and not predicate(exc)
+
+ return self.replace(should_retry=narrowed)
+
+
+# Policy constants. They follow the class because they are instances of it.
+NO_RETRY = RetryPolicy(attempts=1)
+SAFE_RETRY = RetryPolicy(should_retry=any_of(on_transport_error, on_status(*RETRYABLE_SERVER_STATUSES)))
+MUTATION_RETRY = RetryPolicy(should_retry=on_pre_send_transport_error, attempts=MUTATION_ATTEMPTS)
+
+
+@dataclass(frozen=True, slots=True)
+class RetryPolicies:
+ """The defaults a client picks from, one per replay-safety class.
+
+ The verb is only a proxy: idempotence is the real question, so an endpoint that mutates but is
+ idempotent asks for `safe` explicitly.
+ """
+
+ safe: RetryPolicy = SAFE_RETRY
+ mutating: RetryPolicy = MUTATION_RETRY
+
+ def for_method(self, method: str) -> RetryPolicy:
+ """The default for `method`, by whether the verb is replayable."""
+ return self.safe if method.upper() in REPLAYABLE_HTTP_METHODS else self.mutating
+
+
+DEFAULT_RETRY_POLICIES = RetryPolicies()
+
+
+def retry_attempts(policy: RetryPolicy, should_retry: RetryPredicate) -> AsyncIterator[stamina.Attempt]:
+ """Yield one attempt per try of `policy`, retrying what `should_retry` accepts.
+
+ The caller's work goes inside `with attempt:`, which swallows a retryable exception and waits.
+ """
+ return stamina.retry_context(
+ on=should_retry,
+ attempts=policy.attempts,
+ timeout=policy.timeout,
+ wait_initial=policy.wait_initial,
+ wait_max=policy.wait_max,
+ wait_jitter=policy.wait_jitter,
+ )
diff --git a/ddev/src/ddev/utils/github_errors.py b/ddev/src/ddev/utils/github_errors.py
index 09e2c39390bd5..9af5f4d45ee38 100644
--- a/ddev/src/ddev/utils/github_errors.py
+++ b/ddev/src/ddev/utils/github_errors.py
@@ -4,6 +4,7 @@
from __future__ import annotations
import math
+from typing import Self
import httpx
@@ -42,6 +43,28 @@ def github_secondary_rate_limit_wait(response: httpx.Response) -> float | None:
return None
+class GitHubUnexpectedRedirectError(httpx.HTTPStatusError):
+ """A GitHub endpoint answered with a redirect that is not part of its contract.
+
+ The client never follows one, because the `Authorization` header would travel to whatever host
+ `Location` names. Only the artifact download expects a redirect; anywhere else it is surfaced.
+ """
+
+ @classmethod
+ def from_response(cls, method: str, endpoint: str, response: httpx.Response) -> Self:
+ """Build the error for an unexpected redirect returned by *method* *endpoint*.
+
+ Only called for a response that carries a Location.
+ """
+ return cls(
+ f'{method} {endpoint} returned an unexpected redirect (HTTP {response.status_code}) to '
+ f'{response.headers["location"]}. This endpoint is not expected to redirect, so the client '
+ f'did not follow it and the GitHub token was not sent to the target.',
+ request=response.request,
+ response=response,
+ )
+
+
class GitHubAuthenticationError(httpx.HTTPStatusError):
"""A GitHub HTTP failure caused by invalid authentication or insufficient permissions."""
diff --git a/ddev/tests/cli/ci/tests/test_dispatcher_config.py b/ddev/tests/cli/ci/tests/test_dispatcher_config.py
index 5d32e5f65e7b9..57eb5ae4facc7 100644
--- a/ddev/tests/cli/ci/tests/test_dispatcher_config.py
+++ b/ddev/tests/cli/ci/tests/test_dispatcher_config.py
@@ -7,10 +7,16 @@
from collections.abc import Callable
+import httpx
import pytest
from pydantic import ValidationError
-from ddev.cli.ci.tests.dispatcher_config import BatchingConfig, DispatcherConfig
+from ddev.cli.ci.tests.dispatcher_config import (
+ BatchingConfig,
+ DispatcherConfig,
+ GitHubRetryConfig,
+ RetryLimitsConfig,
+)
from ddev.cli.ci.tests.rate_limiting import RateLimiterFactoryConfig
from ddev.repo.config import RepositoryConfig
from ddev.utils.fs import Path
@@ -47,11 +53,19 @@ def test_from_repo_config_reads_full_dispatcher_table(repo_config: RepoConfigBui
[dispatcher.github_rate_limits.slow]
max_rate = 120
+
+ [dispatcher.github_retries.safe]
+ attempts = 5
+
+ [dispatcher.github_retries.mutating]
+ attempts = 1
"""
)
result = DispatcherConfig.from_repo_config(config)
+ assert result.github_retries.safe.attempts == 5
+ assert result.github_retries.mutating.attempts == 1
assert result.batching.max_jobs_per_batch == 120
assert result.batching.allow_integration_splitting is True
assert result.global_timeout_seconds == 3600.0
@@ -127,3 +141,24 @@ def test_batching_rejects_out_of_range_max_jobs_per_batch(repo_config: RepoConfi
with pytest.raises(ValidationError):
DispatcherConfig.from_repo_config(config)
+
+
+def test_configured_retry_limits_reach_the_policies_without_changing_what_is_retried():
+ """The config tunes the ladder; widening it would make a duplicate side effect a setting.
+
+ Catches an `apply_to` that rebuilt the policy from scratch and silently dropped its conditions,
+ which would let a workflow dispatch be replayed after a read timeout.
+ """
+ config = GitHubRetryConfig(
+ safe=RetryLimitsConfig(attempts=7, timeout_seconds=120.0),
+ mutating=RetryLimitsConfig(attempts=1),
+ )
+
+ policies = config.to_policies()
+ request = httpx.Request("GET", "https://api.github.com/x")
+ server_error = httpx.HTTPStatusError("boom", request=request, response=httpx.Response(502, request=request))
+
+ assert (policies.safe.attempts, policies.safe.timeout) == (7, 120.0)
+ assert policies.mutating.attempts == 1
+ assert policies.safe.should_retry(server_error)
+ assert not policies.mutating.should_retry(server_error)
diff --git a/ddev/tests/utils/github_async/conftest.py b/ddev/tests/utils/github_async/conftest.py
new file mode 100644
index 0000000000000..b365b60b420c1
--- /dev/null
+++ b/ddev/tests/utils/github_async/conftest.py
@@ -0,0 +1,17 @@
+# (C) Datadog, Inc. 2026-present
+# All rights reserved
+# Licensed under a 3-clause BSD style license (see LICENSE)
+from __future__ import annotations
+
+import pytest
+
+from tests.helpers.clock import FakeClock, advance_clock_on_sleep
+
+
+@pytest.fixture
+def instant_backoff(monkeypatch: pytest.MonkeyPatch) -> None:
+ """Remove the wait between retries, so a test that exhausts a policy costs no wall-clock time.
+
+ stamina sleeps through `asyncio.sleep`, which this replaces with a fake clock advance.
+ """
+ advance_clock_on_sleep(FakeClock(), monkeypatch)
diff --git a/ddev/tests/utils/github_async/helpers.py b/ddev/tests/utils/github_async/helpers.py
index bb6e6acea17f3..c6e54625de7cd 100644
--- a/ddev/tests/utils/github_async/helpers.py
+++ b/ddev/tests/utils/github_async/helpers.py
@@ -10,7 +10,7 @@
import io
import zipfile
from collections.abc import AsyncIterator, Awaitable, Callable
-from typing import Any
+from typing import Any, Literal
import httpx
from aiolimiter import AsyncLimiter
@@ -109,6 +109,10 @@ class EndpointCase:
id: str
call: Callable[[AsyncGitHubClient], Awaitable[GitHubResponse[Any]]]
ok_response: Callable[[], httpx.Response]
+ # Which retry default the endpoint is expected to use. Stated per endpoint rather than derived
+ # from the verb, because whether a request can be safely replayed is the whole question, and
+ # three of the mutating endpoints can be.
+ default_retry: Literal["safe", "mutating"]
ENDPOINT_CALLS = [
@@ -118,68 +122,84 @@ class EndpointCase:
lambda: json_response(
{"workflow_run_id": 1, "run_url": "https://api.github.com/x", "html_url": "https://github.com/x"}
),
+ default_retry="mutating",
),
EndpointCase(
- "get_workflow_run", lambda c: c.get_workflow_run("o", "r", 42), lambda: json_response(workflow_run_payload())
+ "get_workflow_run",
+ lambda c: c.get_workflow_run("o", "r", 42),
+ lambda: json_response(workflow_run_payload()),
+ default_retry="safe",
),
EndpointCase(
"list_workflow_run_artifacts",
lambda c: first_page(c.list_workflow_run_artifacts("o", "r", 1)),
lambda: json_response({"total_count": 1, "artifacts": [artifact(1)]}),
+ default_retry="safe",
),
EndpointCase(
"list_workflow_jobs",
lambda c: first_page(c.list_workflow_jobs("o", "r", 42)),
lambda: json_response({"total_count": 1, "jobs": [workflow_job(1)]}),
+ default_retry="safe",
),
EndpointCase(
"create_issue_comment",
lambda c: c.create_issue_comment("o", "r", 1, "body"),
lambda: json_response(issue_comment_payload()),
+ default_retry="mutating",
),
EndpointCase(
"update_issue_comment",
lambda c: c.update_issue_comment("o", "r", 1, "body"),
lambda: json_response(issue_comment_payload()),
+ default_retry="safe",
),
EndpointCase(
"list_issue_comments",
lambda c: first_page(c.list_issue_comments("o", "r", 1)),
lambda: json_response([issue_comment_payload()]),
+ default_retry="safe",
),
EndpointCase(
"get_pull_request",
lambda c: c.get_pull_request("o", "r", 5),
lambda: json_response(full_pull_request_payload(number=5)),
+ default_retry="safe",
),
EndpointCase(
"list_pull_requests",
lambda c: c.list_pull_requests("o", "r"),
lambda: json_response([pull_request_payload(number=1)]),
+ default_retry="safe",
),
EndpointCase(
"create_pull_request",
lambda c: c.create_pull_request("o", "r", "t", "h", "b"),
lambda: json_response(pull_request_payload(number=1), status_code=201),
+ default_retry="mutating",
),
EndpointCase(
"add_labels_to_issue",
lambda c: c.add_labels_to_issue("o", "r", 1, ["bug"]),
lambda: json_response([{"id": 1, "name": "bug"}]),
+ default_retry="safe",
),
EndpointCase(
"create_pr_review_comment",
lambda c: c.create_pr_review_comment("o", "r", 1, "body", "sha", "path", position=1),
lambda: json_response(pr_review_comment_payload()),
+ default_retry="mutating",
),
EndpointCase(
"create_check_run",
lambda c: c.create_check_run("o", "r", "ck", "abc", "in_progress"),
lambda: json_response(check_run_payload()),
+ default_retry="mutating",
),
EndpointCase(
"update_check_run",
lambda c: c.update_check_run("o", "r", 77, status="in_progress"),
lambda: json_response(check_run_payload(id=77)),
+ default_retry="safe",
),
]
diff --git a/ddev/tests/utils/github_async/test_client_core.py b/ddev/tests/utils/github_async/test_client_core.py
index cba077dfc9a23..79a60ad2cd6b7 100644
--- a/ddev/tests/utils/github_async/test_client_core.py
+++ b/ddev/tests/utils/github_async/test_client_core.py
@@ -9,6 +9,8 @@
import pytest
from ddev.utils.github_async import GITHUB_API_VERSION, AsyncGitHubClient, PaginationData, async_github_client
+from ddev.utils.github_async.client import QUERY_MASK, failure_reason, with_query_masked
+from ddev.utils.github_async.retry import NO_RETRY
from tests.utils.github_async.helpers import TOKEN, json_response, make_client
from tests.utils.github_async.payloads import artifact, workflow_run_payload
@@ -122,3 +124,67 @@ def handler(request: httpx.Request) -> httpx.Response:
assert len(pages) == 2
assert pages[0].data.artifacts[0].id == 1
assert pages[1].data.artifacts[0].id == 2
+
+
+@pytest.mark.parametrize(
+ ("url", "expected"),
+ [
+ pytest.param(
+ "https://productionresultssa.blob.core.windows.net/zip?se=2026-08-25T15%3A00%3A00Z&sig=abc%2F1%3D&sp=r",
+ f"https://productionresultssa.blob.core.windows.net/zip?{QUERY_MASK}",
+ id="azure-blob",
+ ),
+ pytest.param(
+ "https://s3.amazonaws.com/zip?X-Amz-Signature=deadbeef&X-Amz-Security-Token=tok",
+ f"https://s3.amazonaws.com/zip?{QUERY_MASK}",
+ id="s3",
+ ),
+ pytest.param("https://api.github.com/repos/o/r", "https://api.github.com/repos/o/r", id="no-query"),
+ ],
+)
+def test_a_signed_url_keeps_nothing_of_its_query(url: str, expected: str) -> None:
+ """Every parameter of a signed URL exists to sign it, so none of it is safe to keep.
+
+ Which one holds the signature depends on the storage host, and keeping any of them means deciding
+ that correctly for a host we have not seen yet.
+ """
+ assert with_query_masked(url) == expected
+
+
+def test_a_failed_status_is_reported_without_httpx_quoting_the_url() -> None:
+ """httpx builds a status error's message around the full URL, so we build our own from the status.
+
+ Rewriting that message instead would leave the signature one encoding change away from the log.
+ """
+ request = httpx.Request("GET", "https://blob.example/zip?sig=secret")
+
+ reason = failure_reason(httpx.HTTPStatusError("", request=request, response=httpx.Response(403, request=request)))
+
+ assert reason == "HTTP 403 Forbidden"
+
+
+def test_a_transport_failure_keeps_the_reason_the_os_gave() -> None:
+ """A transport error names why the connection failed, which is the whole of its value."""
+ assert failure_reason(httpx.ConnectError("[Errno 61] Connection refused")) == "[Errno 61] Connection refused"
+
+
+def test_a_transport_failure_that_quotes_a_url_still_loses_the_query() -> None:
+ """httpx keeps the URL on `.request` rather than in the message, but that is not ours to rely on."""
+ error = httpx.ConnectError("connection to https://signed.example/zip?sig=secret refused")
+
+ assert failure_reason(error) == f"connection to https://signed.example/zip?{QUERY_MASK}"
+
+
+async def test_a_transport_failure_still_carries_the_request_it_failed_on() -> None:
+ """The client adds context to a transport failure without discarding what httpx attached.
+
+ A caller reaching for `exc.request` after a dropped connection would otherwise get
+ `RuntimeError: The .request property has not been set` instead of the request.
+ """
+ client = make_client(httpx.MockTransport(lambda request: (_ for _ in ()).throw(httpx.ConnectError("refused"))))
+
+ with pytest.raises(httpx.ConnectError) as exc_info:
+ await client.get_workflow_run("o", "r", 42, retry=NO_RETRY)
+
+ assert exc_info.value.request.url.path == "/repos/o/r/actions/runs/42"
+ assert "GET /repos/o/r/actions/runs/42" in str(exc_info.value)
diff --git a/ddev/tests/utils/github_async/test_download_artifact.py b/ddev/tests/utils/github_async/test_download_artifact.py
index c190ba1f7ddb0..9e8a1fb08e26b 100644
--- a/ddev/tests/utils/github_async/test_download_artifact.py
+++ b/ddev/tests/utils/github_async/test_download_artifact.py
@@ -2,6 +2,9 @@
from __future__ import annotations
+import logging
+import traceback
+
import httpx
import pytest
@@ -9,6 +12,8 @@
from ddev.utils.github_errors import GitHubAuthenticationError
from tests.utils.github_async.helpers import TOKEN, make_client, make_zip, patch_signed_download
+pytestmark = pytest.mark.usefixtures("instant_backoff")
+
async def test_download_artifact_token_not_leaked_to_redirect_target(monkeypatch, tmp_path) -> None:
captured_signed_headers: dict[str, str] = {}
@@ -50,7 +55,7 @@ async def test_download_artifact_authentication_error_remains_actionable(tmp_pat
async def test_download_artifact_signed_url_error_propagates(
monkeypatch: pytest.MonkeyPatch, tmp_path, status_code: int
) -> None:
- """A failed signed-URL download propagates as httpx.HTTPStatusError (no retries)."""
+ """A signed-URL download that keeps failing reaches the caller once the retries are spent."""
def github_handler(request: httpx.Request) -> httpx.Response:
return httpx.Response(302, headers={"location": "https://signed.example/zip"})
@@ -96,3 +101,124 @@ def signed_handler(request: httpx.Request) -> httpx.Response:
# Nothing was extracted before the guard fired.
assert list(dest.rglob("*")) == []
+
+
+async def test_an_expired_signed_url_is_resolved_again_rather_than_refetched(monkeypatch, tmp_path) -> None:
+ """The signed URL is short-lived, so the retry has to start from the redirect.
+
+ Retrying only the download would refetch the same dead URL and fail identically.
+ """
+ signed_urls = ["https://signed.example/expired", "https://signed.example/fresh"]
+ github_calls: list[httpx.Request] = []
+ signed_calls: list[str] = []
+
+ def github_handler(request: httpx.Request) -> httpx.Response:
+ github_calls.append(request)
+ return httpx.Response(302, headers={"location": signed_urls[min(len(github_calls) - 1, 1)]})
+
+ def signed_handler(request: httpx.Request) -> httpx.Response:
+ url = str(request.url)
+ signed_calls.append(url)
+ if url.endswith("/expired"):
+ return httpx.Response(403, content=b"AccessDenied")
+ return httpx.Response(200, content=make_zip({"hello.txt": b"hi"}))
+
+ patch_signed_download(monkeypatch, signed_handler)
+ client = AsyncGitHubClient(token=TOKEN, transport=httpx.MockTransport(github_handler))
+
+ await client.download_artifact("/repos/o/r/actions/artifacts/1/zip", tmp_path / "out")
+
+ assert len(github_calls) == 2
+ assert signed_calls == ["https://signed.example/expired", "https://signed.example/fresh"]
+ assert (tmp_path / "out" / "hello.txt").read_bytes() == b"hi"
+
+
+async def test_a_denial_from_github_itself_is_not_retried_as_an_expired_url(tmp_path) -> None:
+ """The 403 the artifact policy retries is the storage host's, not GitHub's.
+
+ GitHub answers a real permission problem with its own 403, which arrives as an authentication
+ error; retrying that would spend the whole ladder on a failure no wait can fix.
+ """
+ calls: list[httpx.Request] = []
+
+ def github_handler(request: httpx.Request) -> httpx.Response:
+ calls.append(request)
+ return httpx.Response(403)
+
+ client = AsyncGitHubClient(token=TOKEN, transport=httpx.MockTransport(github_handler))
+
+ with pytest.raises(GitHubAuthenticationError):
+ await client.download_artifact("/repos/o/r/actions/artifacts/1/zip", tmp_path / "out")
+
+ assert len(calls) == 1
+
+
+# A signed URL keeps its signature in the query string. Both cases below assume the worst about what
+# a failure quotes: a status whose httpx message is built from the full URL, and a transport error
+# whose message happens to contain it. Neither is ours to control, so neither is relied upon.
+SIGNATURE = "b1acc0dedb1acc0de"
+SIGNED_URL = f"https://signed.example/zip?X-Amz-Expires=900&X-Amz-Signature={SIGNATURE}"
+
+SIGNED_DOWNLOAD_FAILURES = [
+ pytest.param(httpx.Response(403, content=b"AccessDenied"), id="expired-signature"),
+ pytest.param(httpx.ConnectError(f"connection to {SIGNED_URL} refused"), id="transport-error"),
+]
+
+
+def _signed_download(failure: httpx.Response | Exception, *, fail_every_attempt: bool):
+ """GitHub handler and signed handler where the signed download fails at least once."""
+ github_calls: list[httpx.Request] = []
+
+ def github_handler(request: httpx.Request) -> httpx.Response:
+ github_calls.append(request)
+ return httpx.Response(302, headers={"location": SIGNED_URL})
+
+ def signed_handler(request: httpx.Request) -> httpx.Response:
+ if fail_every_attempt or len(github_calls) == 1:
+ if isinstance(failure, Exception):
+ raise failure
+ return failure
+ return httpx.Response(200, content=make_zip({"hello.txt": b"hi"}))
+
+ return github_handler, signed_handler, github_calls
+
+
+@pytest.mark.parametrize("failure", SIGNED_DOWNLOAD_FAILURES)
+async def test_the_signed_url_credentials_never_reach_the_log(monkeypatch, tmp_path, caplog, failure) -> None:
+ """A retry of the signed download must not write usable artifact credentials into CI logs.
+
+ The retried exception reaches this client's log line and stamina's retry hook, both of which
+ render it, and CI logs outlive the signature's validity.
+ """
+ github_handler, signed_handler, github_calls = _signed_download(failure, fail_every_attempt=False)
+ patch_signed_download(monkeypatch, signed_handler)
+ client = AsyncGitHubClient(
+ token=TOKEN, transport=httpx.MockTransport(github_handler), logger=logging.getLogger("test-client")
+ )
+
+ with caplog.at_level(logging.WARNING):
+ await client.download_artifact("/repos/o/r/actions/artifacts/1/zip", tmp_path / "out")
+
+ # The failure was retried, so there is a retry to have logged something.
+ assert len(github_calls) == 2
+ assert caplog.records
+ # Messages and every structured field, since the signature can hide in either.
+ logged = "\n".join(f"{record.getMessage()} {record.__dict__}" for record in caplog.records)
+ assert SIGNATURE not in logged
+
+
+@pytest.mark.parametrize("failure", SIGNED_DOWNLOAD_FAILURES)
+async def test_the_signed_url_credentials_never_reach_the_error_that_escapes(monkeypatch, tmp_path, failure) -> None:
+ """Once the retries are spent the failure reaches the caller, whose handler may log it.
+
+ Python prints a chained cause in full, so the chain has to be as clean as the message.
+ """
+ github_handler, signed_handler, _ = _signed_download(failure, fail_every_attempt=True)
+ patch_signed_download(monkeypatch, signed_handler)
+ client = AsyncGitHubClient(token=TOKEN, transport=httpx.MockTransport(github_handler))
+
+ with pytest.raises(httpx.HTTPError) as exc_info:
+ await client.download_artifact("/repos/o/r/actions/artifacts/1/zip", tmp_path / "out")
+
+ reported = "".join(traceback.format_exception(exc_info.value))
+ assert SIGNATURE not in reported
diff --git a/ddev/tests/utils/github_async/test_rate_limiting.py b/ddev/tests/utils/github_async/test_rate_limiting.py
index ddffc121e16e5..7e7f5b41d1880 100644
--- a/ddev/tests/utils/github_async/test_rate_limiting.py
+++ b/ddev/tests/utils/github_async/test_rate_limiting.py
@@ -113,7 +113,7 @@ async def test_default_rate_limiter_is_constructed_and_observes_403() -> None:
assert governor is not None
with pytest.raises(httpx.HTTPStatusError):
- await client._request("GET", "/x")
+ await client._rate_limited_request("GET", "/x")
# The 403's retry-after was observed (before raise_for_status), arming the shared pause;
# exact pause arithmetic is covered by the clocked governor tests.
@@ -129,7 +129,7 @@ async def test_retry_on_secondary_limit_returns_success(monkeypatch: pytest.Monk
transport, calls = recording_transport([httpx.Response(403, headers={"retry-after": "5"}), httpx.Response(200)])
client = governed_client(clock, transport, on_event=events.append)
- response = await client._request("GET", "/x")
+ response = await client._rate_limited_request("GET", "/x")
assert response.status_code == 200
assert len(calls) == 2
@@ -160,7 +160,7 @@ async def test_retry_on_secondary_limit_without_valid_wait_returns_success(
transport, calls = recording_transport([rate_limited_response, httpx.Response(200)])
client = governed_client(clock, transport, on_event=events.append)
- response = await client._request("GET", "/x")
+ response = await client._rate_limited_request("GET", "/x")
assert response.status_code == 200
assert len(calls) == 2
@@ -184,7 +184,7 @@ async def test_retry_on_primary_exhaustion_waits_until_reset(monkeypatch: pytest
)
client = governed_client(clock, transport, on_event=events.append)
- response = await client._request("GET", "/x")
+ response = await client._rate_limited_request("GET", "/x")
assert response.status_code == 200
assert len(calls) == 2
@@ -200,20 +200,24 @@ async def test_authentication_error_is_actionable_and_not_retried(status_code: i
client = AsyncGitHubClient(token=TOKEN, transport=transport)
with pytest.raises(GitHubAuthenticationError) as exc_info:
- await client._request("GET", "/x")
+ await client._rate_limited_request("GET", "/x")
assert len(calls) == 1
assert exc_info.value.response.status_code == status_code
assert "ddev config set github.token" in str(exc_info.value)
-async def test_no_retry_on_transport_error() -> None:
- """A transport error is never retried (the action may have executed); it propagates immediately."""
+async def test_the_rate_limit_layer_does_not_retry_a_transport_error() -> None:
+ """A transport error is not a rate-limit signal, so this layer must leave it alone.
+
+ Retrying belongs to the retry strategy, which decides by whether the request can be replayed;
+ treating one as a rate-limit event here would retry it for every endpoint, dispatches included.
+ """
transport, calls = recording_transport([httpx.ConnectError("boom")])
client = AsyncGitHubClient(token=TOKEN, transport=transport)
with pytest.raises(httpx.ConnectError):
- await client._request("GET", "/x")
+ await client._rate_limited_request("GET", "/x")
assert len(calls) == 1
@@ -228,7 +232,7 @@ async def test_retries_exhausted_raises_after_max(monkeypatch: pytest.MonkeyPatc
client = governed_client(clock, transport, max_rate_limit_retries=1)
with pytest.raises(httpx.HTTPStatusError) as exc_info:
- await client._request("GET", "/x")
+ await client._rate_limited_request("GET", "/x")
assert len(calls) == 2
assert type(exc_info.value) is httpx.HTTPStatusError
diff --git a/ddev/tests/utils/github_async/test_retry.py b/ddev/tests/utils/github_async/test_retry.py
new file mode 100644
index 0000000000000..80961a5cd07a9
--- /dev/null
+++ b/ddev/tests/utils/github_async/test_retry.py
@@ -0,0 +1,325 @@
+# (C) Datadog, Inc. 2026-present
+# All rights reserved
+# Licensed under a 3-clause BSD style license (see LICENSE)
+"""Tests for retrying the failures that are not rate limiting.
+
+Which failures an endpoint replays, and why that matters: replaying the wrong one duplicates a side
+effect. Rate-limit retries are a different layer, covered in `test_rate_limiting.py`.
+"""
+
+from __future__ import annotations
+
+import logging
+
+import httpx
+import pytest
+
+from ddev.utils.github_async import AsyncGitHubClient
+from ddev.utils.github_async.retry import (
+ DEFAULT_ATTEMPTS,
+ MUTATION_RETRY,
+ NO_RETRY,
+ SAFE_RETRY,
+ RetryPolicies,
+ RetryPolicy,
+ on_status,
+ on_transport_error,
+)
+from ddev.utils.github_errors import GitHubUnexpectedRedirectError
+from tests.utils.github_async.helpers import ENDPOINT_CALLS, TOKEN, json_response, recording_transport
+from tests.utils.github_async.payloads import (
+ full_pull_request_payload,
+ issue_comment_payload,
+ workflow_job,
+ workflow_run_payload,
+)
+
+pytestmark = pytest.mark.usefixtures("instant_backoff")
+
+# A policy that would retry anything, to prove the client's exclusions win regardless.
+RETRY_EVERYTHING = RetryPolicy(should_retry=lambda exc: True, attempts=3)
+
+
+def policy_for(kind: str) -> RetryPolicy:
+ return SAFE_RETRY if kind == "safe" else MUTATION_RETRY
+
+
+# ---------------------------------------------------------------------------
+# Per-endpoint defaults
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.parametrize("case", ENDPOINT_CALLS, ids=lambda case: case.id)
+async def test_a_server_error_is_replayed_only_where_replaying_is_safe(case) -> None:
+ """A 503 says nothing about whether the request landed, so only replayable endpoints try again.
+
+ Catches the mistake that matters most here: widening the default of a create endpoint, where a
+ replay leaves a duplicate workflow run, comment or check run behind.
+ """
+ transport, calls = recording_transport([httpx.Response(503), case.ok_response()])
+ client = AsyncGitHubClient(token=TOKEN, transport=transport)
+
+ if case.default_retry == "safe":
+ await case.call(client)
+ assert len(calls) == 2
+ else:
+ with pytest.raises(httpx.HTTPStatusError):
+ await case.call(client)
+ assert len(calls) == 1
+
+
+@pytest.mark.parametrize("case", ENDPOINT_CALLS, ids=lambda case: case.id)
+async def test_a_request_that_never_left_is_replayed_by_every_endpoint(case) -> None:
+ """A refused connection proves GitHub never saw the request, so even a create can safely repeat.
+
+ Without this, a mutating endpoint would give up on a blip that cost it nothing.
+ """
+ transport, calls = recording_transport([httpx.ConnectError("refused")])
+ client = AsyncGitHubClient(token=TOKEN, transport=transport)
+
+ with pytest.raises(httpx.ConnectError):
+ await case.call(client)
+
+ assert len(calls) == policy_for(case.default_retry).attempts
+
+
+@pytest.mark.parametrize(
+ "error",
+ [
+ pytest.param(httpx.ReadTimeout("timed out"), id="read_timeout"),
+ pytest.param(httpx.RemoteProtocolError("disconnected"), id="server_disconnected"),
+ ],
+)
+async def test_a_mutation_does_not_replay_a_request_that_may_have_landed(error: Exception) -> None:
+ """Once the request is on the wire, a failure cannot tell us whether GitHub acted on it.
+
+ A dispatch replayed in that state starts a second batch of test jobs, so this is the case where
+ giving up is the cheaper mistake.
+ """
+ transport, calls = recording_transport([error])
+ client = AsyncGitHubClient(token=TOKEN, transport=transport)
+
+ with pytest.raises(type(error)):
+ await client.create_workflow_dispatch("o", "r", "wf.yml", "main")
+
+ assert len(calls) == 1
+
+
+# ---------------------------------------------------------------------------
+# Caller overrides
+# ---------------------------------------------------------------------------
+
+
+async def test_a_caller_can_turn_retrying_off_for_one_call() -> None:
+ transport, calls = recording_transport([httpx.Response(503), json_response(workflow_run_payload())])
+ client = AsyncGitHubClient(token=TOKEN, transport=transport)
+
+ with pytest.raises(httpx.HTTPStatusError):
+ await client.get_workflow_run("o", "r", 42, retry=NO_RETRY)
+
+ assert len(calls) == 1
+
+
+async def test_a_caller_can_widen_a_mutation_that_it_knows_is_safe_to_repeat() -> None:
+ """The default is deliberately cautious, so a caller that can absorb a duplicate may opt in."""
+ transport, calls = recording_transport([httpx.Response(503), json_response(issue_comment_payload())])
+ client = AsyncGitHubClient(token=TOKEN, transport=transport)
+
+ await client.create_issue_comment("o", "r", 1, "body", retry=SAFE_RETRY)
+
+ assert len(calls) == 2
+
+
+async def test_the_client_defaults_can_be_replaced_wholesale() -> None:
+ """The limits are configurable, so what the constructor is given has to reach the requests."""
+ transport, calls = recording_transport([httpx.ConnectError("refused")])
+ policies = RetryPolicies(safe=SAFE_RETRY.replace(attempts=5), mutating=MUTATION_RETRY)
+ client = AsyncGitHubClient(token=TOKEN, transport=transport, retry_policies=policies)
+
+ with pytest.raises(httpx.ConnectError):
+ await client.get_workflow_run("o", "r", 42)
+
+ assert len(calls) == 5
+
+
+# ---------------------------------------------------------------------------
+# What no policy may retry
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.parametrize(
+ "response",
+ [
+ pytest.param(httpx.Response(401), id="unauthenticated"),
+ pytest.param(httpx.Response(403), id="permission_denied"),
+ pytest.param(httpx.Response(403, headers={"retry-after": "5", "x-ratelimit-remaining": "0"}), id="rate_limit"),
+ pytest.param(httpx.Response(302, headers={"location": "https://elsewhere.example"}), id="redirect"),
+ ],
+)
+async def test_the_client_refuses_to_replay_what_replaying_cannot_fix(response: httpx.Response) -> None:
+ """Even asked to retry everything, these stay single attempts.
+
+ Each would only reach the same outcome more slowly: bad credentials, a pause the limiter already
+ owns, or a redirect, which is an answer.
+ """
+ transport, calls = recording_transport([response])
+ client = AsyncGitHubClient(token=TOKEN, transport=transport, max_rate_limit_retries=0)
+
+ with pytest.raises(httpx.HTTPStatusError):
+ await client.get_workflow_run("o", "r", 42, retry=RETRY_EVERYTHING)
+
+ assert len(calls) == 1
+
+
+async def test_a_missing_resource_is_an_answer_rather_than_a_failure_to_retry() -> None:
+ """`get_pull_request` returning 404 means there is no pull request for that number.
+
+ Dispatcher falls through to commit resolution on that answer, so retrying only delays it. Out of
+ the defaults rather than banned, since a caller awaiting a fresh resource may opt in.
+ """
+ transport, calls = recording_transport(
+ [httpx.Response(404), httpx.Response(404), json_response(full_pull_request_payload(number=5))]
+ )
+ client = AsyncGitHubClient(token=TOKEN, transport=transport)
+
+ with pytest.raises(httpx.HTTPStatusError):
+ await client.get_pull_request("o", "r", 5)
+
+ assert len(calls) == 1
+
+ # The same 404, with a caller that is waiting for the resource to appear: retried, then found.
+ await client.get_pull_request("o", "r", 5, retry=SAFE_RETRY.also_on(on_status(404)))
+ assert len(calls) == 3
+
+
+# ---------------------------------------------------------------------------
+# Redirects
+# ---------------------------------------------------------------------------
+
+
+async def test_an_unexpected_redirect_names_the_endpoint_and_is_not_followed() -> None:
+ """Following a redirect would send the token to whoever the Location names.
+
+ The client never follows one, so the risk is a caller misreading the generic HTTP error that used
+ to surface and "fixing" it by enabling redirects.
+ """
+ transport, calls = recording_transport([httpx.Response(302, headers={"location": "https://evil.example/steal"})])
+ client = AsyncGitHubClient(token=TOKEN, transport=transport)
+
+ with pytest.raises(GitHubUnexpectedRedirectError) as exc_info:
+ await client.get_workflow_run("o", "r", 42)
+
+ message = str(exc_info.value)
+ assert "/repos/o/r/actions/runs/42" in message
+ assert "https://evil.example/steal" in message
+ assert len(calls) == 1
+ assert calls[0].url.host == "api.github.com"
+
+
+async def test_a_not_modified_response_is_reported_for_what_it_is() -> None:
+ """304 sits in the 3xx range but carries no Location, so it is an answer, not a redirect.
+
+ Reporting it as a redirect means reaching for a Location header that a 304 never has, which
+ raises `KeyError` and buries the status the server actually sent.
+ """
+ transport, _ = recording_transport([httpx.Response(304)])
+ client = AsyncGitHubClient(token=TOKEN, transport=transport)
+
+ with pytest.raises(httpx.HTTPStatusError) as exc_info:
+ await client.get_workflow_run("o", "r", 42, retry=NO_RETRY)
+
+ assert "304" in str(exc_info.value)
+
+
+# ---------------------------------------------------------------------------
+# Pagination
+# ---------------------------------------------------------------------------
+
+
+async def test_a_failing_page_is_retried_without_refetching_the_pages_already_read() -> None:
+ """Pagination is a sequence of requests, so a blip on page two must not restart page one."""
+ page_one = json_response(
+ {"total_count": 2, "jobs": [workflow_job(1)]},
+ headers={"link": '; rel="next"'},
+ )
+ page_two = json_response({"total_count": 2, "jobs": [workflow_job(2)]})
+ transport, calls = recording_transport([page_one, httpx.Response(503), page_two])
+ client = AsyncGitHubClient(token=TOKEN, transport=transport)
+
+ pages = [page async for page in client.list_workflow_jobs("o", "r", 42)]
+
+ assert len(calls) == 3
+ assert [job.id for page in pages for job in page.data.jobs] == [1, 2]
+
+
+# ---------------------------------------------------------------------------
+# Reporting
+# ---------------------------------------------------------------------------
+
+
+async def test_a_retry_is_reported_with_its_cause(caplog: pytest.LogCaptureFixture) -> None:
+ """A silent retry hides a degraded GitHub behind a slow run, so the cause has to reach the log."""
+ transport, _ = recording_transport([httpx.Response(503), json_response(workflow_run_payload())])
+ logger = logging.getLogger("test-github-client")
+ client = AsyncGitHubClient(token=TOKEN, transport=transport, logger=logger)
+
+ with caplog.at_level(logging.WARNING, logger="test-github-client"):
+ await client.get_workflow_run("o", "r", 42)
+
+ records = [record for record in caplog.records if record.name == "test-github-client"]
+ assert len(records) == 1
+ assert "/repos/o/r/actions/runs/42" in records[0].getMessage()
+ assert "503" in records[0].getMessage()
+ assert records[0].attempt == 2
+
+
+# ---------------------------------------------------------------------------
+# Composing policies
+# ---------------------------------------------------------------------------
+
+
+def test_also_on_keeps_what_the_policy_already_retried() -> None:
+ """The point of composing is starting from a default, so widening must not drop its conditions."""
+ policy = SAFE_RETRY.also_on(on_status(404))
+
+ assert policy.should_retry(_status_error(404))
+ assert policy.should_retry(_status_error(503))
+ assert policy.should_retry(httpx.ConnectError("refused"))
+
+
+def test_unless_removes_a_condition_the_policy_would_otherwise_retry() -> None:
+ policy = SAFE_RETRY.unless(on_status(503))
+
+ assert not policy.should_retry(_status_error(503))
+ assert policy.should_retry(_status_error(502))
+
+
+def test_tuning_a_policy_leaves_the_shared_default_alone() -> None:
+ """The defaults live for the whole process, so tuning one client must not reach another's."""
+ tuned = SAFE_RETRY.replace(attempts=1)
+
+ assert tuned.attempts == 1
+ assert SAFE_RETRY.attempts == DEFAULT_ATTEMPTS
+
+
+@pytest.mark.parametrize(
+ ("limits", "message"),
+ [
+ pytest.param({"attempts": 0}, "attempts must be at least 1", id="no_attempts"),
+ pytest.param({"timeout": 0}, "timeout must be positive", id="zero_timeout"),
+ ],
+)
+def test_a_policy_that_could_never_send_a_request_is_rejected(limits: dict[str, int], message: str) -> None:
+ """Both spellings mean "no attempts at all", which would fail every call without sending it.
+
+ stamina reads `timeout=0` that way rather than as an error, so it has to be caught here.
+ """
+ with pytest.raises(ValueError, match=message):
+ RetryPolicy(should_retry=on_transport_error, **limits)
+
+
+def _status_error(status_code: int) -> httpx.HTTPStatusError:
+ request = httpx.Request("GET", "https://api.github.com/x")
+ return httpx.HTTPStatusError(
+ f"HTTP {status_code}", request=request, response=httpx.Response(status_code, request=request)
+ )