test(vcr): fix CI failures from PR #27159 (JSONL crash + replay-incompatible tests)

tests/_vcr_conftest_common.py

@@ -36,6 +36,9 @@
     vcr_verbose_enabled,
 )
 
+
+SAFE_BODY_MATCHER_NAME = "safe_body"
+
 FILTERED_REQUEST_HEADERS = (
     "authorization",
     "x-api-key",
@@ -82,6 +85,51 @@ def _before_record_response(response):
     return filter_non_2xx_response(_scrub_response(response))
 
 
+def _safe_body_matcher(r1, r2) -> None:
+    """Body matcher that compares raw bytes and never raises on bad JSON.
+
+    vcrpy's stock ``body`` matcher inspects ``Content-Type`` and runs
+    ``json.loads`` on bodies typed ``application/json`` so it can compare
+    semantically. That crashes (``json.JSONDecodeError: Extra data``) on
+    JSON Lines payloads — which the Bedrock batch S3 PUT and a few other
+    upload paths use — before the matcher even gets a chance to return
+    "not a match".
+
+    This matcher avoids the JSON normalization step entirely and just
+    compares the request bodies as bytes, falling back to repr equality
+    for non-bytes/str payloads. It is strictly more conservative than
+    vcrpy's default — the only thing it gives up is "different JSON key
+    order is treated as the same body", which doesn't matter for our
+    deterministic litellm-built request payloads. It can never produce a
+    false positive that the default would have rejected.
+
+    The trade-off is that bodies containing nondeterministic values (UUIDs,
+    timestamps) will produce a cache miss; the right fix for those cases
+    is a ``before_record_request`` scrubber, not a smarter matcher.
+    """
+    body1 = getattr(r1, "body", None)
+    body2 = getattr(r2, "body", None)
+    if body1 == body2:
+        return
+
+    def _to_bytes(b):
+        if b is None:
+            return b""
+        if isinstance(b, bytes):
+            return b
+        if isinstance(b, str):
+            return b.encode("utf-8")
+        return None
+
+    n1 = _to_bytes(body1)
+    n2 = _to_bytes(body2)
+    if n1 is not None and n2 is not None:
+        if n1 == n2:
+            return
+        raise AssertionError("request bodies differ")
+    raise AssertionError("request bodies differ")
+
+
 def vcr_config_dict() -> dict:
     """Return the VCR config dict shared across all consuming conftests."""
     return {
@@ -96,7 +144,7 @@ def vcr_config_dict() -> dict:
             "port",
             "path",
             "query",
-            "body",
+            SAFE_BODY_MATCHER_NAME,
         ),
         "before_record_response": _before_record_response,
     }
@@ -110,13 +158,15 @@ def vcr_disabled() -> bool:
 
 
 def register_persister_if_enabled(vcr) -> None:
