tests(e2e-cassette-proxy): fix replay path so cache hits actually serve

Every cache hit was silently falling through to the upstream because
the addon's replay path called http.Response.make(status, body, list[(str,str)]),
and mitmproxy 11's Headers constructor demands bytes — it raised
``TypeError: Header fields must be bytes.`` inside the addon, mitmdump
logged ``Addon error: Header fields must be bytes.`` to its background
log, and the request continued out to the real provider as if it had
been a miss. (Stats counted it as a hit because the log line was
emitted before the raise.) Net effect: the proxy was record-only.

Three fixes:

1. _build_replay_response constructs http.Response directly,
   encoding the cached headers back to bytes (latin-1, RFC 7230) and
   handing mitmproxy the raw on-the-wire body. Going through
   Response.make/set_content would also have re-encoded the
   body (e.g. double-gzip), so we bypass that codepath entirely.

2. The recording side now calls flow.response.headers.items(multi=True)
   so repeated headers (notably multiple Set-Cookie) are preserved
   as distinct entries instead of being silently merged.

3. Adds a contract test file that runs against *real* mitmproxy
   (skipped automatically when only the test stub is loaded). This is
   what would have caught the original bug — the existing fake stubs
   don't model Headers's bytes-strictness, which is precisely why
   the issue hid for the whole record-only run on the previous commit.

The existing addon unit tests now prefer real mitmproxy when it's
installed, so the contract tests run in the same process when both
are available.

tests/e2e_cassette_proxy/addon.py

@@ -18,7 +18,8 @@ from __future__ import annotations
 
 import logging
 import os
-from typing import Optional
+import time
+from typing import Optional, Tuple
 
 from mitmproxy import ctx, http  # type: ignore[import-not-found]
 
@@ -73,6 +74,54 @@ def _is_2xx(status_code: int) -> bool:
     return 200 <= status_code < 300
 
 
+def _to_header_bytes(value: str) -> bytes:
+    """Encode a header name or value back to bytes for mitmproxy.
+
+    HTTP/1.1 header field names are 7-bit ASCII per RFC 7230 § 3.2;
+    values are also ASCII in practice but technically allow opaque
+    octets, which mitmproxy round-trips via the surrogateescape codec.
+    Latin-1 covers both cases without raising on stray high bytes that
+    might have leaked in via misbehaved upstreams.
+    """
+    return value.encode("latin-1", errors="replace")
+
+
+def _build_replay_response(cached: CachedResponse) -> http.Response:
+    """Build a mitmproxy ``Response`` from a cached payload.
+
+    We bypass ``http.Response.make`` because:
+
+    1. ``make`` requires a ``Headers`` instance or a ``dict`` to do its
+       own bytes-conversion. Passing a list of ``(str, str)`` tuples
+       falls into its iterable branch which calls ``Headers(headers)``
+       directly — and ``Headers`` raises ``TypeError: Header fields
+       must be bytes``. (This was the original bug: every cache hit
+       silently fell through to upstream because the addon raised here.)
+    2. ``make`` calls ``Message.set_content`` which re-encodes the body
+       per the cached ``Content-Encoding`` header. Since we recorded
+       ``raw_content`` (i.e. the already-encoded wire bytes), going
+       through ``set_content`` would double-gzip those payloads.
+
+    Constructing the ``Response`` directly lets us hand mitmproxy bytes
+    headers + the raw body in one shot, exactly mirroring what was on
+    the wire when we recorded it.
+    """
+    fields: Tuple[Tuple[bytes, bytes], ...] = tuple(
+        (_to_header_bytes(k), _to_header_bytes(v)) for k, v in cached.headers
+    )
+    now = time.time()
+    return http.Response(
+        http_version=b"HTTP/1.1",
+        status_code=cached.status_code,
+        reason=_to_header_bytes(cached.reason) or b"OK",
+        headers=fields,
+        content=cached.body,
+        trailers=None,
+        timestamp_start=now,
+        timestamp_end=now,
+    )
+
+
 class CassetteAddon:
     """mitmproxy addon class. Created once at startup; ``request`` and
     ``response`` are invoked per flow."""
@@ -151,11 +200,7 @@ class CassetteAddon:
             f"status={cached.status_code} bytes={len(cached.body)}"
         )
         self._stats["hit"] += 1
-        flow.response = http.Response.make(
-            cached.status_code,
-            cached.body,
-            list(cached.headers),
-        )
+        flow.response = _build_replay_response(cached)
 
     def response(self, flow: http.HTTPFlow) -> None:
         if self._store is None or self._should_skip(flow):
