From 6d4183948e1b6b41ae8345d0b3a3eead5f6f5ad6 Mon Sep 17 00:00:00 2001 From: disqualifier Date: Thu, 2 Jul 2026 23:23:24 -0400 Subject: [PATCH] fix: no sleep on exhausted final 429 attempt; correct errors.py doc (v0.1.6) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A 429 retry_after was slept immediately, including on the final attempt that goes on to exhaust max_retries — a pointless wait right before giving up. The wait is now carried in a per-call mutable cell and slept at the start of the next attempt instead, so it's honored before every attempt that actually runs and never after the last one. errors.py's docstring claimed the exported error types cover the missing-[discord]-extra raise path; that raise is a plain RuntimeError, not one of these types. Reworded to match. Compressed essay-length docstrings/comments across sender.py; no behavior change. Verified against the aioproxies twin: aiowebhooks' proxy-key normalization already routes zero-padded ports through urlsplit().port (parses to int, no zero-pad on render), so host:080 and host:80 already collapse to one canonical key — no code change needed there. Signed-off-by: disqualifier --- README.md | 18 +++++-- pyproject.toml | 2 +- src/aiowebhooks/__init__.py | 2 +- src/aiowebhooks/errors.py | 5 +- src/aiowebhooks/sender.py | 104 +++++++++++++++++------------------- 5 files changed, 70 insertions(+), 61 deletions(-) diff --git a/README.md b/README.md index 7d11006..cc6289e 100644 --- a/README.md +++ b/README.md @@ -13,16 +13,16 @@ send to the core — inheriting rotation, proxy, retry, and result for free. ## Install ``` -aiowebhooks @ git+ssh://git@git.rethinkstudios.io/rethink-public/aiowebhooks.git@v0.1.5 +aiowebhooks @ git+ssh://git@git.rethinkstudios.io/rethink-public/aiowebhooks.git@v0.1.6 # discord embeds / identity helpers need the extra: -aiowebhooks[discord] @ git+ssh://git@git.rethinkstudios.io/rethink-public/aiowebhooks.git@v0.1.5 +aiowebhooks[discord] @ git+ssh://git@git.rethinkstudios.io/rethink-public/aiowebhooks.git@v0.1.6 ``` The base pulls `aiohttp` and `commons` (for the retry/backoff engine). Only `aiowebhooks[discord]` adds `discord.py` (>=2.3, mainline — not discord.py-self), and only for `DiscordWebhook`. -Drop the `@v0.1.5` suffix from the line above to install the latest unpinned. +Drop the `@v0.1.6` suffix from the line above to install the latest unpinned. ## Core sender @@ -148,6 +148,18 @@ Without the extra installed, importing `aiowebhooks` still works; constructing o ## Changelog +### v0.1.6 + +- **429 `retry_after` no longer sleeps on an exhausted final attempt:** the wait is + now carried to the START of the next attempt instead of slept immediately after + seeing the 429. Previously the last (exhausted) attempt slept the full + `retry_after` before giving up — a pointless wait since no retry followed. The + wait is still honored (additive with aretry's backoff) before every attempt that + actually runs. +- Docs: `errors.py` no longer implies the missing-`[discord]`-extra raise is one of + the exported error types — it's a plain `RuntimeError`. +- Docstrings/comments compressed; no behavior change. + ### v0.1.5 - **429 `retry_after` bounded:** a non-finite server-controlled wait (`inf`/`nan`, from diff --git a/pyproject.toml b/pyproject.toml index c5df956..7e03bca 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "aiowebhooks" -version = "0.1.5" +version = "0.1.6" description = "async webhook sender (aiohttp) with round-robin urls, retry, and proxy rotation; optional discord.py embeds" requires-python = ">=3.10" dependencies = [ diff --git a/src/aiowebhooks/__init__.py b/src/aiowebhooks/__init__.py index d0cdb0a..4e8adf8 100644 --- a/src/aiowebhooks/__init__.py +++ b/src/aiowebhooks/__init__.py @@ -21,4 +21,4 @@ from .sender import Webhook __all__ = ["Webhook", "WebhookResult", "WebhookError", "NoUrlsError"] -__version__ = "0.1.4" +__version__ = "0.1.6" diff --git a/src/aiowebhooks/errors.py b/src/aiowebhooks/errors.py index 5acb3f4..eff36e0 100644 --- a/src/aiowebhooks/errors.py +++ b/src/aiowebhooks/errors.py @@ -3,8 +3,9 @@ these are surfaced for callers that want to branch on a specific failure cause. note the core `Webhook.send` does NOT raise on a send failure — it returns a `WebhookResult` with `ok=False` and the cause captured in `error`. these types -exist for the few raise paths (bad construction, missing extra) and as a base for -any future raising surface. +cover bad construction (e.g. `NoUrlsError`) and serve as a base for any future +raising surface; the missing-`[discord]`-extra path raises a plain `RuntimeError`, +not one of these. """ diff --git a/src/aiowebhooks/sender.py b/src/aiowebhooks/sender.py index 724c23b..da1f5b2 100644 --- a/src/aiowebhooks/sender.py +++ b/src/aiowebhooks/sender.py @@ -29,12 +29,9 @@ log = logging.getLogger(__name__) MAX_RETRY_AFTER = 300.0 """ceiling (seconds) honored from a server-controlled 429 retry_after/Retry-After -a server can send an arbitrarily large or non-finite wait (float('inf'), a -Cloudflare-scale value, or a ms-vs-s unit mismatch turning 5s into 5000s); sleeping -that verbatim would stall a send() far past its ClientTimeout and outside -max_retries accounting, breaking the 'always returns a WebhookResult' contract. -non-finite values are rejected outright (treated as unparseable); finite values are -clamped to this ceiling. +guards against an arbitrarily large or non-finite wait (inf/nan, or a ms-vs-s unit +mismatch) stalling a send() past its ClientTimeout. non-finite values are rejected +outright; finite values are clamped to this ceiling. """ @@ -111,8 +108,7 @@ class Webhook: """seconds to wait on a 429, from body retry_after then Retry-After header non-finite values (inf/nan) are rejected as unparseable; finite values are - clamped to MAX_RETRY_AFTER so a server-controlled wait can never stall a - send() past a bounded ceiling. + clamped to MAX_RETRY_AFTER. """ if status != 429: return None @@ -159,27 +155,26 @@ class Webhook: """status-retry (via commons.aretry) wrapping proxy rotation; never raises commons.aretry owns the 429/5xx/no-provider-connection-error backoff schedule - + retry cap (max_retries), retrying on the internal _Retryable signal. on - exhaustion it re-raises the last _Retryable, whose carried result is the REAL + + retry cap (max_retries), retrying on the internal _Retryable signal; on + exhaustion it re-raises the last _Retryable, whose carried result is the real last response (not a synthetic status-0). proxy rotation on connection errors - lives inside the attempt and is capped separately (max_proxy_retries); once a - provider's rotation cap is hit that failure is terminal, not retried again - via aretry. + lives inside the attempt, capped separately (max_proxy_retries); once that cap + is hit the failure is terminal, not retried again via aretry. """ counter = [0] + pending_wait: List[Optional[float]] = [None] try: return await aretry( - lambda: self._attempt(session, url, payload, counter), + lambda: self._attempt(session, url, payload, counter, pending_wait), attempts=self.max_retries + 1, on=(_Retryable,), ) except _Retryable as exhausted: return exhausted.result except Exception as error: - # never-raises safety net: an unexpected error that escapes the attempt (a - # closed injected session -> RuntimeError, a malformed proxy url -> ValueError, - # anything not aiohttp.ClientError/TimeoutError) must come back as a failed - # result, not propagate out of send() + # never-raises safety net: anything not aiohttp.ClientError/TimeoutError + # (closed injected session, malformed proxy url, ...) comes back as a + # failed result instead of escaping send() log.warning("webhook send failed unexpectedly on %s: %s", url, error, exc_info=True) return WebhookResult( ok=False, status=None, url=url, attempts=counter[0] or 1, @@ -187,28 +182,38 @@ class Webhook: ) async def _attempt( - self, session: aiohttp.ClientSession, url: str, payload: Dict, counter: List[int] + self, session: aiohttp.ClientSession, url: str, payload: Dict, counter: List[int], + pending_wait: List[Optional[float]], ) -> WebhookResult: """one logical send: proxy rotation + a single POST; may raise _Retryable raises _Retryable (carrying the real response) on a 429/5xx so the caller's - aretry applies backoff; honors an explicit 429 retry_after by sleeping it - before signalling. a connection/timeout error also raises _Retryable when no - proxy provider is set, so it retries under aretry's backoff + max_retries - instead of failing one-shot; with a provider, the same error first drives - burn+rotate up to max_proxy_retries, and only becomes terminal (not retried - via aretry) once that cap is hit. returns a final WebhookResult on success or - a terminal (non-retryable) failure — never lets a provider error escape. + aretry applies backoff. an explicit 429 retry_after is carried over and slept + at the START of the next attempt, never after the one that raises, so an + exhausted final attempt never sleeps a wait it won't use. a connection/timeout + error also raises _Retryable when no proxy provider is set (retries under + aretry like a 5xx); with a provider it instead drives burn+rotate up to + max_proxy_retries and only becomes terminal once that cap is hit. always + returns a final WebhookResult or raises _Retryable — never lets a provider + error escape. - `counter` is a per-call mutable cell ([0]) owned by the calling `_send_loop`, - so the attempt count is local to one `send()` and concurrent sends on the - same instance don't corrupt each other's tally. + `counter`/`pending_wait` are per-call mutable cells ([0] / [None]) owned by + `_send_loop` and threaded through because aretry calls `_attempt` fresh on + every top-level retry, so a plain local wouldn't survive across calls. + `counter` tallies attempts; `pending_wait` carries a 429 retry_after to the + next attempt's sleep. keeps both local to one `send()`, safe for concurrent + sends on the same instance. """ timeout = aiohttp.ClientTimeout(total=self.timeout) last_proxy: Optional[str] = None proxy_tries = 0 while True: + if pending_wait[0] is not None: + wait = pending_wait[0] + pending_wait[0] = None + log.warning("webhook 429 on %s; honoring retry_after %.3fs", url, wait) + await asyncio.sleep(wait) counter[0] += 1 attempts = counter[0] proxy_url = None @@ -216,8 +221,7 @@ class Webhook: try: proxy_dict = self._proxies.get() except Exception: - # duck-typed provider; any error from get() means no proxy is - # available — fail cleanly rather than escaping send(). + # duck-typed provider: any get() error means no proxy available log.warning("webhook: proxy get() failed; no proxy available", exc_info=True) return WebhookResult( @@ -246,19 +250,16 @@ class Webhook: ) if status == 429: - # every 429 is retryable; honor an explicit retry_after by - # sleeping it, but a 429 with no parseable wait (edge/Cloudflare/ - # generic webhook) still retries under aretry's backoff + cap. - # note: aretry ALSO sleeps its backoff between retries, so an - # honored retry_after is additive (retry_after + backoff) — this - # only ever over-waits, never under-waits the server's hint. - # non-finite values (inf/nan) are rejected and finite values are - # clamped to MAX_RETRY_AFTER by _retry_after, so a server-controlled - # wait can never stall this attempt past a bounded ceiling + # every 429 is retryable; an explicit retry_after is carried + # to the NEXT attempt's sleep (never slept here), so an + # exhausted final attempt never sleeps a wait it won't use. + # no parseable wait still retries under aretry's backoff+cap; + # an honored wait is additive with that backoff (over-waits, + # never under-waits). _retry_after rejects non-finite values + # and clamps finite ones to MAX_RETRY_AFTER. wait = self._retry_after(status, resp.headers, body) if wait is not None: - log.warning("webhook 429 on %s; honoring retry_after %.3fs", url, wait) - await asyncio.sleep(wait) + pending_wait[0] = wait else: log.warning("webhook 429 on %s; no retry_after, backing off", url) raise _Retryable(result) @@ -280,13 +281,10 @@ class Webhook: ok=False, status=None, url=url, attempts=attempts, error=f"{type(error).__name__}: {error}", proxy=last_proxy, ) - # no proxy provider: a connection/timeout error is the single most - # retry-worthy failure class (the 5xx equivalent already retries under - # aretry) — raise _Retryable so aretry applies backoff + max_retries - # instead of failing one-shot. aretry re-raises this same _Retryable on - # exhaustion, and _send_loop's `except _Retryable` returns its carried - # result, so the never-raises / always-returns-a-WebhookResult contract - # still holds after retries run out. + # no proxy provider: retry a connection/timeout error like a 5xx, + # under aretry's backoff + max_retries instead of failing one-shot; + # _send_loop's `except _Retryable` returns the carried result once + # exhausted, so never-raises still holds. raise _Retryable(WebhookResult( ok=False, status=None, url=url, attempts=attempts, error=f"{type(error).__name__}: {error}", proxy=last_proxy, @@ -295,11 +293,9 @@ class Webhook: def _burn(self, proxy: Optional[str]) -> bool: """burn the current proxy; return False if it can't be rotated - the provider is duck-typed and never imported, so we cannot catch its - exception types by class. ANY exception from burn (a ProxiesExhaustedError - on a dead pool, a ValueError when the proxy isn't in the pool, etc.) means - we can't rotate — return False so the caller ends the loop with a failed - result rather than letting it escape send() (which must never raise). + the provider is duck-typed and never imported, so any exception from burn + (dead pool, unknown proxy, ...) is caught broadly and means "can't rotate" — + return False rather than let it escape send(). """ try: self._proxies.burn(proxy)