-    """Wire the Redis persister into vcrpy if VCR is enabled.
+    """Wire the Redis persister and custom matchers into vcrpy if VCR is
+    enabled.
 
     Call this from ``pytest_recording_configure(config, vcr)`` in conftest.
     """
     if vcr_disabled():
         return
     vcr.register_persister(make_redis_persister())
+    vcr.register_matcher(SAFE_BODY_MATCHER_NAME, _safe_body_matcher)
     patch_vcrpy_aiohttp_record_path()
 
 

tests/local_testing/conftest.py

@@ -40,6 +40,29 @@
     }
 )
 
+# Files where VCR replay actively breaks the test:
+# - ``test_assistants.py`` exercises the OpenAI Assistants polling APIs
+#   which mint fresh thread/run/message IDs every recording session and
+#   then poll until ``status == "completed"``. Replays of those polled
+#   GETs would have to match the new run id (impossible) or be played
+#   back in lockstep with a freshly recorded creation, neither of which
+#   ``record_mode="new_episodes"`` does well. The result in CI is that
+#   every run effectively re-records, blowing past the 15-minute step
+#   timeout for ``litellm_assistants_api_testing``.
+_VCR_INCOMPATIBLE_FILES = frozenset(
+    {
+        "test_assistants.py",
+    }
+)
+
+# Specific tests where VCR replay actively breaks the test:
+# - ``test_amazing_sync_embedding`` deliberately calls the embedding API
+#   with ``api_key="my-bad-key"`` to assert the failure callback fires.
+#   We scrub auth headers from cassettes (so the bad-key request matches
+#   the prior good-key request), and vcrpy replays the recorded 200 — so
+#   the failure callback never fires and the assertion flips.
+_VCR_INCOMPATIBLE_NODEID_SUFFIXES = ("::test_amazing_sync_embedding",)
+
 
 _verbose_state = VerboseReporterState()
 
@@ -201,7 +224,8 @@ def setup_and_teardown():
 def pytest_collection_modifyitems(config, items):
     apply_vcr_auto_marker_to_items(
         items,
-        skip_files=_RESPX_CONFLICTING_FILES,
+        skip_files=_RESPX_CONFLICTING_FILES | _VCR_INCOMPATIBLE_FILES,
+        skip_nodeid_suffixes=_VCR_INCOMPATIBLE_NODEID_SUFFIXES,
     )
 
     # Separate tests in 'test_amazing_proxy_custom_logger.py' and other tests

tests/logging_callback_tests/conftest.py

@@ -36,6 +36,26 @@
     }
 )
 
+# Files where VCR replay actively breaks the test:
+# - ``test_amazing_s3_logs.py`` exercises the S3 success callback using
+#   ``mock_response`` (so there is no upstream LLM call worth caching) and
+#   asserts on a per-run ``response_id`` round-tripped through a real S3
+#   PUT/LIST. vcrpy's boto3 stub intercepts the PUT and replays a stale LIST,
+#   so the freshly-generated id is never found in the cached keys.
+_VCR_INCOMPATIBLE_FILES = frozenset(
+    {
+        "test_amazing_s3_logs.py",
+    }
+)
+
+# Specific tests where VCR replay actively breaks the test:
+# - The "failure" branches of these callback tests deliberately pass a bad
+#   API key to assert that the ``async_failure`` / ``failure`` callback fires.
+#   We scrub auth headers from cassettes (so the bad-key request matches the
+#   prior good-key request), and vcrpy replays the recorded 200 — so the
+#   failure callback never fires and the assertion flips.
+_VCR_INCOMPATIBLE_NODEID_SUFFIXES = ("::test_async_embedding_azure",)
+
 
 _verbose_state = VerboseReporterState()
 
@@ -189,7 +209,8 @@ def setup_and_teardown():
 def pytest_collection_modifyitems(config, items):
     apply_vcr_auto_marker_to_items(
         items,
-        skip_files=_RESPX_CONFLICTING_FILES,
+        skip_files=_RESPX_CONFLICTING_FILES | _VCR_INCOMPATIBLE_FILES,
+        skip_nodeid_suffixes=_VCR_INCOMPATIBLE_NODEID_SUFFIXES,
     )
 
     # Separate tests in 'test_amazing_proxy_custom_logger.py' and other tests

tests/test_litellm/test_vcr_safe_body_matcher.py

@@ -0,0 +1,105 @@
+"""Unit tests for the shared VCR helpers in ``tests/_vcr_conftest_common``.
+
+The most important guarantee here is that the custom ``safe_body`` matcher
+gracefully handles JSON Lines (and other non-strict-JSON) request bodies
+without raising ``json.JSONDecodeError`` — vcrpy's default ``body`` matcher
+crashes on those because it unconditionally runs ``json.loads`` for any
+``application/json`` request body.
+"""
+
+from __future__ import annotations
+
+import os
+import sys
+from types import SimpleNamespace
+
+import pytest
+
+# Tests live in ``tests/test_litellm/`` but ``_vcr_conftest_common`` lives in
+# the parent ``tests/`` package. Make sure both are importable regardless of
+# how pytest is invoked.
+_REPO_ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", ".."))
+if _REPO_ROOT not in sys.path:
+    sys.path.insert(0, _REPO_ROOT)
+
+from tests._vcr_conftest_common import (  # noqa: E402
+    SAFE_BODY_MATCHER_NAME,
+    _safe_body_matcher,
+    vcr_config_dict,
+)
+
+
+def _req(body):
+    return SimpleNamespace(body=body, headers={"Content-Type": "application/json"})
+
+
+def test_safe_body_matcher_is_in_match_on():
+    cfg = vcr_config_dict()
+    assert SAFE_BODY_MATCHER_NAME in cfg["match_on"]
+    assert "body" not in cfg["match_on"]
+
+
+def test_safe_body_matcher_accepts_identical_bytes():
+    _safe_body_matcher(_req(b"hello"), _req(b"hello"))
+
+
+def test_safe_body_matcher_accepts_str_bytes_equivalent():
+    _safe_body_matcher(_req("hello"), _req(b"hello"))
+
+
+def test_safe_body_matcher_handles_jsonl_without_crashing():
+    """vcrpy's default ``body`` matcher raises ``JSONDecodeError`` on JSONL.
+
+    The Bedrock batch S3 PUT sends a JSON Lines body under
+    ``Content-Type: application/json``. The safe matcher must compare such
+    bodies as bytes and never invoke ``json.loads``.
+    """
+    jsonl = (
+        b'{"recordId": "request-1", "modelInput": {}}\n'
+        b'{"recordId": "request-2", "modelInput": {}}\n'
+    )
+    _safe_body_matcher(_req(jsonl), _req(jsonl))
+
+
+def test_safe_body_matcher_rejects_different_jsonl_bodies():
+    a = b'{"recordId": "request-1"}\n{"recordId": "request-2"}\n'
+    b = b'{"recordId": "request-1"}\n{"recordId": "request-3"}\n'
+    with pytest.raises(AssertionError):
+        _safe_body_matcher(_req(a), _req(b))
+
+
+def test_safe_body_matcher_rejects_different_bytes():
+    with pytest.raises(AssertionError):
+        _safe_body_matcher(_req(b"a"), _req(b"b"))
+
+
+def test_safe_body_matcher_treats_none_bodies_as_equal():
+    _safe_body_matcher(_req(None), _req(None))
+
+
+def test_safe_body_matcher_does_not_normalize_json_key_order():
+    """The safe matcher is strictly more conservative than vcrpy's default.
+
+    Two semantically-equal JSON bodies with different key order are
+    treated as *different* requests (cache miss, never a false hit).
+    """
+    with pytest.raises(AssertionError):
+        _safe_body_matcher(_req(b'{"a":1,"b":2}'), _req(b'{"b":2,"a":1}'))
+
+
+def test_default_vcrpy_body_matcher_crashes_on_jsonl_for_documentation():
+    """Document the behavior we are working around.
+
+    vcrpy's stock body matcher raises ``json.JSONDecodeError`` (not even
+    a clean ``AssertionError``) when given a JSONL payload typed as
+    ``application/json``. This is precisely the crash that broke
+    ``tests/batches_tests/test_bedrock_files_and_batches.py::test_async_create_file``
+    and is the reason ``safe_body`` exists.
+    """
+    import json
+
+    from vcr.matchers import body as vcrpy_body  # type: ignore
+
+    jsonl = b'{"recordId": "request-1"}\n{"recordId": "request-2"}\n'
+    with pytest.raises(json.JSONDecodeError):
+        vcrpy_body(_req(jsonl), _req(jsonl))