From 826aa2fbfe4d67b26fdce7cb0035b5223c5d3bba Mon Sep 17 00:00:00 2001 From: Matt Miller Date: Fri, 9 Oct 2026 22:36:43 +0000 Subject: [PATCH 1/2] docs(errors): list rate_limited explicitly as the fourth code both surfaces share A v2 envelope rate_limited already reached router_exceptions.RateLimited through _class_for's fall-through to _BY_ERROR_TYPE. Name it in _BY_CODE beside unauthorized/forbidden/insufficient_credits, re-export it from comfy_sdk.exceptions, and update the README and docstrings that said three. No change to which class is raised. --- CHANGELOG.md | 7 +++++++ README.md | 16 ++++++++++------ src/comfy_sdk/exceptions.py | 20 ++++++++++++++------ src/comfy_sdk/router_exceptions.py | 24 +++++++++++++++--------- tests/test_error_mapping.py | 25 ++++++++++++++++++++++--- 5 files changed, 68 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ea73f68..c595ca9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -219,6 +219,13 @@ the fuller account of each version, including verification notes. body's `error_type`), or `None` when it named none — which is also how the SDK tells which surface answered. +### Changed + +- `rate_limited` is now listed explicitly in `comfy_sdk.exceptions._BY_CODE` beside the three + other codes both surfaces share; a v2 jobs/assets throttle raised + `comfy_sdk.router_exceptions.RateLimited` before and still does. README and docstrings now say + so. `RateLimited` is also re-exported from `comfy_sdk.exceptions`, as the other three are. + ## [0.3.0] - 2026-09-14 ### Added diff --git a/README.md b/README.md index 9c57e90..eb0d539 100644 --- a/README.md +++ b/README.md @@ -1101,8 +1101,9 @@ bucket newer than your installed version — Router names the bucket on every error it sends, and that is what types the exception. A response that never reached Router carries no bucket to read (an intermediary's HTML `404`, a bare `503 no healthy upstream`), so it arrives as a plain `ComfyError` with the -status on `.http_status`; keep an `except ComfyError` outside the clause above -if you need to handle those in the same place. +status on `.http_status` — unless it is a v2 envelope naming one of the four +codes both surfaces share (below), which raises that shared class instead; +keep an `except ComfyError` outside the clause above if you need to handle those in the same place. `RouterError` is exported from the package root because it is the handler most callers write first. The nineteen per-bucket classes stay in @@ -1115,14 +1116,17 @@ callers write first. The nineteen per-bucket classes stay in for the whole set rather than half of it here and half of it there. A bucket added to Router after your installed version arrives as `RouterError` itself, with the raw value readable on `.error_type`. -Three of those names — `Unauthorized`, `Forbidden`, `InsufficientCredits` — -are also exported by `comfy_sdk` and `comfy_sdk.exceptions`. **They are the same +Four of those names — `Unauthorized`, `Forbidden`, `InsufficientCredits`, +`RateLimited` — are also exported by `comfy_sdk.exceptions` (and the first three +by `comfy_sdk` too). **They are the same class**, re-exported, not a second one wearing the same name, so `except InsufficientCredits` catches the refusal whichever import you wrote. The consequence worth knowing is the other direction: because one class cannot descend from `RouterError` on one surface and not on the other, a *workflow* -call that fails `401`/`403`/`402` raises a `RouterError` subclass too, so -`except RouterError` is slightly wider than its name for exactly those three. +call that fails `401`/`403`/`402` raises a `RouterError` subclass too, and so +does a jobs/assets call throttled with `429 rate_limited`, which raises +`RateLimited` — so `except RouterError` is slightly wider than its name for +exactly those four. A `cancel()` the server declines raises `AlreadyCompleted` when the request had already finished — there was nothing left to stop, and the result is still diff --git a/src/comfy_sdk/exceptions.py b/src/comfy_sdk/exceptions.py index 5656bf7..e69d663 100644 --- a/src/comfy_sdk/exceptions.py +++ b/src/comfy_sdk/exceptions.py @@ -44,6 +44,7 @@ CancelRefused, Forbidden, InsufficientCredits, + RateLimited, RouterError, Unauthorized, _detail_from, @@ -116,14 +117,19 @@ def __init__(self, message: str, *, error: JobError | None = None) -> None: #: :data:`comfy_sdk.router_exceptions._BY_ERROR_TYPE` instead, so that table #: stays the single copy of the contract's closed set. #: -#: Three entries -- ``insufficient_credits``, ``unauthorized``, ``forbidden`` -- -#: name classes that live in :mod:`comfy_sdk.router_exceptions` and descend from +#: Four entries -- ``insufficient_credits``, ``unauthorized``, ``forbidden``, +#: ``rate_limited`` -- name classes that live in +#: :mod:`comfy_sdk.router_exceptions` and descend from #: :class:`~comfy_sdk.router_exceptions.RouterError`. That is deliberate and it #: is the fix: both surfaces spell those buckets identically, the wire ``code`` #: alone cannot say which surface answered, and raising a class that is *not* a #: ``RouterError`` for a Router refusal is what made ``except RouterError`` a #: dead handler. One class per bucket, reachable from both modules, is the only -#: shape where neither ``except`` clause is wrong. +#: shape where neither ``except`` clause is wrong. The v2 contract's account rate +#: limit (``components/responses/RateLimited`` in ``spec/openapi.yaml``) and +#: Router's ``rate_limited`` bucket are the same windowed allowance spelled the +#: same way, so one class serves both, and ``.error_type == "rate_limited"`` is +#: set on it from either surface. _BY_CODE: dict[str, type[ComfyError]] = { "invalid_workflow": InvalidWorkflow, "workflow_format_ui": WorkflowFormatUi, @@ -132,6 +138,7 @@ def __init__(self, message: str, *, error: JobError | None = None) -> None: "blob_not_found": BlobNotFound, "idempotency_key_reuse": IdempotencyKeyReuse, "insufficient_credits": InsufficientCredits, + "rate_limited": RateLimited, "not_found": NotFound, # public-api currently returns entity-specific 404 codes even though the # spec documents the generic `not_found`; map them so a missing job/asset @@ -150,7 +157,7 @@ def _class_for(exc: ApiError) -> type[ComfyError]: Three lookups in precedence order, then a fallback that depends on which surface answered: - 1. :data:`_BY_CODE` -- the v2 envelope's codes, plus the three buckets both + 1. :data:`_BY_CODE` -- the v2 envelope's codes, plus the four buckets both surfaces spell the same way. 2. :data:`~comfy_sdk.router_exceptions._BY_ERROR_TYPE` -- the Router contract's closed set. The low layer preserves Router's bucket as the @@ -249,7 +256,7 @@ def to_sdk_error(exc: ApiError) -> ComfyError: # `_class_for` already reads it: `error_type` is set only for a response # that identified itself as Router's, and for those this branch is # unreachable — every Router bucket resolves to a `RouterError` subclass, - # including the three both surfaces spell alike, so the entries are + # including the four both surfaces spell alike, so the entries are # forwarded above. return cls( str(exc), @@ -364,7 +371,7 @@ def translating(*, idempotency_key: str | None = None) -> Iterator[None]: #: Explicit because several of these names are re-exports rather than -#: definitions: :class:`~comfy_sdk.router_exceptions.RouterError` and the three +#: definitions: :class:`~comfy_sdk.router_exceptions.RouterError` and the four #: buckets both surfaces spell the same way are defined in #: :mod:`comfy_sdk.router_exceptions`, and ``ComfyError`` in #: :mod:`comfy_sdk._errors` -- one class object each, so a name this module and @@ -385,6 +392,7 @@ def translating(*, idempotency_key: str | None = None) -> Iterator[None]: "MissingAsset", "NotFound", "QueueFull", + "RateLimited", "RouterError", "Unauthorized", "WorkflowFormatUi", diff --git a/src/comfy_sdk/router_exceptions.py b/src/comfy_sdk/router_exceptions.py index 77de7f9..f96fe0e 100644 --- a/src/comfy_sdk/router_exceptions.py +++ b/src/comfy_sdk/router_exceptions.py @@ -53,9 +53,9 @@ class docstrings below reproduce. ``tests/test_router_spec_contract.py`` reads ``internal_error``. A client that rejected the unknown value would fail hardest exactly when something has already gone wrong. -Three of these names -- ``Unauthorized``, ``Forbidden``, ``InsufficientCredits`` --- also exist in :mod:`comfy_sdk.exceptions`, which is where the workflow -surface's error codes are mapped. **They are the same class object, defined +Four of these names -- ``Unauthorized``, ``Forbidden``, ``InsufficientCredits``, +``RateLimited`` -- also exist in :mod:`comfy_sdk.exceptions`, which is where +the workflow surface's error codes are mapped. **They are the same class object, defined here and re-exported there**, so ``comfy_sdk.exceptions.Unauthorized is comfy_sdk.router_exceptions.Unauthorized`` and either import catches whatever the other one does. ``tests/test_exception_modules.py`` asserts that of every @@ -68,11 +68,12 @@ class docstrings below reproduce. ``tests/test_router_spec_contract.py`` reads for insufficient credits, a rejected key or a model the caller is not entitled to compiled, type-checked, and caught nothing. Merging them is what makes the broad catch honest. The consequence to know is the other direction: a *workflow* -call that fails ``401``/``403``/``402`` now raises a :class:`RouterError` -subclass too, because one class cannot be a ``RouterError`` on one surface and -not on the other. ``except Unauthorized`` (from either module) is unchanged; -``except RouterError`` is wider than the name suggests for exactly those three -buckets. +call that fails ``401``/``403``/``402``, or a v2 jobs/assets call refused +``429 rate_limited``, now raises a :class:`RouterError` subclass too, because +one class cannot be a ``RouterError`` on one surface and not on the other. +``except Unauthorized`` (from either module) is unchanged; ``except +RouterError`` is wider than the name suggests for exactly those four buckets: +``unauthorized``, ``forbidden``, ``insufficient_credits`` and ``rate_limited``. This module is still deliberately *not* star-re-exported from the package root: lifting :class:`NotEnabled` there while ``comfy_sdk.InvalidInput`` stayed a name @@ -471,7 +472,12 @@ class RateLimited(RouterError): It shares ``429`` with :class:`ConcurrencyLimitExceeded` and is not the same thing: that one clears the moment one of the caller's own in-flight calls finishes, so retrying in seconds is right, whereas nothing the caller does - drains this one early. ``detail`` names the window. + drains this one early. On the Router surface, ``detail`` names the window. + + The v2 jobs/assets gateway raises this same class for its per-account + request rate limit (``code == "rate_limited"``), with ``retry_after`` read + from ``Retry-After`` when the response sent one: both surfaces spell the + windowed allowance identically, so one class serves both. """ error_type = "rate_limited" diff --git a/tests/test_error_mapping.py b/tests/test_error_mapping.py index cb40ef3..1ab3ee6 100644 --- a/tests/test_error_mapping.py +++ b/tests/test_error_mapping.py @@ -278,7 +278,6 @@ def test_a_body_that_names_no_bucket_still_falls_back_to_the_status(body: object ("invalid_input", 422, "InvalidInput"), ("model_not_found", 404, "ModelNotFound"), ("concurrency_limit_exceeded", 409, "ConcurrencyLimitExceeded"), - ("rate_limited", 429, "RateLimited"), ("provider_error", 500, "ProviderError"), ("deadline_exceeded", 504, "DeadlineExceeded"), ], @@ -295,9 +294,11 @@ def test_a_router_only_bucket_raises_its_typed_class( assert err.retry_after == 7 -@pytest.mark.parametrize("code", ["unauthorized", "forbidden", "insufficient_credits"]) +@pytest.mark.parametrize( + "code", ["unauthorized", "forbidden", "insufficient_credits", "rate_limited"] +) def test_a_bucket_both_surfaces_spell_keeps_its_v2_class(code: str) -> None: - # These three codes are spelled identically by the v2 envelope and by + # These four codes are spelled identically by the v2 envelope and by # Router, and the code string alone cannot say which surface answered. # Retyping them to the RouterError twins would break every jobs-surface # handler to fix none -- the v2 classes fire on the router surface too. @@ -307,6 +308,24 @@ def test_a_bucket_both_surfaces_spell_keeps_its_v2_class(code: str) -> None: assert type(err) is getattr(sdk, "".join(p.title() for p in code.split("_"))) +def test_a_v2_envelope_rate_limited_is_the_shared_router_class() -> None: + # Deliberate sharing, not a mis-mapping: the v2 jobs/assets throttle and + # Router's `rate_limited` bucket are the same windowed allowance spelled the + # same way, so a v2 envelope with no Router header still raises the Router + # class -- exactly as `unauthorized`, `forbidden` and `insufficient_credits` + # do. `error_type` is left unset to prove the class comes from the code alone. + from comfy_sdk.router_exceptions import RateLimited, RouterError + + exc = ApiError("slow down", code="rate_limited", http_status=429, retry_after=7) + assert exc.error_type is None + err = to_sdk_error(exc) + assert type(err) is RateLimited + assert isinstance(err, RouterError) + assert err.code == "rate_limited" + assert err.error_type == "rate_limited" + assert err.retry_after == 7 + + # --- a response nothing in the stack recognised --- # # A 5xx answered by a load balancer in front of the deployment carries no From 07253bad925c7b66bf83c20d94d63354faba0161 Mon Sep 17 00:00:00 2001 From: Matt Miller Date: Fri, 9 Oct 2026 23:48:12 +0000 Subject: [PATCH 2/2] docs(errors): cover status-derived shared codes, fold changelog bullet, fix _BY_CODE comment --- CHANGELOG.md | 3 --- README.md | 9 ++++++--- src/comfy_sdk/exceptions.py | 9 +++++---- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c595ca9..614fb61 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -218,9 +218,6 @@ the fuller account of each version, including verification notes. - `ApiError.error_type` records the Router bucket a response named (`X-Comfy-Error-Type`, or the body's `error_type`), or `None` when it named none — which is also how the SDK tells which surface answered. - -### Changed - - `rate_limited` is now listed explicitly in `comfy_sdk.exceptions._BY_CODE` beside the three other codes both surfaces share; a v2 jobs/assets throttle raised `comfy_sdk.router_exceptions.RateLimited` before and still does. README and docstrings now say diff --git a/README.md b/README.md index eb0d539..5a9a42a 100644 --- a/README.md +++ b/README.md @@ -1101,9 +1101,12 @@ bucket newer than your installed version — Router names the bucket on every error it sends, and that is what types the exception. A response that never reached Router carries no bucket to read (an intermediary's HTML `404`, a bare `503 no healthy upstream`), so it arrives as a plain `ComfyError` with the -status on `.http_status` — unless it is a v2 envelope naming one of the four -codes both surfaces share (below), which raises that shared class instead; -keep an `except ComfyError` outside the clause above if you need to handle those in the same place. +status on `.http_status` — unless it lands on one of the four codes both +surfaces share (below), which raises that shared class instead: a v2 envelope +naming one of them, or a bare `401`/`402`/`403` whose status alone maps to +`unauthorized`/`insufficient_credits`/`forbidden` — so `except RouterError` +firing is not by itself proof that Router answered. Keep an `except ComfyError` +outside the clause above if you need to handle the rest in the same place. `RouterError` is exported from the package root because it is the handler most callers write first. The nineteen per-bucket classes stay in diff --git a/src/comfy_sdk/exceptions.py b/src/comfy_sdk/exceptions.py index e69d663..6562503 100644 --- a/src/comfy_sdk/exceptions.py +++ b/src/comfy_sdk/exceptions.py @@ -112,10 +112,11 @@ def __init__(self, message: str, *, error: JobError | None = None) -> None: self.error = error -#: Wire ``code`` -> the class this SDK raises for it. Only the codes the v2 -#: envelope owns are listed here; the Router buckets are looked up in -#: :data:`comfy_sdk.router_exceptions._BY_ERROR_TYPE` instead, so that table -#: stays the single copy of the contract's closed set. +#: Wire ``code`` -> the class this SDK raises for it. The codes listed here are +#: the ones the v2 envelope can send; every other Router bucket is looked up in +#: :data:`comfy_sdk.router_exceptions._BY_ERROR_TYPE` instead, which stays the +#: single copy of the contract's closed set -- the four shared codes below are +#: the only Router buckets that appear in both tables. #: #: Four entries -- ``insufficient_credits``, ``unauthorized``, ``forbidden``, #: ``rate_limited`` -- name classes that live in