Skip to content

Commit d6e5a2b

Browse files
fix: ensure content node is dumped for non-streaming HTTP responses
Previously, response_body() incorrectly used response.raw.stream which is not a valid attribute, causing content to be skipped for authentication requests and other non-streaming responses that needed content capture. Changes: - Add is_streaming_response() function using response.raw.isclosed() - Update response_body() to properly detect streaming vs non-streaming - Add comprehensive tests with local HTTP server - Verify JSON/text responses include content in HAR entries - Verify streaming responses skip content to prevent memory leaks - Test with multi-GB streams to ensure no memory consumption This fixes authentication request logging where content was missing from HAR entries, improving debugging capabilities for auth issues. Related to #1240 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent df2ba11 commit d6e5a2b

File tree

2 files changed

+362
-1
lines changed

2 files changed

+362
-1
lines changed

src/foundation/http.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,16 @@ def cookie_str_to_dict(cookie_header: str) -> Mapping[str, str]:
3131
return cookies
3232

3333

34+
def is_streaming_response(response: Response) -> bool:
35+
"""Check if response was created with stream=True"""
36+
try:
37+
return hasattr(response, "raw") and not response.raw.isclosed()
38+
except Exception:
39+
return False
40+
41+
3442
def response_body(response: Response) -> Any:
35-
if response.raw.stream:
43+
if is_streaming_response(response):
3644
return None
3745
else:
3846
try:

tests/test_http.py

