diff --git a/CHANGELOG.md b/CHANGELOG.md index a81f44d..02a7ede 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,15 @@ the fuller account of each version, including verification notes. - `models.run` now populates `RouterError.errors` from a Router 422's per-field `detail[]` and uses the entries' messages as `detail`, instead of `HTTP 422`; `comfy_low.ApiError.validation_errors` carries the raw entries. +- **A Router `detail[]` summary now names its fields, and is sanitised.** Both the awaited + (`models.run`) and the queued (`submit`) paths build the human-readable string with one shared + function, so a single server response reads the same way whichever surface raised it. Each entry + renders as `: `, so two `field required` errors now read `body.seed: field required; + body.steps: field required` rather than collapsing to an unrecoverable `field required; field + required`. The joined line gets the same treatment every other body-derived string already gets: + control characters, ANSI escapes and bidi overrides reduced, whitespace collapsed, and a 256-character + cap — so a hostile or merely careless `msg` can no longer scribble on a terminal or flood a log line. + Only the summary string changes; `.errors` still carries the raw typed entries. ### Changed diff --git a/src/comfy_low/errors.py b/src/comfy_low/errors.py index e85e6e0..b518320 100644 --- a/src/comfy_low/errors.py +++ b/src/comfy_low/errors.py @@ -263,12 +263,17 @@ def _clean(value: Any) -> str | None: reads as absent rather than being coerced, so a malformed body degrades to the status-derived default instead of producing a nonsense code. - The list form is still dropped here, and that is still right for the CODE - fields: a list is not a code and stringifying one would put a Python repr - where a caller expects a wire value. It is no longer how the list ``detail`` - is *read*, though — :func:`error_from_envelope` handles that shape itself, - before it reaches this function, because the entries carry both the - per-field data and the messages that summarise it. + The list form is dropped here, and that is right for the CODE fields: a list + is not a code and stringifying one would put a Python repr where a caller + expects a wire value. It is load-bearing for ``detail`` too: a Router + validation array that produced no summary — every member a non-mapping, or + every ``msg`` missing or blank — still reaches this function through the + ``message = _clean(raw_detail)`` fallback in :func:`error_from_envelope`, so + the ``isinstance(value, str)`` guard below is what keeps a Python list repr + out of the caller's message on that path. It is *not* how a populated array + is read: :func:`error_from_envelope` runs it through :func:`summarise_detail` + before that fallback, because the entries carry both the per-field data and + the messages that summarise it. """ if not isinstance(value, str): return None @@ -276,6 +281,89 @@ def _clean(value: Any) -> str | None: return stripped or None +def _location(raw_loc: Any) -> str: + """A ``detail[]`` entry's ``loc`` rendered as a dotted path, or ``""``. + + :meth:`comfy_sdk.router_exceptions.ValidationErrorDetail.location` delegates + here rather than restating the join, so a per-field failure reads the same + whichever surface built the summary — ``body.images.0`` where an integer + indexes into an array — and the two cannot drift apart. + """ + if isinstance(raw_loc, Sequence) and not isinstance(raw_loc, (str, bytes)): + # Only the scalar member types a `loc` path is made of. `str(part)` on + # anything else would put a Python repr into the caller's message — a + # server-controlled `{"loc": [["a", "b"]]}` rendering as `['a', 'b']: + # ...` — which is the same leak `_clean`'s isinstance guard exists to + # stop, and `clean_body_excerpt` does not strip brackets. `bool` is + # excluded despite being an `int`: `True` is not an index or a field + # name, and rendering one as a path segment would be nonsense. + parts = [ + str(part) + for part in raw_loc + if isinstance(part, str) or (isinstance(part, int) and not isinstance(part, bool)) + ] + return ".".join(parts) + return "" + + +def summarise_detail(entries: Any) -> str | None: + """A bounded, single-line, printable summary of a Router per-field validation + body, or ``None`` when it named nothing. + + ``entries`` is the raw ``detail`` array — a sequence of ``{loc, msg, ...}`` + mappings. Each entry renders as ``: `` (the dotted ``loc`` path and + the field's own message), or whichever one of the two it carries; the parts + join with ``"; "``. Non-mapping members are skipped rather than coerced: this + runs while handling a failure, and one malformed member must not cost the + caller the others. + + Including ``loc`` is what makes two ``field required`` entries read as the + two fields they name rather than collapsing to an unrecoverable ``field + required; field required``. The joined line is then reduced by + :func:`clean_body_excerpt` — the same unprintable-category reduction, + whitespace collapse and :data:`_BODY_EXCERPT_LIMIT` cap every other + server-controlled string headed for a traceback or a log line already gets. + Without it a proxy-, provider- or server-supplied ``msg`` carrying newlines, + C0/C1 controls, ANSI escapes or bidi overrides would reach ``str(exc)`` + verbatim and unbounded — the exact thing :data:`_UNPRINTABLE_CATEGORIES` + exists to prevent — and enough long entries would make the message + arbitrarily large. + + Defined on this lowest layer because both error surfaces summarise the same + array off their own response and must produce one string for one wire body: + :func:`error_from_envelope` on the awaited ``models.run`` path, + ``comfy_sdk.router_exceptions`` on the queued one. + """ + if not isinstance(entries, Sequence) or isinstance(entries, (str, bytes)): + return None + # Stop accumulating once there is provably enough to fill the capped result, + # rather than rendering a server-controlled array in full and slicing 256 + # characters off the end. `clean_body_excerpt` already reads only its first + # `_BODY_EXCERPT_LIMIT * _BODY_EXCERPT_WINDOW` characters, so anything past + # that budget cannot reach the output — collecting it would only make the + # cost of describing a body scale with the body, which is the property + # `_BODY_EXCERPT_WINDOW` exists to bound. + budget = _BODY_EXCERPT_LIMIT * _BODY_EXCERPT_WINDOW + parts: list[str] = [] + size = 0 + for entry in entries: + if not isinstance(entry, Mapping): + continue + location = _location(entry.get("loc")) + message = _clean(entry.get("msg")) + if location and message: + part = f"{location}: {message}" + elif location or message: + part = location or message or "" + else: + continue + parts.append(part) + size += len(part) + 2 # the "; " this part will join on + if size >= budget: + break + return clean_body_excerpt("; ".join(parts)) + + #: The status a cancel refusal is answered with. _CANCEL_REFUSAL_STATUS = 409 @@ -351,11 +439,12 @@ def error_from_envelope( Router also spells ``detail`` two ways, and both are read here. A request-level failure sends a string, which becomes the message. A per-field model-validation failure sends an ARRAY of ``{loc, msg, type, ...}`` entries - instead; those land raw on :attr:`ApiError.validation_errors`, and their - ``msg`` values — joined with ``"; "`` — become the message. Before that the - array read as no message at all, so such a failure reached a caller as a - bare ``HTTP 422`` with the raw JSON glued on as a body excerpt, and the - per-field reasons the array exists to carry were dropped on the floor. + instead; those land raw on :attr:`ApiError.validation_errors`, and + :func:`summarise_detail` renders them — each ``: ``, sanitised and + bounded — into the message. Before that the array read as no message at all, + so such a failure reached a caller as a bare ``HTTP 422`` with the raw JSON + glued on as a body excerpt, and the per-field reasons the array exists to + carry were dropped on the floor. The precedence is: the envelope's ``code`` always wins; then, when the response identifies itself as Router's — the ``X-Comfy-Error-Type`` header @@ -431,15 +520,12 @@ def error_from_envelope( # and one malformed member must not cost the caller the others. validation_errors = tuple(entry for entry in raw_detail if isinstance(entry, Mapping)) if not message: - # The entries' own messages, joined, rather than the status-derived - # default: a body that named every failing field would otherwise - # reach a caller as a bare `HTTP 422`. - summary = "; ".join( - cleaned - for cleaned in (_clean(entry.get("msg")) for entry in validation_errors) - if cleaned - ) - message = summary or message + # The entries summarised — each `: `, sanitised and bounded + # — rather than the status-derived default: a body that named every + # failing field would otherwise reach a caller as a bare `HTTP 422`. + # `comfy_sdk.router_exceptions` summarises the same array the same way + # off the queued surface, so one wire body yields one message. + message = summarise_detail(raw_detail) or message if not message: # Router names its human-readable string `detail`, not `error.message`. message = _clean(raw_detail) diff --git a/src/comfy_sdk/exceptions.py b/src/comfy_sdk/exceptions.py index 8e6c2a5..5656bf7 100644 --- a/src/comfy_sdk/exceptions.py +++ b/src/comfy_sdk/exceptions.py @@ -238,7 +238,19 @@ def to_sdk_error(exc: ApiError) -> ComfyError: # and none of the remaining classes takes the argument. A validation body # that reaches this branch — a `detail[]` under a v2 `error.code`, or under # the status-derived guess when no bucket was sent at all — still gets the - # entries' messages, since those became `exc.message` one layer down. + # entries' summary, since `summarise_detail` made it `exc.message` one layer + # down. The same holds for the `queue_full` early return above. + # + # The array is deliberately NOT used to reroute these into the Router + # hierarchy. `detail[]` is a body shape any server, proxy or gateway can + # send (a FastAPI `RequestValidationError` is exactly it), so keying the + # class off it would let an intermediary in front of the v2 jobs surface + # decide which `except` a caller runs. Provenance is the header, and + # `_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 + # forwarded above. return cls( str(exc), code=exc.code, diff --git a/src/comfy_sdk/router_exceptions.py b/src/comfy_sdk/router_exceptions.py index eb1164a..7b2924d 100644 --- a/src/comfy_sdk/router_exceptions.py +++ b/src/comfy_sdk/router_exceptions.py @@ -100,7 +100,7 @@ class docstrings below reproduce. ``tests/test_router_spec_contract.py`` reads from dataclasses import dataclass from typing import Any -from comfy_low.errors import clean_request_id +from comfy_low.errors import _location, clean_request_id, summarise_detail from ._errors import ComfyError @@ -181,8 +181,13 @@ class ValidationErrorDetail: @property def location(self) -> str: - """``loc`` as a dotted path -- ``body.images.0`` -- for display.""" - return ".".join(str(part) for part in self.loc) + """``loc`` as a dotted path -- ``body.images.0`` -- for display. + + The one renderer :func:`comfy_low.errors.summarise_detail` uses, shared + rather than restated so this property and the summary built from the + same array can never disagree about what a field is called. + """ + return _location(self.loc) class RouterError(ComfyError): @@ -709,6 +714,9 @@ def error_from_response( errors = tuple( _detail_from(entry) for entry in raw_detail if isinstance(entry, Mapping) ) + # The same summariser the awaited `models.run` path uses, so one wire + # body produces one `.detail` whichever surface built the exception. + detail = summarise_detail(raw_detail) if error_type is None: error_type = _clean(body.get("error_type")) @@ -716,7 +724,7 @@ def error_from_response( error_type = _ERROR_TYPE_BY_STATUS.get(http_status) if detail is None: - detail = _summarise(errors) or f"HTTP {http_status}" + detail = f"HTTP {http_status}" return exception_for(error_type)( detail, @@ -768,9 +776,12 @@ def error_from_completion( detail = raw_detail or None elif isinstance(raw_detail, Sequence) and not isinstance(raw_detail, (str, bytes)): errors = tuple(_detail_from(entry) for entry in raw_detail if isinstance(entry, Mapping)) + # One summariser across both surfaces (see `error_from_response`), so the + # queued path's `.detail` matches the awaited one for the same body. + detail = summarise_detail(raw_detail) if detail is None: - detail = _summarise(errors) or f"the request completed with error_type {error_type!r}" + detail = f"the request completed with error_type {error_type!r}" return exception_for(error_type)( detail, @@ -830,7 +841,17 @@ def _detail_from(entry: Mapping[str, Any]) -> ValidationErrorDetail: raw_loc = entry.get("loc") loc: tuple[str | int, ...] = () if isinstance(raw_loc, Sequence) and not isinstance(raw_loc, (str, bytes)): - loc = tuple(part if isinstance(part, (str, int)) else str(part) for part in raw_loc) + # A member that is not a path segment is DROPPED, not stringified. The + # field is typed as a path -- a field name or an array index -- and + # `str(part)` on a server-controlled nested value put a Python repr in + # there instead (`('body', "['a', 'b']")`), which then reached the user + # through `.location`. `bool` is excluded despite being an `int`: `True` + # is neither a field name nor an index. + loc = tuple( + part + for part in raw_loc + if isinstance(part, str) or (isinstance(part, int) and not isinstance(part, bool)) + ) msg, reason, ctx = entry.get("msg"), entry.get("type"), entry.get("ctx") return ValidationErrorDetail( @@ -842,22 +863,6 @@ def _detail_from(entry: Mapping[str, Any]) -> ValidationErrorDetail: ) -def _summarise(errors: Sequence[ValidationErrorDetail]) -> str: - """A one-line message for a per-field failure. - - This is *in addition to* ``.errors``, never instead of it -- the entries stay - readable as data, and a caller branching on a field reads them rather than - parsing this back apart. - """ - parts: list[str] = [] - for entry in errors: - if entry.location and entry.msg: - parts.append(f"{entry.location}: {entry.msg}") - elif entry.location or entry.msg: - parts.append(entry.location or entry.msg) - return "; ".join(parts) - - __all__ = [ "CANCEL_REFUSALS", "ERROR_TYPE_HEADER", diff --git a/tests/test_error_mapping.py b/tests/test_error_mapping.py index a1e95c8..7688562 100644 --- a/tests/test_error_mapping.py +++ b/tests/test_error_mapping.py @@ -195,7 +195,8 @@ def test_a_router_validation_body_is_read_rather_than_coerced_into_the_message() error_type="internal_error", ) assert err.code == "internal_error" - assert err.message == "too large" + # `: `, the same rendering the queued surface uses. + assert err.message == "body.steps: too large" # The intent the old status-derived message protected, unchanged: whatever # reaches `str(exc)` is prose, never a repr of the array. assert "[" not in err.message @@ -204,17 +205,31 @@ def test_a_router_validation_body_is_read_rather_than_coerced_into_the_message() ) -def test_a_validation_body_whose_entries_state_no_message_still_degrades() -> None: - # The entries decoded, so they are carried; none of them named a reason, so - # there is nothing to say but the status. Both halves matter: a caller that - # branches on `.errors` still gets them, and the message never becomes an - # empty string. +def test_a_validation_body_whose_entries_name_nothing_still_degrades() -> None: + # The entries decoded, so they are carried; none of them named a field OR a + # reason, so there is nothing to say but the status. Both halves matter: a + # caller that branches on `.errors` still gets them, and the message never + # becomes an empty string. (An entry that names a field but no message is a + # separate case -- it surfaces the field, see the loc-only test below.) err = error_from_envelope( 422, - {"detail": [{"loc": ["body", "steps"]}, {"msg": " "}]}, + {"detail": [{"type": "missing"}, {"msg": " "}]}, error_type="invalid_input", ) assert err.message == "HTTP 422" + assert err.validation_errors == ({"type": "missing"}, {"msg": " "}) + + +def test_a_validation_entry_with_a_loc_but_no_msg_names_the_field() -> None: + # A field named with no reason still beats the bare status: the loc alone + # tells the caller which field the server rejected. A blank `msg` reads as + # absent, exactly as `_clean` treats every other whitespace-only wire string. + err = error_from_envelope( + 422, + {"detail": [{"loc": ["body", "steps"]}, {"msg": " "}]}, + error_type="invalid_input", + ) + assert err.message == "body.steps" assert err.validation_errors == ({"loc": ["body", "steps"]}, {"msg": " "}) @@ -355,7 +370,7 @@ def test_a_validation_array_that_states_a_message_drops_the_excerpt_too() -> Non error_type="invalid_input", body_excerpt=raw, ) - assert str(err) == "too large" + assert str(err) == "body.steps: too large" assert err.body_excerpt is None @@ -532,3 +547,175 @@ def test_an_undecodable_success_body_keeps_what_was_served_instead() -> None: assert str(err) == ( "Could not decode the 200 response body as JSON: proxy: gateway timeout" ) + + +# --- the per-field summary is sanitised, bounded, and one string per body --- +# +# The `detail[]` summary reaches `str(exc)`, a traceback and a log line, and its +# `msg` is server-, proxy- or provider-controlled text. It gets the same +# treatment every other body-derived string gets (`clean_body_excerpt`), and the +# awaited `models.run` path and the queued surface render it through one +# summariser so a caller's branch cannot work on one and silently not the other. + + +def test_summarise_detail_names_each_field_rather_than_collapsing() -> None: + # Two `field required` entries must not collapse to the unrecoverable + # `field required; field required`: the loc is what tells them apart. + from comfy_low.errors import summarise_detail + + summary = summarise_detail( + [ + {"loc": ["body", "a"], "msg": "field required"}, + {"loc": ["body", "b"], "msg": "field required"}, + ] + ) + assert summary == "body.a: field required; body.b: field required" + # An integer index renders like the queued surface's `.location`. + assert summarise_detail([{"loc": ["body", "images", 0], "msg": "bad"}]) == "body.images.0: bad" + # Nothing to summarise -> None, so the caller falls back to the status. + assert summarise_detail([]) is None + assert summarise_detail([1, "x"]) is None + assert summarise_detail("not a sequence of entries") is None + + +def test_a_validation_summary_is_sanitised_before_it_reaches_str() -> None: + # A newline, a C0 NUL and an ANSI escape in a provider `msg` must not reach + # the terminal reading the error verbatim -- the escape's ESC byte is the + # dangerous part and is reduced to a space, whitespace collapses to one line. + err = error_from_envelope( + 422, + {"detail": [{"loc": ["body", "steps"], "msg": "line one\nline\x1b[2Jtwo\x00three"}]}, + error_type="invalid_input", + ) + assert "\n" not in err.message + assert "\x1b" not in err.message + assert "\x00" not in err.message + assert err.message == "body.steps: line one line [2Jtwo three" + + +def test_a_validation_summary_is_capped_at_the_excerpt_limit() -> None: + # Many long entries could otherwise make the message arbitrarily large; the + # same 256-char cap the excerpt gets applies to the summary. + from comfy_low.errors import _BODY_EXCERPT_LIMIT + + err = error_from_envelope( + 422, + {"detail": [{"msg": "x" * 5000} for _ in range(10)]}, + error_type="invalid_input", + ) + assert len(err.message) == _BODY_EXCERPT_LIMIT + + +def test_a_detail_array_of_non_mappings_never_leaks_a_list_repr() -> None: + # When the array yields no summary -- every member a non-mapping -- the + # message falls through to `_clean(raw_detail)` with the LIST. `_clean`'s + # isinstance guard is load-bearing there: without it the caller's message + # would be a Python list repr. It degrades to the status instead. + err = error_from_envelope(422, {"detail": [1, "x", 2]}, error_type="invalid_input") + assert err.message == "HTTP 422" + assert "[" not in err.message + assert err.validation_errors == () + + +# --- a `detail[]` array identifies the Router surface even under the +# status-derived guess a stripped header falls to, or a code this version +# does not know: the typed entries must not be dropped by the non-Router +# branch. + + +def test_a_validation_array_under_a_shared_bucket_keeps_its_entries() -> None: + import comfy_sdk.router_exceptions as rx + + low = error_from_envelope( + 401, + {"detail": [{"loc": ["body", "key"], "msg": "field required"}]}, + error_type="unauthorized", + ) + err = to_sdk_error(low) + # `unauthorized` is one of the three buckets both surfaces spell alike. They + # are a single class now (#157), re-exported from both modules, so there is + # no longer a collision to resolve -- `except RouterError` and + # `except comfy_sdk.Unauthorized` both catch this. What this test pins is the + # part that is still this change's: the array's typed entries survive the + # mapping instead of being dropped on the non-Router branch. + assert isinstance(err, rx.Unauthorized) + assert isinstance(err, rx.RouterError) + assert rx.Unauthorized is SdkUnauthorized + assert [e.msg for e in err.errors] == ["field required"] + assert err.errors[0].location == "body.key" + + +def test_a_stripped_422_header_validation_array_keeps_its_v2_class() -> None: + import comfy_sdk.router_exceptions as rx + from comfy_sdk.exceptions import InvalidWorkflow + + # No `error_type`: the 422 falls to the status-derived `invalid_workflow` + # guess. The `detail[]` array must NOT retype it into the Router hierarchy + # -- the array is a body shape any proxy or gateway can send, and the module + # rule is that only a response carrying a BUCKET gets retyped. Rerouting on + # the array would silently stop `except InvalidWorkflow` from firing. + low = error_from_envelope(422, {"detail": [{"loc": ["body", "steps"], "msg": "too large"}]}) + assert low.code == "invalid_workflow" + err = to_sdk_error(low) + assert isinstance(err, InvalidWorkflow) + assert not isinstance(err, rx.RouterError) + # The per-field reasons still reach the caller: `summarise_detail` made them + # the message one layer down, which is what this change fixed. + assert "body.steps: too large" in str(err) + + +def test_a_v2_envelope_with_an_array_keeps_its_class_details_and_summary() -> None: + import comfy_sdk.router_exceptions as rx + from comfy_sdk.exceptions import InvalidWorkflow + + # An `error.message` AND a `detail[]` array, under a v2 envelope code. The + # class stays the one integrators catch, and `details` -- the per-node + # diagnostics `InvalidWorkflow` documents -- is still forwarded. + low = error_from_envelope( + 422, + { + "error": { + "code": "invalid_workflow", + "message": "the graph is invalid", + "details": {"node_errors": {"3": "bad"}}, + }, + "detail": [{"loc": ["body", "steps"], "msg": "too large"}], + }, + ) + err = to_sdk_error(low) + assert isinstance(err, InvalidWorkflow) + assert not isinstance(err, rx.RouterError) + assert err.details == {"node_errors": {"3": "bad"}} + # The envelope's own message still wins as the human-readable string. + assert "the graph is invalid" in str(err) + + +def test_a_loc_member_that_is_not_a_scalar_never_leaks_a_repr() -> None: + # `str(part)` on a nested member would render `['a', 'b']: bad` into the + # user-visible message -- the same list-repr leak `_clean`'s isinstance + # guard exists to stop, and `clean_body_excerpt` does not strip brackets. + err = error_from_envelope( + 422, + {"detail": [{"loc": ["body", ["a", "b"], 0], "msg": "bad"}]}, + error_type="invalid_input", + ) + assert "[" not in err.message + assert err.message == "body.0: bad" + + +def test_a_huge_detail_array_stops_accumulating_at_the_excerpt_budget() -> None: + from comfy_low.errors import _BODY_EXCERPT_LIMIT + + # The cap must not be reached by rendering the whole server-controlled array + # and slicing the tail off: describing a body costs the same whether it is + # small or huge. + entries = [{"msg": "x" * 100} for _ in range(10_000)] + err = error_from_envelope(422, {"detail": entries}, error_type="invalid_input") + assert len(err.message) == _BODY_EXCERPT_LIMIT + + +def test_a_shared_bucket_without_an_array_still_maps_by_code() -> None: + # The array is not what types a shared bucket: a bare `unauthorized` with no + # `detail[]` still maps by code to the one class both surfaces export. + err = to_sdk_error(ApiError("no", code="unauthorized", http_status=401)) + assert isinstance(err, SdkUnauthorized) diff --git a/tests/test_models_run.py b/tests/test_models_run.py index df5fe86..8fc9fd6 100644 --- a/tests/test_models_run.py +++ b/tests/test_models_run.py @@ -581,9 +581,13 @@ def _assert_validation_surface(exc: InvalidInput) -> None: # and which a caller reads to say what the limit actually was. assert exc.errors[0].ctx == {"limit_value": 8} assert exc.errors[0].input == 50 - # The human-readable line is the entries' own messages, joined -- not the - # `HTTP 422` a caller used to get, and not a Python repr of the array. - assert exc.detail == "ensure this value is less than or equal to 8; unknown model variant" + # The human-readable line is the entries summarised -- each `: `, + # the same rendering the queued surface produces -- not the `HTTP 422` a + # caller used to get, and not a Python repr of the array. + assert exc.detail == ( + "body.steps: ensure this value is less than or equal to 8; " + "body.model: unknown model variant" + ) assert str(exc) == exc.detail assert "[" not in exc.detail assert exc.error_type == "invalid_input" @@ -650,6 +654,15 @@ def test_the_run_and_queued_paths_agree_on_one_validation_body(server) -> None: ) assert queued is not None assert excinfo.value.errors == queued.errors + # ...and the human-readable line, too: both surfaces summarise the array + # through one function, so a body that names two fields reads the same on + # `run()` and on the queued surface rather than collapsing to a bare join of + # the messages on one of them. + assert excinfo.value.detail == queued.detail + assert excinfo.value.detail == ( + "body.steps: ensure this value is less than or equal to 8; " + "body.model: unknown model variant" + ) # --- the key survives the failure ---------------------------------------- diff --git a/tests/test_router_exceptions.py b/tests/test_router_exceptions.py index 0f59e3c..08a69e5 100644 --- a/tests/test_router_exceptions.py +++ b/tests/test_router_exceptions.py @@ -552,3 +552,14 @@ def test_the_default_policy_retries_a_throttled_bucket_that_named_a_pace() -> No assert isinstance(paced, RateLimited) assert policy.should_retry(paced) is True assert policy.should_retry(unpaced) is False + + +def test_the_location_rendering_is_shared_with_the_summariser() -> None: + """`.location` and the summary must never disagree about a field's name.""" + from comfy_low.errors import summarise_detail + from comfy_sdk.router_exceptions import _detail_from + + # A non-scalar `loc` member is dropped by both, so neither renders a repr. + entry = {"loc": ["body", ["a", "b"], 0], "msg": "bad"} + assert _detail_from(entry).location == "body.0" + assert summarise_detail([entry]) == "body.0: bad"