@@ -172,7 +217,11 @@ class CassetteAddon:
         key = self._key_for(flow)
         cached = CachedResponse(
             status_code=flow.response.status_code,
-            headers=tuple((k, v) for k, v in flow.response.headers.items()),
+            # ``items(multi=True)`` preserves repeated headers (e.g. multiple
+            # ``Set-Cookie``) which the plain ``items()`` flattens into a
+            # single comma-joined string. Strings are fine on the wire to
+            # Redis; we re-encode to bytes on replay.
+            headers=tuple((k, v) for k, v in flow.response.headers.items(multi=True)),
             body=flow.response.raw_content or b"",
             reason=flow.response.reason or "",
         )

tests/test_litellm/e2e_cassette_proxy/test_addon.py

@@ -21,6 +21,17 @@ sys.path.insert(
     0, os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "..", ".."))
 )
 
+# Prefer the *real* mitmproxy if it's installed: that way the contract
+# tests in ``test_replay_response_contract.py`` exercise the same
+# objects this file does, and any future skew between our fake and
+# real mitmproxy can't silently hide regressions. Fall through to the
+# fake stub when mitmproxy isn't available (the typical unit-test env).
+try:
+    import mitmproxy  # noqa: F401  pre-load so setdefault below is a no-op
+    import mitmproxy.http  # noqa: F401
+except ImportError:
+    pass
+
 # Install a fake `mitmproxy` package *before* importing the addon, so the
 # real mitmproxy doesn't need to be present in the unit-test env.
 _mitmproxy_pkg = types.ModuleType("mitmproxy")
@@ -28,16 +39,64 @@ _http_mod = types.ModuleType("mitmproxy.http")
 
 
 class _FakeHeaders(dict):
-    def items(self):
+    def items(self, multi: bool = False):
+        # Real mitmproxy ``Headers.items(multi=True)`` would return one
+        # tuple per repeated header (e.g. multiple ``Set-Cookie``). The
+        # fake doesn't model duplicates, so the kwarg is a no-op — we
+        # just need to accept it without ``TypeError``.
         return list(super().items())
 
 
 class _FakeResponse:
-    def __init__(self, status_code, body=b"", headers=None, reason=""):
+    """Fake mitmproxy ``http.Response`` that accepts both the real
+    constructor signature (used by the addon's replay path) and a
+    simplified positional signature (used by these tests when faking an
+    upstream response).
+    """
+
+    def __init__(
+        self,
+        *args,
+        status_code=None,
+        body=None,
+        headers=None,
+        reason="",
+        # real-mitmproxy kwargs (the addon passes these via _build_replay_response)
+        http_version=b"HTTP/1.1",
+        content=None,
+        trailers=None,
+        timestamp_start=None,
+        timestamp_end=None,
+    ):
+        # Positional support: _FakeResponse(status_code, body=..., headers=...)
+        if args:
+            if status_code is None:
+                status_code = args[0]
+            if len(args) > 1 and body is None:
+                body = args[1]
+        # ``content`` (real mitmproxy) and ``body`` (test shorthand) are
+        # the same field — raw_content.
+        if body is None:
+            body = content if content is not None else b""
+        if isinstance(headers, list):
+            # Bytes-tuple list (real mitmproxy contract) → str dict for
+            # the fake's case-insensitive lookups.
+            headers = {
+                (k.decode("latin-1") if isinstance(k, bytes) else k): (
+                    v.decode("latin-1") if isinstance(v, bytes) else v
+                )
+                for k, v in headers
+            }
+        if isinstance(reason, bytes):
+            reason = reason.decode("latin-1")
         self.status_code = status_code
         self.raw_content = body
         self.headers = _FakeHeaders(headers or {})
         self.reason = reason
+        self.http_version = http_version
+        self.trailers = trailers
+        self.timestamp_start = timestamp_start
+        self.timestamp_end = timestamp_end
 
     @classmethod
     def make(cls, status_code, body=b"", headers=None):

tests/test_litellm/e2e_cassette_proxy/test_replay_response_contract.py