Lines changed: 353 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,353 @@
1+
import json
2+
import threading
3+
import time
4+
import unittest
5+
from http.server import BaseHTTPRequestHandler, HTTPServer
6+
from typing import Any
7+
from unittest.mock import Mock
8+
from urllib.parse import parse_qs, urlparse
9+
10+
import requests
11+
from requests import Response
12+
13+
from foundation.http import is_streaming_response, response_to_har_entry
14+
15+
16+
class TestHttpUtils(unittest.TestCase):
17+
"""Test HTTP utility functions"""
18+
19+
def test_is_streaming_response_with_closed_connection(self) -> None:
20+
"""Test that is_streaming_response returns False for closed connections"""
21+
response = Mock(spec=Response)
22+
response.raw = Mock()
23+
response.raw.isclosed.return_value = True
24+
25+
result = is_streaming_response(response)
26+
27+
self.assertFalse(result)
28+
29+
def test_is_streaming_response_with_open_connection(self) -> None:
30+
"""Test that is_streaming_response returns True for open connections"""
31+
response = Mock(spec=Response)
32+
response.raw = Mock()
33+
response.raw.isclosed.return_value = False
34+
35+
result = is_streaming_response(response)
36+
37+
self.assertTrue(result)
38+
39+
def test_is_streaming_response_with_no_raw_attribute(self) -> None:
40+
"""Test that is_streaming_response returns False when raw attribute is missing"""
41+
response = Mock(spec=Response)
42+
# Don't set raw attribute
43+
44+
result = is_streaming_response(response)
45+
46+
self.assertFalse(result)
47+
48+
def test_is_streaming_response_with_exception(self) -> None:
49+
"""Test that is_streaming_response returns False when exception occurs"""
50+
response = Mock(spec=Response)
51+
response.raw = Mock()
52+
response.raw.isclosed.side_effect = Exception("Connection error")
53+
54+
result = is_streaming_response(response)
55+
56+
self.assertFalse(result)
57+
58+
def test_response_to_har_entry_with_json_response(self) -> None:
59+
"""Test that response_to_har_entry creates content node for JSON responses"""
60+
# Create mock request and response
61+
mock_request = Mock()
62+
mock_request.method = "GET"
63+
mock_request.url = "https://example.com/api"
64+
mock_request.headers = {"Accept": "application/json"}
65+
mock_request.body = None
66+
67+
response = Mock(spec=Response)
68+
response.request = mock_request
69+
response.status_code = 200
70+
response.headers = {"Content-Type": "application/json"}
71+
response.cookies = []
72+
response.raw = Mock()
73+
response.raw.isclosed.return_value = True # Non-streaming response
74+
response.json.return_value = {"result": "success"}
75+
76+
har_entry = response_to_har_entry(response)
77+
78+
self.assertIn("response", har_entry)
79+
self.assertIn("content", har_entry["response"])
80+
self.assertEqual(har_entry["response"]["content"], {"result": "success"})
81+
82+
def test_response_to_har_entry_with_stream_response(self) -> None:
83+
"""Test that response_to_har_entry skips content node for stream responses"""
84+
# Create mock request and response
85+
mock_request = Mock()
86+
mock_request.method = "GET"
87+
mock_request.url = "https://example.com/stream"
88+
mock_request.headers = {"Accept": "*/*"}
89+
mock_request.body = None
90+
91+
response = Mock(spec=Response)
92+
response.request = mock_request
93+
response.status_code = 200
94+
response.headers = {"Content-Type": "application/octet-stream"}
95+
response.cookies = []
96+
response.raw = Mock()
97+
response.raw.isclosed.return_value = False # Streaming response
98+
99+
har_entry = response_to_har_entry(response)
100+
101+
self.assertIn("response", har_entry)
102+
self.assertIn("content", har_entry["response"])
103+
self.assertIsNone(har_entry["response"]["content"])
104+
105+
106+
class HttpTestRequestHandler(BaseHTTPRequestHandler):
107+
"""Test HTTP request handler for local server"""
108+
109+
def log_message(self, format: str, *args: Any) -> None: # noqa: ARG002
110+
"""Suppress server log messages during tests"""
111+
pass
112+
113+
def do_GET(self) -> None:
114+
if self.path == "/json":
115+
# JSON response
116+
response_data = {"message": "Hello, World!", "status": "success", "data": [1, 2, 3]}
117+
self.send_response(200)
118+
self.send_header("Content-Type", "application/json")
119+
self.end_headers()
120+
self.wfile.write(json.dumps(response_data).encode())
121+
122+
elif self.path == "/text":
123+
# Plain text response
124+
response_text = "This is a plain text response for testing purposes."
125+
self.send_response(200)
126+
self.send_header("Content-Type", "text/plain")
127+
self.end_headers()
128+
self.wfile.write(response_text.encode())
129+
130+
elif self.path.startswith("/stream"):
131+
# Streaming binary response with optional size parameter
132+
parsed_url = urlparse(self.path)
133+
query_params = parse_qs(parsed_url.query)
134+
135+
# Get size parameter (in MB), default to 5 chunks of ~100 bytes each (~500 bytes total)
136+
size_mb = float(query_params.get("size_mb", [0.0005])[0]) # Default ~500 bytes
137+
138+
self.send_response(200)
139+
self.send_header("Content-Type", "application/octet-stream")
140+
self.send_header("Transfer-Encoding", "chunked")
141+
self.end_headers()
142+
143+
# Calculate chunks: each chunk is ~1MB for large sizes, or smaller for tiny sizes
144+
if size_mb >= 1:
145+
chunk_size_bytes = 1024 * 1024 # 1MB per chunk for large streams
146+
num_chunks = int(size_mb)
147+
remainder_bytes = int((size_mb - num_chunks) * 1024 * 1024)
148+
else:
149+
# For small sizes, use the original logic
150+
chunk_size_bytes = 100 # 100 bytes per chunk
151+
num_chunks = max(1, int(size_mb * 1024 * 1024 / chunk_size_bytes))
152+
remainder_bytes = 0
153+
154+
# Send data in chunks, handle client disconnections gracefully
155+
try:
156+
for i in range(num_chunks):
157+
chunk_data = f"chunk_{i}_".encode() + b"x" * (
158+
chunk_size_bytes - len(f"chunk_{i}_".encode())
159+
)
160+
chunk_size_hex = hex(len(chunk_data))[2:].encode()
161+
self.wfile.write(chunk_size_hex + b"\r\n" + chunk_data + b"\r\n")
162+
self.wfile.flush()
163+
164+
# Send remainder if any
165+
if remainder_bytes > 0:
166+
chunk_data = b"remainder_" + b"x" * (remainder_bytes - len(b"remainder_"))
167+
chunk_size_hex = hex(len(chunk_data))[2:].encode()
168+
self.wfile.write(chunk_size_hex + b"\r\n" + chunk_data + b"\r\n")
169+
self.wfile.flush()
170+
171+
# End chunked transfer
172+
self.wfile.write(b"0\r\n\r\n")
173+
except (ConnectionResetError, BrokenPipeError, OSError):
174+
# Client closed connection early - this is expected for streaming tests
175+
# where we don't read all the data
176+
pass
177+
178+
else:
179+
self.send_response(404)
180+
self.end_headers()
181+
self.wfile.write(b"Not Found")
182+
183+
184+
class TestHttpIntegration(unittest.TestCase):
185+
"""Integration tests using real HTTP server"""
186+
187+
server: HTTPServer
188+
port: int
189+
base_url: str
190+
server_thread: threading.Thread
191+
192+
@classmethod
193+
def setUpClass(cls) -> None:
194+
"""Start test server"""
195+
cls.server = HTTPServer(("localhost", 0), HttpTestRequestHandler)
196+
cls.port = cls.server.server_port
197+
cls.base_url = f"http://localhost:{cls.port}"
198+
199+
# Start server in background thread
200+
cls.server_thread = threading.Thread(target=cls.server.serve_forever)
201+
cls.server_thread.daemon = True
202+
cls.server_thread.start()
203+
204+
# Wait for server to start
205+
time.sleep(0.1)
206+
207+
@classmethod
208+
def tearDownClass(cls) -> None:
209+
"""Stop test server"""
210+
cls.server.shutdown()
211+
cls.server.server_close()
212+
213+
def test_response_to_har_entry_with_real_json_response(self) -> None:
214+
"""Test response_to_har_entry with real JSON response"""
215+
response = requests.get(f"{self.base_url}/json")
216+
217+
har_entry = response_to_har_entry(response)
218+
219+
# Verify structure
220+
self.assertIn("request", har_entry)
221+
self.assertIn("response", har_entry)
222+
223+
# Verify request details
224+
self.assertEqual(har_entry["request"]["method"], "GET")
225+
self.assertIn("/json", har_entry["request"]["url"])
226+
227+
# Verify response details
228+
self.assertEqual(har_entry["response"]["status_code"], 200)
229+
self.assertIn("content", har_entry["response"])
230+
231+
# Verify JSON content is parsed correctly
232+
expected_content = {"message": "Hello, World!", "status": "success", "data": [1, 2, 3]}
233+
self.assertEqual(har_entry["response"]["content"], expected_content)
234+
235+
# Verify it's not a streaming response
236+
self.assertFalse(is_streaming_response(response))
237+
238+
def test_response_to_har_entry_with_real_text_response(self) -> None:
239+
"""Test response_to_har_entry with real text response"""
240+
response = requests.get(f"{self.base_url}/text")
241+
242+
har_entry = response_to_har_entry(response)
243+
244+
# Verify structure
245+
self.assertIn("request", har_entry)
246+
self.assertIn("response", har_entry)
247+
248+
# Verify request details
249+
self.assertEqual(har_entry["request"]["method"], "GET")
250+
self.assertIn("/text", har_entry["request"]["url"])
251+
252+
# Verify response details
253+
self.assertEqual(har_entry["response"]["status_code"], 200)
254+
self.assertIn("content", har_entry["response"])
255+
256+
# Verify text content is included
257+
expected_content = "This is a plain text response for testing purposes."
258+
self.assertEqual(har_entry["response"]["content"], expected_content)
259+
260+
# Verify it's not a streaming response
261+
self.assertFalse(is_streaming_response(response))
262+
263+
def test_response_to_har_entry_with_real_streaming_response(self) -> None:
264+
"""Test response_to_har_entry with real streaming binary response"""
265+
response = requests.get(f"{self.base_url}/stream", stream=True)
266+
267+
har_entry = response_to_har_entry(response)
268+
269+
# Verify structure
270+
self.assertIn("request", har_entry)
271+
self.assertIn("response", har_entry)
272+
273+
# Verify request details
274+
self.assertEqual(har_entry["request"]["method"], "GET")
275+
self.assertIn("/stream", har_entry["request"]["url"])
276+
277+
# Verify response details
278+
self.assertEqual(har_entry["response"]["status_code"], 200)
279+
self.assertIn("content", har_entry["response"])
280+
281+
# Verify streaming response has no content in HAR entry
282+
self.assertIsNone(har_entry["response"]["content"])
283+
284+
# Verify it is detected as a streaming response
285+
self.assertTrue(is_streaming_response(response))
286+
287+
# Clean up the response connection
288+
response.close()
289+
290+
def test_streaming_vs_non_streaming_behavior(self) -> None:
291+
"""Test the difference between streaming and non-streaming requests to same endpoint"""
292+
# Non-streaming request
293+
response_normal = requests.get(f"{self.base_url}/json")
294+
har_normal = response_to_har_entry(response_normal)
295+
296+
# Streaming request to same endpoint
297+
response_stream = requests.get(f"{self.base_url}/json", stream=True)
298+
har_stream = response_to_har_entry(response_stream)
299+
300+
# Normal response should have content
301+
self.assertIsNotNone(har_normal["response"]["content"])
302+
self.assertEqual(har_normal["response"]["content"]["message"], "Hello, World!")
303+
self.assertFalse(is_streaming_response(response_normal))
304+
305+
# Streaming response should have no content
306+
self.assertIsNone(har_stream["response"]["content"])
307+
self.assertTrue(is_streaming_response(response_stream))
308+
309+
# Clean up streaming response
310+
response_stream.close()
311+
312+
def test_response_to_har_entry_with_large_streaming_response(self) -> None:
313+
"""Test response_to_har_entry with multi-GB streaming response to verify no memory leaks"""
314+
# Request a 200.5GB stream (large enough to cause memory issues if not handled properly)
315+
size_gb = 200.5
316+
response = requests.get(f"{self.base_url}/stream?size_mb={size_gb * 1024}", stream=True)
317+
318+
# This should complete quickly without loading all data into memory
319+
start_time = time.time()
320+
har_entry = response_to_har_entry(response)
321+
end_time = time.time()
322+
323+
# Should complete in well under a second since we don't load the content
324+
self.assertLess(
325+
end_time - start_time, 1.0, "HAR entry creation took too long, possible memory leak"
326+
)
327+
328+
# Verify structure
329+
self.assertIn("request", har_entry)
330+
self.assertIn("response", har_entry)
331+
332+
# Verify request details
333+
self.assertEqual(har_entry["request"]["method"], "GET")
334+
self.assertIn("/stream", har_entry["request"]["url"])
335+
self.assertIn(f"size_mb={size_gb * 1024}", har_entry["request"]["url"])
336+
337+
# Verify response details
338+
self.assertEqual(har_entry["response"]["status_code"], 200)
339+
self.assertIn("content", har_entry["response"])
340+
341+
# Most importantly: streaming response should have NO content in HAR entry
342+
# This proves we didn't try to load 200.5GB into memory
343+
self.assertIsNone(har_entry["response"]["content"])
344+
345+
# Verify it is detected as a streaming response
346+
self.assertTrue(is_streaming_response(response))
347+
348+
# Clean up the response connection (important for large streams)
349+
response.close()
350+
351+
352+
if __name__ == "__main__":
353+
unittest.main()

0 commit comments

Comments
 (0)