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)