From 82b130cb14ad8606e5f6bf76907fd7b41b1f6f0a Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sat, 17 Oct 2026 11:35:12 +0300 Subject: [PATCH 1/6] fix: keep notes and attributes when pickling or copying any ClientError One __reduce__ on ClientError rebuilds from args without calling __init__ and restores __dict__, replacing _KeywordReduceMixin and the StatusError reconstructor. add_note() notes and user-set attributes now survive pickle, copy.copy and copy.deepcopy for every error class. --- AGENTS.md | 8 ++-- src/httpware/errors.py | 53 ++++++++-------------- tests/test_errors.py | 99 +++++++++++++++++++++++++----------------- 3 files changed, 82 insertions(+), 78 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 5c584ad..3fcc3d6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -36,9 +36,11 @@ Every module under `src/httpware/` is named for what it does; read it. What a si sibling). - `errors.py` owns the tree and both construction rules. Status-keyed `StatusError` subclasses take a single positional `response` and never define `__init__`; the six non-status subclasses are - keyword-only and inherit `__reduce__` from `_KeywordReduceMixin`, whose precondition is enforced - by `tests/test_errors.py::test_keyword_reduce_classes_dict_mirrors_their_init_parameters`. Adding - a class means picking a side, and both sides are checked. + keyword-only. Pickling and copying go through one `__reduce__` on `ClientError`: it rebuilds the + instance from `args` without calling `__init__` and restores `__dict__`, so a new class needs no + pickling code, only picklable attributes. Adding a class means picking a construction side and + adding a case to `_ERROR_FACTORIES` in `tests/test_errors.py`, which an invariant test enforces + for every exported error class. - `decoders/_resolver.py` is the **single dispatch path** for Seam B: `_DecoderResolver.resolve` walks the frozen decoder tuple, raises `MissingDecoderError` *before* the HTTP call when nothing claims the model, and returns a `_BoundDecoder` whose `decode` wraps any decoder-side failure as diff --git a/src/httpware/errors.py b/src/httpware/errors.py index 6e0ae4a..7dde2e0 100644 --- a/src/httpware/errors.py +++ b/src/httpware/errors.py @@ -22,8 +22,20 @@ from httpware._internal.redaction import redact_url +def _restore_client_error(cls: "type[ClientError]", args: tuple[Any, ...]) -> "ClientError": + return cls.__new__(cls, *args) + + class ClientError(Exception): - """Root of the httpware exception tree.""" + """Root of the httpware exception tree. + + Pickle, ``copy.copy`` and ``copy.deepcopy`` rebuild every subclass from its + ``args`` without calling ``__init__``, then restore the instance ``__dict__``, + so notes and attributes set after construction survive the round trip. + """ + + def __reduce__(self) -> tuple[Any, ...]: + return (_restore_client_error, (type(self), self.args), self.__dict__) class TransportError(ClientError): @@ -48,10 +60,6 @@ class TimeoutError(ClientError, builtins.TimeoutError): # noqa: A001 """ -def _reconstruct_status_error(cls: "type[StatusError]", response: httpx2.Response) -> "StatusError": - return cls(response) - - class StatusError(ClientError): """Base for HTTP-status-keyed errors. @@ -75,9 +83,6 @@ def __repr__(self) -> str: url = redact_url(str(self.response.request.url)) return f"<{cls_name} status={self.response.status_code} method={method} url={url}>" - def __reduce__(self) -> tuple[Any, ...]: - return (_reconstruct_status_error, (type(self), self.response)) - class ClientStatusError(StatusError): """Base for 4xx HTTP status errors.""" @@ -136,27 +141,7 @@ class ServiceUnavailableError(ServerStatusError): } -def _reconstruct_kwonly(cls: type, kwargs: dict[str, Any]) -> Any: # noqa: ANN401 - return cls(**kwargs) - - -class _KeywordReduceMixin: - """Shared __reduce__ for keyword-only ClientError subclasses. - - For subclasses whose __init__ is keyword-only and whose instance - __dict__ exactly mirrors it. Reconstruction replays self.__dict__ as - keyword arguments (cls(**kwargs)): an attribute stored beyond - __init__'s keyword parameters raises TypeError on unpickle (unexpected - keyword argument); a keyword parameter __init__ doesn't assign to self - is silently dropped if it has a default (unpickle reverts to it) or - raises TypeError if it doesn't (missing required argument). - """ - - def __reduce__(self) -> tuple[Any, ...]: - return (_reconstruct_kwonly, (type(self), self.__dict__)) - - -class RetryBudgetExhaustedError(_KeywordReduceMixin, ClientError): +class RetryBudgetExhaustedError(ClientError): """Raised when a retry was needed but the RetryBudget refused to permit it. Carries the last response and/or exception observed before the budget refused, @@ -180,7 +165,7 @@ def __init__( super().__init__(f"retry budget exhausted after {attempts} attempt(s)") -class BulkheadFullError(_KeywordReduceMixin, ClientError): +class BulkheadFullError(ClientError): """Raised when ``acquire_timeout`` elapses before a bulkhead slot becomes available. Carries the configured caps for caller logging/alerting. @@ -195,7 +180,7 @@ def __init__(self, *, max_concurrent: int, acquire_timeout: float | None) -> Non super().__init__(f"bulkhead full (max_concurrent={max_concurrent}, acquire_timeout={acquire_timeout})") -class CircuitOpenError(_KeywordReduceMixin, ClientError): +class CircuitOpenError(ClientError): """Raised when a CircuitBreaker refuses a request because the circuit is not closed. Fires when the circuit is OPEN, or when it is HALF_OPEN and the single probe @@ -214,7 +199,7 @@ def __init__(self, *, retry_after: float | None) -> None: super().__init__(f"circuit open (retry_after={retry_after:.3f}s)") -class DecodeError(_KeywordReduceMixin, ClientError): +class DecodeError(ClientError): """Raised when the active ResponseDecoder failed to decode response.content. The HTTP call itself succeeded — status was 2xx/3xx and the transport @@ -253,7 +238,7 @@ def _missing_decoder_summary(model: type, registered_names: tuple[str, ...]) -> return f"no decoder for response_model={model!r}: {hint}" -class MissingDecoderError(_KeywordReduceMixin, ClientError): +class MissingDecoderError(ClientError): """Raised when response_model= is set but no registered decoder claims the model. Fires at .send() entry, BEFORE the HTTP call — no point sending a request @@ -270,7 +255,7 @@ def __init__(self, *, model: type, registered_names: tuple[str, ...]) -> None: super().__init__(_missing_decoder_summary(model, registered_names)) -class ResponseTooLargeError(_KeywordReduceMixin, ClientError): +class ResponseTooLargeError(ClientError): """Raised when a response body exceeds the client's max_response_body_bytes cap. Status-agnostic: fires on any non-streaming send() and on stream()'s internal diff --git a/tests/test_errors.py b/tests/test_errors.py index b15f44e..4756d5c 100644 --- a/tests/test_errors.py +++ b/tests/test_errors.py @@ -1,14 +1,16 @@ """Tests for the status-keyed exception tree in httpware.errors.""" import builtins -import inspect +import copy import pickle +from collections.abc import Callable from http import HTTPStatus import httpx2 import pydantic import pytest +import httpware from httpware.errors import ( STATUS_TO_EXCEPTION, BadRequestError, @@ -33,7 +35,6 @@ TransportError, UnauthorizedError, UnprocessableEntityError, - _KeywordReduceMixin, ) @@ -470,45 +471,61 @@ def test_response_too_large_error_pickle_round_trip() -> None: assert restored.reason == "streamed" -def _keyword_reduce_instances() -> tuple[ClientError, ...]: - return ( - RetryBudgetExhaustedError(last_response=None, last_exception=None, attempts=3), - BulkheadFullError(max_concurrent=1, acquire_timeout=None), - CircuitOpenError(retry_after=None), - DecodeError(response=_make_response(200), model=_DecodeErrorModel, original=ValueError("x")), - MissingDecoderError(model=_Foo, registered_names=()), - ResponseTooLargeError(status_code=200, limit=10, content_length=None, reason="streamed"), - ) +_ERROR_FACTORIES: tuple[Callable[[], ClientError], ...] = ( + lambda: ClientError("root"), + lambda: TransportError("connection refused"), + lambda: NetworkError("connection reset"), + lambda: TimeoutError("timed out"), + lambda: StatusError(_make_response(418)), + lambda: ClientStatusError(_make_response(418)), + lambda: ServerStatusError(_make_response(502)), + lambda: BadRequestError(_make_response(400)), + lambda: UnauthorizedError(_make_response(401)), + lambda: ForbiddenError(_make_response(403)), + lambda: NotFoundError(_make_response(404)), + lambda: ConflictError(_make_response(409)), + lambda: UnprocessableEntityError(_make_response(422)), + lambda: RateLimitedError(_make_response(429)), + lambda: InternalServerError(_make_response(500)), + lambda: ServiceUnavailableError(_make_response(503)), + lambda: RetryBudgetExhaustedError(last_response=_make_response(503), last_exception=None, attempts=3), + lambda: BulkheadFullError(max_concurrent=1, acquire_timeout=None), + lambda: CircuitOpenError(retry_after=None), + lambda: DecodeError(response=_make_response(200), model=_DecodeErrorModel, original=ValueError("x")), + lambda: MissingDecoderError(model=_Foo, registered_names=()), + lambda: ResponseTooLargeError(status_code=200, limit=10, content_length=None, reason="streamed"), +) + + +def _pickle_round_trip(exc: ClientError) -> ClientError: + return pickle.loads(pickle.dumps(exc)) # noqa: S301 — round-tripping our own exception -def test_every_keyword_reduce_class_has_a_case_here() -> None: - """INVARIANT: no class carries _KeywordReduceMixin without its precondition being checked. - - The mixin is safe only for classes whose instance __dict__ mirrors their keyword-only - __init__, and that is a property of each class rather than of the mixin. A seventh class - added to the tree would otherwise inherit __reduce__ and inherit nothing that checks it. - A root-level __reduce__ on ClientError was rejected for the same reason: the argless - subclasses keep their message in args and have an empty __dict__, so replaying it would - silently drop the message. Enumerating the subclasses here makes adding one an edit - somebody has to make. - """ - assert {type(exc) for exc in _keyword_reduce_instances()} == set(_KeywordReduceMixin.__subclasses__()) - - -@pytest.mark.parametrize("exc", _keyword_reduce_instances(), ids=lambda e: type(e).__name__) -def test_keyword_reduce_classes_dict_mirrors_their_init_parameters(exc: ClientError) -> None: - """INVARIANT: a _KeywordReduceMixin instance's __dict__ is exactly its __init__ keywords. - - __reduce__ replays the instance __dict__ as keyword arguments, so the two drift apart - silently: storing a derived attribute alongside the declared fields, or accepting a - keyword that __init__ consumes without assigning, both leave a class that constructs, - reprs and raises perfectly well. The damage appears only in a process that unpickles the - exception — a Celery worker or a multiprocessing pool — which is rarely the process the - suite runs in, and never the one the author is looking at. - """ - keywords = { - name - for name, parameter in inspect.signature(type(exc).__init__).parameters.items() - if parameter.kind is inspect.Parameter.KEYWORD_ONLY +def test_every_exported_error_class_has_a_round_trip_case() -> None: + """INVARIANT: every exported ClientError class is covered by the copy round-trip below.""" + exported = { + obj + for name in httpware.__all__ + if isinstance(obj := getattr(httpware, name), type) and issubclass(obj, ClientError) } - assert set(vars(exc)) == keywords + assert {type(factory()) for factory in _ERROR_FACTORIES} == exported + + +@pytest.mark.parametrize("copier", [_pickle_round_trip, copy.copy, copy.deepcopy], ids=["pickle", "copy", "deepcopy"]) +@pytest.mark.parametrize("factory", _ERROR_FACTORIES, ids=lambda factory: type(factory()).__name__) +def test_error_round_trip_keeps_notes_and_attributes( + factory: Callable[[], ClientError], + copier: Callable[[ClientError], ClientError], +) -> None: + exc = factory() + exc.add_note("request id abc") + vars(exc)["tenant"] = "acme" + + restored = copier(exc) + + assert type(restored) is type(exc) + assert restored.args == exc.args + assert str(restored) == str(exc) + assert restored.__notes__ == ["request id abc"] + assert vars(restored)["tenant"] == "acme" + assert set(vars(restored)) == set(vars(exc)) From 330d2d2d2e3446bbc4bc930b36efd4cc6797b1ab Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sat, 17 Oct 2026 11:35:45 +0300 Subject: [PATCH 2/6] fix!: make STATUS_TO_EXCEPTION read-only STATUS_TO_EXCEPTION is now a types.MappingProxyType. Code that assigned to it to remap a status now gets TypeError; catch the error in a middleware and raise your own class instead. --- docs/errors.md | 2 +- src/httpware/errors.py | 25 ++++++++++++++----------- tests/test_errors.py | 5 +++++ 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/docs/errors.md b/docs/errors.md index cb7778b..cefaf63 100644 --- a/docs/errors.md +++ b/docs/errors.md @@ -49,7 +49,7 @@ ClientError (catch-all for anything httpware raises) Only statuses from 400 to 599 raise. Any other status returns the response unchanged. -The numbered rows are exported as `STATUS_TO_EXCEPTION`, a `Mapping[int, type[StatusError]]`, so you can look up a class in code with `STATUS_TO_EXCEPTION.get(404)`. The two fallback rows are not in the mapping. +The numbered rows are exported as `STATUS_TO_EXCEPTION`, a `Mapping[int, type[StatusError]]`, so you can look up a class in code with `STATUS_TO_EXCEPTION.get(404)`. The two fallback rows are not in the mapping. The mapping is read-only, and assigning to it raises `TypeError`. To raise your own class for a status, catch the httpware error in a middleware and raise yours from it. ## Catching strategies diff --git a/src/httpware/errors.py b/src/httpware/errors.py index 7dde2e0..c0d32c0 100644 --- a/src/httpware/errors.py +++ b/src/httpware/errors.py @@ -14,6 +14,7 @@ """ import builtins +import types from collections.abc import Mapping from typing import Any, Literal @@ -128,17 +129,19 @@ class ServiceUnavailableError(ServerStatusError): """HTTP 503.""" -STATUS_TO_EXCEPTION: Mapping[int, type[StatusError]] = { - 400: BadRequestError, - 401: UnauthorizedError, - 403: ForbiddenError, - 404: NotFoundError, - 409: ConflictError, - 422: UnprocessableEntityError, - 429: RateLimitedError, - 500: InternalServerError, - 503: ServiceUnavailableError, -} +STATUS_TO_EXCEPTION: Mapping[int, type[StatusError]] = types.MappingProxyType( + { + 400: BadRequestError, + 401: UnauthorizedError, + 403: ForbiddenError, + 404: NotFoundError, + 409: ConflictError, + 422: UnprocessableEntityError, + 429: RateLimitedError, + 500: InternalServerError, + 503: ServiceUnavailableError, + } +) class RetryBudgetExhaustedError(ClientError): diff --git a/tests/test_errors.py b/tests/test_errors.py index 4756d5c..834fd6d 100644 --- a/tests/test_errors.py +++ b/tests/test_errors.py @@ -83,6 +83,11 @@ def test_status_to_exception_table() -> None: } == STATUS_TO_EXCEPTION +def test_status_to_exception_cannot_be_mutated() -> None: + with pytest.raises(TypeError): + STATUS_TO_EXCEPTION[418] = BadRequestError # ty: ignore[invalid-assignment] + + def test_status_error_stores_response() -> None: response = _make_response(404) exc = NotFoundError(response) From b80a33948aee798ab5cb7a13b543dbaee270f61a Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sat, 17 Oct 2026 11:37:39 +0300 Subject: [PATCH 3/6] fix: mask every query value in error messages and events redact_url now replaces the value of every query and fragment parameter with REDACTED and keeps the keys, instead of masking an allowlist of sensitive names, so presigned-URL signatures and other unlisted secrets no longer reach messages, reprs, log records or span events. Userinfo is cut at the last @ so a raw password containing @ is stripped whole. --- docs/errors.md | 2 +- docs/observability.md | 2 + src/httpware/_internal/redaction.py | 99 +++++++---------------------- src/httpware/errors.py | 4 +- tests/test_errors.py | 17 +++-- tests/test_observability.py | 11 ++-- tests/test_redaction.py | 65 +++++++------------ 7 files changed, 62 insertions(+), 138 deletions(-) diff --git a/docs/errors.md b/docs/errors.md index cefaf63..f593f88 100644 --- a/docs/errors.md +++ b/docs/errors.md @@ -112,7 +112,7 @@ exc.response.request.url # the failing URL (httpx2.URL) exc.response.request.method # the HTTP method ``` -The exception's `repr` and message strip `user:pass@` userinfo and mask the values of known-sensitive query and URL-fragment parameters (`api_key`, `apikey`, `access_token`, `refresh_token`, `token`, `secret`, `client_secret`, `password`, `passwd`, `pwd`, `auth`, `authorization`, `sig`, `signature`, `key`, `private_key`, `session`, `sessionid`, `x-api-key`) as `REDACTED`, keeping the keys. Values under other names are left as they are, so avoid putting other secrets in query strings. Request headers such as `Authorization` and `Cookie` are never redacted, and `exc.response.request.headers` returns them in full. +The exception's `repr` and message strip `user:pass@` userinfo and replace every query-parameter value with `REDACTED`, keeping the keys, so `?page=2&X-Amz-Signature=abc` shows as `?page=REDACTED&X-Amz-Signature=REDACTED`. Fragment values get the same treatment, and a fragment without `=`, like `#section-3`, is kept. The unredacted URL is still on `exc.response.request.url`. Request headers such as `Authorization` and `Cookie` are never redacted, and `exc.response.request.headers` returns them in full. ## Resilience-error payloads diff --git a/docs/observability.md b/docs/observability.md index 7d532f6..05abf2b 100644 --- a/docs/observability.md +++ b/docs/observability.md @@ -13,6 +13,8 @@ The logger and event names below are a stable public contract: Each log record has an `event` field holding the event name, such as `event="circuit.opened"`, which you can filter on in your log aggregator. Events from `AsyncKeyedCircuitBreaker` and `KeyedCircuitBreaker` also carry `circuit_key`, the circuit they belong to. [Resilience](resilience.md) describes when each event fires. +Events that carry a `url` attribute get it redacted first: httpware strips `user:pass@` userinfo and replaces every query-parameter value with `REDACTED`, keeping the keys. A URL logged as `https://api.test/items?page=REDACTED&sort=REDACTED` came from `https://api.test/items?page=2&sort=name`. + ```python import logging diff --git a/src/httpware/_internal/redaction.py b/src/httpware/_internal/redaction.py index 1192d7f..b06dc9f 100644 --- a/src/httpware/_internal/redaction.py +++ b/src/httpware/_internal/redaction.py @@ -1,50 +1,19 @@ """URL sanitation for logs, telemetry, and error messages. -Strips ``user:pass@`` userinfo and masks the values of known-sensitive query -parameters so secrets embedded in URLs do not leak into observability output. -Shared by ``errors.py`` (StatusError messages) and the resilience middleware -(event attributes). +Strips ``user:pass@`` userinfo and masks every query and fragment parameter +value, keeping the keys, so secrets embedded in URLs do not leak into +observability output. Shared by ``errors.py`` (StatusError messages) and +``observability._emit_event`` (event attributes). """ -from urllib.parse import parse_qsl, urlencode, urlsplit, urlunsplit +from urllib.parse import urlsplit, urlunsplit -SENSITIVE_QUERY_KEYS = frozenset( - { - "api_key", - "apikey", - "access_token", - "refresh_token", - "token", - "secret", - "client_secret", - "password", - "passwd", - "pwd", - "auth", - "authorization", - "sig", - "signature", - "key", - "private_key", - "session", - "sessionid", - "x-api-key", - } -) - _REDACTED = "REDACTED" def _reassemble(scheme: str, netloc: str, path: str, query: str, fragment: str) -> str: - """Like ``urlunsplit``, but avoid the spurious triple-slash for an empty authority. - - ``urlunsplit(("http", "", "/path", ...))`` yields ``http:///path`` for a - netloc-using scheme. When userinfo stripping leaves no host (e.g. - ``http://user:pass@/path``) we want ``http:/path`` (scheme + path), not a - triple-slash. With a non-empty netloc this delegates to ``urlunsplit``, so - normal URLs are byte-identical. - """ + """Like ``urlunsplit``, but return ``scheme:path`` rather than ``scheme:///path`` for an empty authority.""" if netloc: return urlunsplit((scheme, netloc, path, query, fragment)) tail = path @@ -61,56 +30,32 @@ def _strip_userinfo(url: str) -> str: parts = urlsplit(url) if parts.username is None and parts.password is None: return url - # Strip the "user:pass@" prefix from the raw netloc to preserve host:port - # exactly (including IPv6 brackets), rather than reconstructing from parts. - netloc = parts.netloc.split("@", 1)[1] if "@" in parts.netloc else parts.netloc + netloc = parts.netloc.rpartition("@")[2] return _reassemble(parts.scheme, netloc, parts.path, parts.query, parts.fragment) -def _mask_component(component: str) -> tuple[str, bool]: - """Mask sensitive key=value pairs in a query or fragment string. - - Returns ``(masked_component, was_changed)``; when no sensitive key is - found the original string is returned unchanged (``was_changed=False``). - """ - pairs = parse_qsl(component, keep_blank_values=True) - if not any(key.strip().lower() in SENSITIVE_QUERY_KEYS for key, _ in pairs): - return component, False - masked = [(key, _REDACTED if key.strip().lower() in SENSITIVE_QUERY_KEYS else value) for key, value in pairs] - return urlencode(masked), True +def _mask_component(component: str) -> str: + """Replace the value of every ``key=value`` pair with ``REDACTED``; a component without ``=`` is kept.""" + if "=" not in component: + return component + return "&".join( + key + "=" + _REDACTED if sep else key for key, sep, _ in (pair.partition("=") for pair in component.split("&")) + ) def _mask_query(url: str) -> str: - parts = urlsplit(url) - has_query = bool(parts.query) - has_fragment = bool(parts.fragment) - - if not has_query and not has_fragment: + if "=" not in url: return url - - new_query = parts.query - new_fragment = parts.fragment - changed = False - - if has_query: - new_query, q_changed = _mask_component(parts.query) - changed = changed or q_changed - - if has_fragment: - new_fragment, f_changed = _mask_component(parts.fragment) - changed = changed or f_changed - - if not changed: - return url # common-path guard: nothing sensitive, leave bytes untouched - - return _reassemble(parts.scheme, parts.netloc, parts.path, new_query, new_fragment) + rest, hash_sign, fragment = url.partition("#") + base, question_mark, query = rest.partition("?") + return base + question_mark + _mask_component(query) + hash_sign + _mask_component(fragment) def redact_url(url: str) -> str: - """Return ``url`` safe for logs/telemetry/errors. + """Return ``url`` safe for logs, telemetry and error messages. - Userinfo is stripped and the values of known-sensitive query parameters are - replaced with ``REDACTED`` (keys preserved). URLs with no sensitive query - key are returned byte-identical to the userinfo-stripped input. + Userinfo is stripped and every query and fragment parameter value is + replaced with ``REDACTED``, keeping the keys as written. A fragment with no + ``=`` in it, such as ``#section-3``, is kept as it is. """ return _mask_query(_strip_userinfo(url)) diff --git a/src/httpware/errors.py b/src/httpware/errors.py index c0d32c0..ec7bcfe 100644 --- a/src/httpware/errors.py +++ b/src/httpware/errors.py @@ -7,8 +7,8 @@ The fallback assumes 400 <= status < 600. __repr__ and the summary message run response.request.url through -_internal.redaction.redact_url, which strips user:pass@ userinfo and masks the -values of known-sensitive query parameters. NOTE: the full request headers +_internal.redaction.redact_url, which strips user:pass@ userinfo and masks +every query parameter value. NOTE: the full request headers (Authorization, Cookie, ...) remain reachable via exc.response.request — handler authors must redact those before logging. """ diff --git a/tests/test_errors.py b/tests/test_errors.py index 834fd6d..995173b 100644 --- a/tests/test_errors.py +++ b/tests/test_errors.py @@ -188,10 +188,9 @@ def test_status_error_summary_preserves_port() -> None: assert str(exc) == "404 GET https://example.test:8080/x" -def test_status_error_summary_passthrough_when_at_in_query_only() -> None: - # `@` in query-string with no userinfo — should fall through after urlsplit returns no user/pass. +def test_status_error_summary_masks_at_sign_in_query_value() -> None: exc = NotFoundError(_make_response(404, url="https://example.test/x?email=foo@bar.com")) - assert str(exc) == "404 GET https://example.test/x?email=foo@bar.com" + assert str(exc) == "404 GET https://example.test/x?email=REDACTED" def test_status_error_strips_userinfo_with_ipv6_host() -> None: @@ -430,14 +429,14 @@ def test_circuit_open_error_pickleable_with_none() -> None: assert restored.retry_after is None -def test_status_error_message_masks_query_secret() -> None: - request = httpx2.Request("GET", "https://example.test/p?api_key=topsecret&page=2") +def test_status_error_message_masks_every_query_value() -> None: + request = httpx2.Request("GET", "https://example.test/p?api_key=topsecret&page=2&X-Amz-Signature=sig") response = httpx2.Response(404, request=request) exc = NotFoundError(response) - assert "topsecret" not in str(exc) - assert "api_key=REDACTED" in str(exc) - assert "page=2" in str(exc) - assert "topsecret" not in repr(exc) + expected_url = "https://example.test/p?api_key=REDACTED&page=REDACTED&X-Amz-Signature=REDACTED" + assert str(exc) == f"404 GET {expected_url}" + assert repr(exc) == f"" + assert "X-Amz-Signature=sig" in str(exc.response.request.url) def test_response_too_large_error_fields_and_message() -> None: diff --git a/tests/test_observability.py b/tests/test_observability.py index 9c3fb03..f4e6f7c 100644 --- a/tests/test_observability.py +++ b/tests/test_observability.py @@ -40,13 +40,11 @@ def test_emit_event_redacts_url_secret_in_log_record(caplog: pytest.LogCaptureFi "test.event", level=logging.WARNING, message="leaky", - attributes={"url": "https://u:p@example.test/x?api_key=topsecret"}, + attributes={"url": "https://u:p@example.test/x?api_key=topsecret&X-Amz-Signature=sig"}, ) record = caplog.records[0] - assert "topsecret" not in record.url # ty: ignore[unresolved-attribute] - assert "api_key=REDACTED" in record.url # ty: ignore[unresolved-attribute] - assert "u:p@" not in record.url # ty: ignore[unresolved-attribute] + assert record.url == "https://example.test/x?api_key=REDACTED&X-Amz-Signature=REDACTED" # ty: ignore[unresolved-attribute] def test_emit_event_redacts_url_secret_in_otel_event() -> None: @@ -61,12 +59,11 @@ def test_emit_event_redacts_url_secret_in_otel_event() -> None: "test.event", level=logging.WARNING, message="leaky", - attributes={"url": "https://example.test/x?token=topsecret"}, + attributes={"url": "https://example.test/x?token=topsecret&X-Amz-Signature=sig"}, ) _, kwargs = mock_span.add_event.call_args - assert "topsecret" not in kwargs["attributes"]["url"] - assert "token=REDACTED" in kwargs["attributes"]["url"] + assert kwargs["attributes"]["url"] == "https://example.test/x?token=REDACTED&X-Amz-Signature=REDACTED" def test_emit_event_respects_level_parameter(caplog: pytest.LogCaptureFixture) -> None: diff --git a/tests/test_redaction.py b/tests/test_redaction.py index 012f9c3..fef42c8 100644 --- a/tests/test_redaction.py +++ b/tests/test_redaction.py @@ -8,21 +8,23 @@ @pytest.mark.parametrize( ("url", "expected"), [ - # no-op cases (common-path guard: bytes unchanged) ("https://example.test/path", "https://example.test/path"), - ("https://example.test/path?page=2&limit=10", "https://example.test/path?page=2&limit=10"), ("not-a-url", "not-a-url"), ("https://example.test", "https://example.test"), - # userinfo stripped ("https://user:pass@example.test/p", "https://example.test/p"), ("https://user:pass@example.test:8443/p", "https://example.test:8443/p"), ("https://user:pass@[2001:db8::1]:8443/p", "https://[2001:db8::1]:8443/p"), - # sensitive query value masked, key + other params preserved + ("https://user:p@ss@example.test/p", "https://example.test/p"), ("https://example.test/p?api_key=abc123", "https://example.test/p?api_key=REDACTED"), - ("https://example.test/p?page=2&access_token=xyz", "https://example.test/p?page=2&access_token=REDACTED"), - # case-insensitive key match + ("https://example.test/p?page=2&limit=10", "https://example.test/p?page=REDACTED&limit=REDACTED"), + ( + "https://bucket.test/k?X-Amz-Credential=AKIA%2F1&X-Amz-Signature=deadbeef", + "https://bucket.test/k?X-Amz-Credential=REDACTED&X-Amz-Signature=REDACTED", + ), ("https://example.test/p?API_KEY=abc", "https://example.test/p?API_KEY=REDACTED"), - # userinfo AND query both handled + ("https://example.test/p?sig=abc==", "https://example.test/p?sig=REDACTED"), + ("https://example.test/p?%20api_key=topsecret", "https://example.test/p?%20api_key=REDACTED"), + ("https://example.test/p?flag&page=2", "https://example.test/p?flag&page=REDACTED"), ("https://u:p@example.test/p?token=t", "https://example.test/p?token=REDACTED"), ], ) @@ -30,56 +32,35 @@ def test_redact_url(url: str, expected: str) -> None: assert redact_url(url) == expected -def test_redact_url_masks_repeated_sensitive_keys() -> None: - result = redact_url("https://example.test/p?token=a&token=b&page=1") - assert "token=a" not in result - assert "token=b" not in result - assert result.count("token=REDACTED") == 2 # noqa: PLR2004 — two token params above - assert "page=1" in result +def test_redact_url_masks_repeated_keys() -> None: + assert redact_url("https://example.test/p?token=a&token=b&page=1") == ( + "https://example.test/p?token=REDACTED&token=REDACTED&page=REDACTED" + ) -def test_redact_url_masks_blank_sensitive_value() -> None: +def test_redact_url_masks_blank_value() -> None: assert redact_url("https://example.test/p?secret=") == "https://example.test/p?secret=REDACTED" -def test_redact_url_masks_fragment_secret() -> None: - result = redact_url("https://example.test/p?page=2#access_token=topsecret") - assert "topsecret" not in result - assert "access_token=REDACTED" in result - assert "page=2" in result +def test_redact_url_masks_fragment_values() -> None: + assert redact_url("https://example.test/p?page=2#access_token=topsecret&state=xyz") == ( + "https://example.test/p?page=REDACTED#access_token=REDACTED&state=REDACTED" + ) def test_redact_url_preserves_benign_fragment() -> None: assert redact_url("https://example.test/p#section-3") == "https://example.test/p#section-3" -def test_redact_url_masks_whitespace_padded_key() -> None: - # a space-padded sensitive key name must still be masked - result = redact_url("https://example.test/p?%20api_key=topsecret") - assert "topsecret" not in result - assert "REDACTED" in result - - def test_strip_userinfo_no_hostname_does_not_produce_triple_slash() -> None: """http://user:pass@/path must strip creds without yielding http:///path.""" - result = redact_url("http://user:pass@/path") - assert "user" not in result - assert "pass" not in result - assert ":///" not in result # must not be triple-slash - assert result == "http:/path" # no authority, just scheme + path + assert redact_url("http://user:pass@/path") == "http:/path" -def test_redact_url_at_sign_in_path_without_userinfo_is_unchanged() -> None: - """An `@` in the path (not credentials) leaves the URL untouched.""" - assert redact_url("https://example.test/@handle?page=2") == "https://example.test/@handle?page=2" +def test_redact_url_at_sign_in_path_without_userinfo_keeps_path() -> None: + assert redact_url("https://example.test/@handle?page=2") == "https://example.test/@handle?page=REDACTED" def test_strip_userinfo_no_hostname_preserves_query_and_fragment() -> None: - """The no-authority reconstruction keeps the query (masked) and fragment, with no triple-slash.""" - result = redact_url("http://user:pass@/path?api_key=secret#section") - assert ":///" not in result - assert "user" not in result - assert "pass" not in result - assert "secret" not in result - assert "api_key=REDACTED" in result - assert "#section" in result + """The no-authority reconstruction keeps the masked query and the fragment, with no triple-slash.""" + assert redact_url("http://user:pass@/path?api_key=secret#section") == "http:/path?api_key=REDACTED#section" From edfe82baa4741f89a2e6b4ac9eabf0de565fc918 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sat, 17 Oct 2026 11:39:08 +0300 Subject: [PATCH 4/6] fix: keep body content out of the DecodeError message The message is now 'failed to decode response into : ' with the error count appended when the original exception has error_count(). A non-class model such as X | None is shown by repr, which stops the AttributeError on Python 3.11 to 3.13. exc.original is unchanged. --- docs/errors.md | 13 +++++++------ src/httpware/errors.py | 16 ++++++++++++++-- tests/test_errors.py | 42 ++++++++++++++++++++++++++++++++++-------- 3 files changed, 55 insertions(+), 16 deletions(-) diff --git a/docs/errors.md b/docs/errors.md index f593f88..e4f09f0 100644 --- a/docs/errors.md +++ b/docs/errors.md @@ -149,6 +149,12 @@ Fields: - `model: type`: the type passed as `response_model=`. - `original: BaseException`: the decoder's own exception, such as `pydantic.ValidationError`, `msgspec.ValidationError` or `msgspec.DecodeError`. It is also `exc.__cause__`. +The message names the model and the type of `original`, and adds the error count when `original` has an `error_count()` method, as `pydantic.ValidationError` does: + + failed to decode response into User: ValidationError (2 errors) + +A model that is not a class, such as `User | None` or `list[User]`, appears as its `repr`. The message never quotes the body. `str(exc.original)` can: a pydantic `ValidationError` echoes the input values it rejected, so check what it holds before you log it. + ```python from httpware import AsyncClient, DecodeError @@ -156,12 +162,7 @@ from httpware import AsyncClient, DecodeError try: user = await client.get("/users/1", response_model=User) except DecodeError as exc: - _LOGGER.error( - "decode failed for %s into %s: %s", - exc.response.request.url, - exc.model.__name__, - exc.original, - ) + _LOGGER.error("%s", exc) raise ``` diff --git a/src/httpware/errors.py b/src/httpware/errors.py index ec7bcfe..123711b 100644 --- a/src/httpware/errors.py +++ b/src/httpware/errors.py @@ -202,6 +202,16 @@ def __init__(self, *, retry_after: float | None) -> None: super().__init__(f"circuit open (retry_after={retry_after:.3f}s)") +def _decode_error_summary(model: type, original: BaseException) -> str: + model_name = model.__name__ if isinstance(model, type) else repr(model) + summary = f"failed to decode response into {model_name}: {type(original).__qualname__}" + error_count = getattr(original, "error_count", None) + if not callable(error_count): + return summary + count = error_count() + return f"{summary} ({count} error{'' if count == 1 else 's'})" + + class DecodeError(ClientError): """Raised when the active ResponseDecoder failed to decode response.content. @@ -209,7 +219,9 @@ class DecodeError(ClientError): delivered the body intact — but the body could not be parsed into the requested response_model. Always chained from the underlying library exception via ``raise ... from exc``; that exception is also exposed as - ``self.original`` for structured handling. + ``self.original`` for structured handling. The message names the model + and the original exception's type, plus its error count when it has an + ``error_count()`` method (pydantic), and never includes body content. """ response: httpx2.Response @@ -226,7 +238,7 @@ def __init__( self.response = response self.model = model self.original = original - super().__init__(f"failed to decode response into {model.__name__}: {original}") + super().__init__(_decode_error_summary(model, original)) def _missing_decoder_summary(model: type, registered_names: tuple[str, ...]) -> str: diff --git a/tests/test_errors.py b/tests/test_errors.py index 995173b..abe98e0 100644 --- a/tests/test_errors.py +++ b/tests/test_errors.py @@ -298,14 +298,40 @@ def test_decode_error_stores_fields() -> None: assert exc.original is inner -def test_decode_error_summary_includes_model_and_original() -> None: - response = _make_response(200) - inner = ValueError("bad payload") - exc = DecodeError(response=response, model=_DecodeErrorModel, original=inner) - summary = str(exc) - assert "_DecodeErrorModel" in summary - assert "bad payload" in summary - assert summary.startswith("failed to decode response into ") +def _validation_error(payload: bytes) -> pydantic.ValidationError: + with pytest.raises(pydantic.ValidationError) as caught: + _DecodeErrorModel.model_validate_json(payload) + return caught.value + + +def test_decode_error_summary_names_model_and_exception_type_only() -> None: + exc = DecodeError(response=_make_response(200), model=_DecodeErrorModel, original=ValueError("body: topsecret")) + assert str(exc) == "failed to decode response into _DecodeErrorModel: ValueError" + + +def test_decode_error_summary_counts_validation_errors_without_body_content() -> None: + original = _validation_error(b'{"id": "topsecret", "extra": 1}') + exc = DecodeError(response=_make_response(200), model=_DecodeErrorModel, original=original) + assert str(exc) == "failed to decode response into _DecodeErrorModel: ValidationError (1 error)" + assert exc.original is original + + +class _TwoFieldModel(pydantic.BaseModel): + id: int + name: str + + +def test_decode_error_summary_pluralises_error_count() -> None: + with pytest.raises(pydantic.ValidationError) as caught: + _TwoFieldModel.model_validate_json(b'{"id": "topsecret", "name": 3}') + exc = DecodeError(response=_make_response(200), model=_TwoFieldModel, original=caught.value) + assert str(exc) == "failed to decode response into _TwoFieldModel: ValidationError (2 errors)" + + +@pytest.mark.parametrize("model", [_DecodeErrorModel | None, list[_DecodeErrorModel]], ids=["optional", "generic"]) +def test_decode_error_summary_uses_repr_for_non_class_models(model: type) -> None: + exc = DecodeError(response=_make_response(200), model=model, original=ValueError("x")) + assert str(exc) == f"failed to decode response into {model!r}: ValueError" def test_decode_error_rejects_positional_args() -> None: From 6474b57f79f55f91fd64864dfc772d1bfb40e9e4 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sat, 17 Oct 2026 11:39:43 +0300 Subject: [PATCH 5/6] fix: let phase decorator reprs handle callables without __qualname__ repr() of a middleware built by before_request, after_response, on_error or their async siblings no longer raises AttributeError for a functools.partial or a callable instance; it falls back to repr(f). --- src/httpware/middleware/__init__.py | 12 +++++----- tests/test_middleware.py | 33 ++++++++++++++++++++++++++++ tests/test_middleware_sync.py | 34 +++++++++++++++++++++++++++++ 3 files changed, 73 insertions(+), 6 deletions(-) diff --git a/src/httpware/middleware/__init__.py b/src/httpware/middleware/__init__.py index 80cbbd5..7e468cd 100644 --- a/src/httpware/middleware/__init__.py +++ b/src/httpware/middleware/__init__.py @@ -44,7 +44,7 @@ async def __call__(self, request: httpx2.Request, next: AsyncNext) -> httpx2.Res return await next(await f(request)) def __repr__(self) -> str: - return f"" # ty: ignore[unresolved-attribute] + return f"" return _BeforeRequestMiddleware() @@ -60,7 +60,7 @@ async def __call__(self, request: httpx2.Request, next: AsyncNext) -> httpx2.Res return await f(request, response) def __repr__(self) -> str: - return f"" # ty: ignore[unresolved-attribute] + return f"" return _AfterResponseMiddleware() @@ -85,7 +85,7 @@ async def __call__(self, request: httpx2.Request, next: AsyncNext) -> httpx2.Res return result def __repr__(self) -> str: - return f"" # ty: ignore[unresolved-attribute] + return f"" return _OnErrorMiddleware() @@ -110,7 +110,7 @@ def __call__(self, request: httpx2.Request, next: Next) -> httpx2.Response: # n return next(f(request)) def __repr__(self) -> str: - return f"" # ty: ignore[unresolved-attribute] + return f"" return _BeforeRequestMiddleware() @@ -126,7 +126,7 @@ def __call__(self, request: httpx2.Request, next: Next) -> httpx2.Response: # n return f(request, response) def __repr__(self) -> str: - return f"" # ty: ignore[unresolved-attribute] + return f"" return _AfterResponseMiddleware() @@ -151,6 +151,6 @@ def __call__(self, request: httpx2.Request, next: Next) -> httpx2.Response: # n return result def __repr__(self) -> str: - return f"" # ty: ignore[unresolved-attribute] + return f"" return _OnErrorMiddleware() diff --git a/tests/test_middleware.py b/tests/test_middleware.py index 1dba71e..cb7f7cd 100644 --- a/tests/test_middleware.py +++ b/tests/test_middleware.py @@ -1,7 +1,9 @@ """Tests for the AsyncMiddleware protocol, AsyncNext type, chain composition, and decorators.""" import asyncio +import functools import typing +from collections.abc import Callable from http import HTTPStatus import httpx2 @@ -189,3 +191,34 @@ def test_compose_get_type_hints_resolves_without_nameerror() -> None: """Sync mirror — sync `compose` get_type_hints must also resolve.""" hints = typing.get_type_hints(compose) assert "middleware" in hints + + +async def _passthrough(*args: typing.Any) -> typing.Any: # noqa: ANN401 + return args[-1] # pragma: no cover — never invoked; the test only checks repr() + + +class _CallableTransform: + async def __call__(self, *args: typing.Any) -> typing.Any: # noqa: ANN401 + return args[-1] # pragma: no cover — never invoked; the test only checks repr() + + +@pytest.mark.parametrize( + ("decorator", "name"), + [ + (async_before_request, "async_before_request"), + (async_after_response, "async_after_response"), + (async_on_error, "async_on_error"), + ], +) +@pytest.mark.parametrize( + "factory", + [lambda: functools.partial(_passthrough), _CallableTransform], + ids=["partial", "callable_instance"], +) +def test_decorator_repr_falls_back_to_repr_without_qualname( + decorator: Callable[[typing.Any], AsyncMiddleware], + name: str, + factory: Callable[[], typing.Any], +) -> None: + f = factory() + assert repr(decorator(f)) == f"<{name}({f!r})>" diff --git a/tests/test_middleware_sync.py b/tests/test_middleware_sync.py index 1659211..8b71ebf 100644 --- a/tests/test_middleware_sync.py +++ b/tests/test_middleware_sync.py @@ -1,5 +1,8 @@ """Tests for the sync Middleware protocol, Next type, chain composition, and decorators.""" +import functools +import typing +from collections.abc import Callable from http import HTTPStatus import httpx2 @@ -197,3 +200,34 @@ def terminal_se(request: httpx2.Request) -> httpx2.Response: # noqa: ARG001 dispatch = compose((swallow_all,), terminal_se) with pytest.raises(SystemExit): dispatch(_make_request()) + + +def _passthrough(*args: typing.Any) -> typing.Any: # noqa: ANN401 + return args[-1] # pragma: no cover — never invoked; the test only checks repr() + + +class _CallableTransform: + def __call__(self, *args: typing.Any) -> typing.Any: # noqa: ANN401 + return args[-1] # pragma: no cover — never invoked; the test only checks repr() + + +@pytest.mark.parametrize( + ("decorator", "name"), + [ + (before_request, "before_request"), + (after_response, "after_response"), + (on_error, "on_error"), + ], +) +@pytest.mark.parametrize( + "factory", + [lambda: functools.partial(_passthrough), _CallableTransform], + ids=["partial", "callable_instance"], +) +def test_decorator_repr_falls_back_to_repr_without_qualname( + decorator: Callable[[typing.Any], Middleware], + name: str, + factory: Callable[[], typing.Any], +) -> None: + f = factory() + assert repr(decorator(f)) == f"<{name}({f!r})>" From 65d8e06a5a581145df2e56b21e1f4b2da189be71 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sat, 17 Oct 2026 11:42:25 +0300 Subject: [PATCH 6/6] docs: describe redaction as masking every query value --- docs/middleware.md | 2 +- src/httpware/_internal/observability.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/middleware.md b/docs/middleware.md index bacbe8e..3eed2e0 100644 --- a/docs/middleware.md +++ b/docs/middleware.md @@ -13,7 +13,7 @@ Middleware is for behavior that applies to every call through a client. Other ca - Side effects that don't need httpware's exceptions or chain order, including on post-redirect hops: use `event_hooks` on an `httpx2` client you build yourself and pass as `httpx2_client=`. Middleware and phase decorators run in the httpware chain, see httpware exceptions, and compose with `AsyncRetry` and `AsyncBulkhead`. `event_hooks` run a layer below, on every transport attempt. That is also why `httpware` refuses `event_hooks=` as a client option: an `httpx2.HTTPStatusError` raised in a hook (say, by `raise_for_status()`) reaches the chain as a plain `TransportError` instead of a `StatusError`, so `AsyncRetry` never retries it and the circuit breaker never counts it. - URL or header validation: `httpx2` already does it. - Creating HTTP spans for tracing: install `opentelemetry-instrumentation-httpx`, which already traces every transport call. See [Observability](observability.md). -- Redaction: httpware redacts URLs before they reach logs, telemetry, and error messages. It strips `user:pass@` userinfo and masks the values of sensitive query and fragment parameters ([Errors](errors.md#excresponse-access-pattern) lists them). It does not touch headers or bodies, so if your middleware logs those, redact them yourself, for example with a `logging.Filter`. +- Redaction: httpware redacts URLs before they reach logs, telemetry, and error messages. It strips `user:pass@` userinfo and replaces every query and fragment parameter value with `REDACTED`, keeping the keys ([Errors](errors.md#excresponse-access-pattern) has an example). It does not touch headers or bodies, so if your middleware logs those, redact them yourself, for example with a `logging.Filter`. ## Writing your own diff --git a/src/httpware/_internal/observability.py b/src/httpware/_internal/observability.py index 506afbc..0b25458 100644 --- a/src/httpware/_internal/observability.py +++ b/src/httpware/_internal/observability.py @@ -26,7 +26,7 @@ def _emit_event( The ``url`` attribute, when present, is run through ``redaction.redact_url`` here — at the single emission boundary — so a - request URL's userinfo and known-sensitive query/fragment secrets never + request URL's userinfo and query/fragment parameter values never reach a log record or span event, regardless of how a caller built the attributes dict.