From 30c6241e3a5f534037dc57a6ea544d5ff9d8bdeb Mon Sep 17 00:00:00 2001
From: Yuneng Jiang <yuneng@berri.ai>
Date: Wed, 16 Sep 2026 03:01:07 -0700
Subject: [PATCH] fix(e2e): a null error field is not an error

Every OpenAI Responses body carries `error: null` at the top level, and the
completeness check tested the key's presence rather than its value, so it
rejected every single one. The cost was silent: nothing failed, the endpoint
simply never cached, which is exactly the outcome the endpoint was added for.

Found by driving the edge against the real providers rather than the synthetic
fixtures, which carried no error key at all. Reading the value instead of the
key is also more accurate for chat completions and messages, where a real error
body carries a populated error object.
---
 .../test_provider_cache.py                    | 42 +++++++++++++++++--
 tests/e2e/provider_cache.py                   |  7 +++-
 2 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/tests/code_coverage_tests/test_provider_cache.py b/tests/code_coverage_tests/test_provider_cache.py
index 0278bf8fd48..3dafbadf508 100644
--- a/tests/code_coverage_tests/test_provider_cache.py
+++ b/tests/code_coverage_tests/test_provider_cache.py
@@ -508,10 +508,13 @@ EMBEDDING_SUCCESS: Final = (
     b'{"object":"list","data":[{"object":"embedding","index":0,"embedding":[0.1,0.2]}],'
     b'"model":"text-embedding-3-small","usage":{"prompt_tokens":2,"total_tokens":2}}'
 )
-RESPONSE_SUCCESS: Final = b'{"id":"resp_synthetic","object":"response","status":"completed","output":[]}'
+RESPONSE_SUCCESS: Final = (
+    b'{"id":"resp_synthetic","object":"response","status":"completed","error":null,'
+    b'"incomplete_details":null,"output":[]}'
+)
 RESPONSE_STREAM_SUCCESS: Final = (
-    b'data: {"type":"response.created","response":{"id":"resp_synthetic"}}\n\n'
-    b'data: {"type":"response.completed","response":{"id":"resp_synthetic","status":"completed"}}\n\n'
+    b'data: {"type":"response.created","response":{"id":"resp_synthetic","error":null}}\n\n'
+    b'data: {"type":"response.completed","response":{"id":"resp_synthetic","status":"completed"},"error":null}\n\n'
 )
 
 
@@ -586,6 +589,39 @@ class TestNonChatOpenAiEndpoints:
                 assert call(url, MARKED).body == payload
         assert len(provider.hits) == 2
 
+    @pytest.mark.parametrize("path,response", [
+        ("/v1/chat/completions", b'{"id":"x","error":null,"choices":[{"message":{"content":"hi"},'
+                                 b'"finish_reason":"stop"}]}'),
+        ("/v1/messages", b'{"id":"msg_x","type":"message","role":"assistant","error":null,'
+                         b'"content":[{"type":"text","text":"hi"}],"stop_reason":"end_turn"}'),
+        ("/v1/responses", RESPONSE_SUCCESS),
+    ])
+    def test_a_null_error_field_is_not_an_error(
+        self, store: RedisResponseStore, provider: Provider, path: str, response: bytes,
+    ) -> None:
+        """Every OpenAI Responses body carries `error: null`, and testing the key's
+        presence rather than its value rejected all of them. The cost was silent:
+        nothing failed, the endpoint simply never cached."""
+        assert b'"error":null' in response
+        provider.response = response
+        for _ in range(2):
+            with openai_edge(cache_edge(store), provider, path) as url:
+                assert call(url, MARKED).body == response
+        assert len(provider.hits) == 1
+
+    @pytest.mark.parametrize("path,response", [
+        ("/v1/chat/completions", b'{"error":{"message":"rate limited","type":"rate_limit_error"}}'),
+        ("/v1/responses", b'{"object":"response","status":"completed","error":{"message":"bad"},"output":[]}'),
+    ])
+    def test_a_populated_error_field_still_rejects(
+        self, store: RedisResponseStore, provider: Provider, path: str, response: bytes,
+    ) -> None:
+        provider.response = response
+        for _ in range(2):
+            with openai_edge(cache_edge(store), provider, path) as url:
+                assert call(url, MARKED).body == response
+        assert len(provider.hits) == 2
+
     @pytest.mark.parametrize("path,cacheable", [
         ("/v1/chat/completions", True), ("/v1/messages", True),
         ("/v1/embeddings", True), ("/v1/responses", True),
diff --git a/tests/e2e/provider_cache.py b/tests/e2e/provider_cache.py
index a2b18a3a466..0dee33f33c6 100644
--- a/tests/e2e/provider_cache.py
+++ b/tests/e2e/provider_cache.py
@@ -167,7 +167,10 @@ def successful_response(mount: str, url: str, status: int, headers: Mapping[str,
             values: Final = tuple(JSON_VALUE.validate_json(event) for event in events if event != "[DONE]")
         except (UnicodeDecodeError, ValidationError):
             return False
-        if not values or any(not isinstance(value, dict) or "error" in value or value.get("type") == "error" for value in values):
+        if not values or any(
+            not isinstance(value, dict) or value.get("error") is not None or value.get("type") == "error"
+            for value in values
+        ):
             return False
         if urlsplit(url).path == "/v1/responses":
             return complete_responses_stream(values)
@@ -187,7 +190,7 @@ def successful_response(mount: str, url: str, status: int, headers: Mapping[str,
         value: Final = JSON_VALUE.validate_json(body)
     except ValidationError:
         return False
-    if not isinstance(value, dict) or "error" in value:
+    if not isinstance(value, dict) or value.get("error") is not None:
         return False
     path: Final = urlsplit(url).path
     if path == "/v1/messages":