@@ -0,0 +1,146 @@
+"""Contract tests against real mitmproxy.
+
+The other addon unit tests stub out ``mitmproxy.http`` so they can run
+without mitmproxy installed (it's a CI-only dependency installed via
+``uv tool install`` rather than declared in ``pyproject.toml``).
+
+That stubbing once hid a real production bug: the addon called
+``http.Response.make(status, body, list[(str, str)])`` which the fake
+accepted, but real mitmproxy 11 raises ``TypeError: Header fields must
+be bytes``. Every cache hit silently fell through to the upstream
+because the addon raised inside ``request()``.
+
+This file exercises the *real* mitmproxy ``Response`` contract for the
+replay path, so any future regression is caught locally instead of only
+showing up as ``Addon error: ...`` in CI logs while tests pass.
+"""
+
+from __future__ import annotations
+
+import pytest
+
+mitmproxy_http = pytest.importorskip("mitmproxy.http")
+
+# ``test_addon.py`` (collected alongside this file) installs a stubbed
+# ``mitmproxy`` package into ``sys.modules`` so it can run without the
+# real dependency. ``importorskip`` will happily return that stub and
+# defeat the whole point of *this* file. Detect the stub by checking
+# whether the loaded ``Headers`` enforces bytes the way real mitmproxy
+# does, and skip if it doesn't.
+try:
+    mitmproxy_http.Headers([("not", "bytes")])  # type: ignore[arg-type]
+except TypeError:
+    pass  # real mitmproxy — proceed
+except Exception:
+    pytest.skip(
+        "real mitmproxy not available (fake stub detected); "
+        "install ``mitmproxy==11.0.2`` to run these contract tests",
+        allow_module_level=True,
+    )
+else:
+    pytest.skip(
+        "real mitmproxy not available (fake stub detected); "
+        "install ``mitmproxy==11.0.2`` to run these contract tests",
+        allow_module_level=True,
+    )
+
+from tests.e2e_cassette_proxy.addon import _build_replay_response  # noqa: E402
+from tests.e2e_cassette_proxy.redis_store import CachedResponse  # noqa: E402
+
+
+def test_replay_response_accepts_string_headers_and_emits_bytes():
+    """Cached headers are stored as ``str`` (msgpack roundtrip); the
+    replay builder must encode them back to ``bytes`` because mitmproxy
+    11's ``Headers`` rejects anything else."""
+    cached = CachedResponse(
+        status_code=200,
+        headers=(
+            ("Content-Type", "application/json"),
+            ("X-Trace-Id", "abc123"),
+        ),
+        body=b'{"id":"chatcmpl-1"}',
+        reason="OK",
+    )
+
+    response = _build_replay_response(cached)
+
+    assert isinstance(response, mitmproxy_http.Response)
+    assert response.status_code == 200
+    # mitmproxy's ``Headers`` decodes back to str on read; the underlying
+    # ``fields`` are bytes — verify both directions.
+    assert response.headers["Content-Type"] == "application/json"
+    assert response.headers["X-Trace-Id"] == "abc123"
+    for name, value in response.headers.fields:
+        assert isinstance(name, bytes), name
+        assert isinstance(value, bytes), value
+
+
+def test_replay_response_preserves_raw_content_without_double_encoding():
+    """When the upstream replied with ``Content-Encoding: gzip`` we recorded
+    the *gzipped* bytes (i.e. ``flow.response.raw_content``). On replay the
+    body must be set as ``raw_content`` so mitmproxy serves the same wire
+    payload — going through ``set_content`` would gzip-encode an already-
+    gzipped blob."""
+    gzipped_body = (
+        b"\x1f\x8b\x08\x00\x00\x00\x00\x00\x00\x03\xab\xe6"
+        b"R\xa8\xe5\x02\x00\x9b\xae\x10\xee\x05\x00\x00\x00"
+    )  # arbitrary gzip-magic-prefixed bytes
+    cached = CachedResponse(
+        status_code=200,
+        headers=(
+            ("Content-Type", "application/json"),
+            ("Content-Encoding", "gzip"),
+            ("Content-Length", str(len(gzipped_body))),
+        ),
+        body=gzipped_body,
+        reason="OK",
+    )
+
+    response = _build_replay_response(cached)
+
+    assert response.raw_content == gzipped_body, (
+        "raw_content must be byte-identical to what we recorded; "
+        "going through set_content would re-gzip and corrupt it"
+    )
+
+
+def test_replay_response_handles_repeated_set_cookie_headers():
+    """``items(multi=True)`` is used on the recording side specifically so
+    multi-value headers like ``Set-Cookie`` aren't silently merged into a
+    comma-joined string (which is RFC 7230 illegal for ``Set-Cookie``)."""
+    cached = CachedResponse(
+        status_code=200,
+        headers=(
+            ("Set-Cookie", "session=abc; Path=/"),
+            ("Set-Cookie", "csrf=xyz; Path=/"),
+            ("Content-Type", "text/html"),
+        ),
+        body=b"<html></html>",
+        reason="OK",
+    )
+
+    response = _build_replay_response(cached)
+
+    set_cookies = [
+        v.decode("latin-1")
+        for k, v in response.headers.fields
+        if k.lower() == b"set-cookie"
+    ]
+    assert set_cookies == ["session=abc; Path=/", "csrf=xyz; Path=/"]
+
+
+def test_replay_response_tolerates_empty_reason_phrase():
+    cached = CachedResponse(
+        status_code=204,
+        headers=(),
+        body=b"",
+        reason="",
+    )
+
+    response = _build_replay_response(cached)
+
+    assert response.status_code == 204
+    # mitmproxy expects bytes; an empty reason should default to ``b"OK"``
+    # at the wire level so the response is still serializable by
+    # mitmproxy's HTTP/1.1 codec. ``reason`` decodes back to str on read.
+    assert response.reason == "OK"