Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,10 @@ 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.
- `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.4.0] - 2026-09-18

Expand Down
19 changes: 13 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1101,8 +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`; 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 twenty per-bucket classes stay in
Expand All @@ -1115,14 +1119,17 @@ callers write first. The twenty 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
Expand Down
29 changes: 19 additions & 10 deletions src/comfy_sdk/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
CancelRefused,
Forbidden,
InsufficientCredits,
RateLimited,
RouterError,
Unauthorized,
_detail_from,
Expand Down Expand Up @@ -111,19 +112,25 @@ 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.
#:
#: 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,
Expand All @@ -132,6 +139,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
Expand All @@ -150,7 +158,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
Expand Down Expand Up @@ -249,7 +257,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),
Expand Down Expand Up @@ -364,7 +372,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
Expand All @@ -385,6 +393,7 @@ def translating(*, idempotency_key: str | None = None) -> Iterator[None]:
"MissingAsset",
"NotFound",
"QueueFull",
"RateLimited",
"RouterError",
"Unauthorized",
"WorkflowFormatUi",
Expand Down
26 changes: 16 additions & 10 deletions src/comfy_sdk/router_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -478,8 +479,13 @@ 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, and ``Retry-After`` the
seconds to wait, carried on ``retry_after``.
drains this one early. On the Router surface, ``detail`` names the window,
and ``Retry-After`` the seconds to wait, carried on ``retry_after``.

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"
Expand Down
25 changes: 22 additions & 3 deletions tests/test_error_mapping.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
],
Expand All @@ -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.
Expand All @@ -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
Expand Down
Loading