Skip to content

Commit 63b20fa

Browse files
Fix HTTP 404 error handling in urllib instrumentation REPLAY mode
When urllib raises HTTPError for 4xx/5xx responses, the instrumentation now correctly replays this by raising HTTPError instead of returning a normal MockHTTPResponse. This ensures application code catching HTTPError behaves the same in RECORD and REPLAY modes. - Add _raise_http_error_from_mock method to reconstruct HTTPError - Check for errorName in _try_get_mock and raise appropriately - Re-raise HTTPError/URLError exceptions to propagate correctly
1 parent 3303afe commit 63b20fa

3 files changed

Lines changed: 114 additions & 1 deletion

File tree

drift/instrumentation/urllib/e2e-tests/src/app.py

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,53 @@ def urlopen_with_data():
328328
return jsonify({"error": str(e)}), 500
329329

330330

331+
# Test: HTTP 404 error handling (BUG-EXPOSING TEST)
332+
@app.route("/test/http-404-error", methods=["GET"])
333+
def test_http_404_error():
334+
"""Test handling of HTTP 404 error responses.
335+
336+
When urlopen receives a 4xx/5xx response, it raises HTTPError which is
337+
also a valid response object. This tests if the instrumentation correctly
338+
handles this case.
339+
"""
340+
from urllib.error import HTTPError as UrllibHTTPError
341+
try:
342+
with urlopen("https://httpbin.org/status/404", timeout=10) as response:
343+
return jsonify({"status": response.status})
344+
except UrllibHTTPError as e:
345+
# HTTPError is also a response object - we can read its body
346+
body = e.read().decode("utf-8") if e.fp else ""
347+
return jsonify({
348+
"error": True,
349+
"code": e.code,
350+
"reason": e.reason,
351+
"body": body,
352+
}), 200 # Return 200 since we handled the error
353+
except Exception as e:
354+
return jsonify({"error": str(e)}), 500
355+
356+
357+
# Test: HTTP redirect handling (BUG-EXPOSING TEST)
358+
@app.route("/test/http-redirect", methods=["GET"])
359+
def test_http_redirect():
360+
"""Test HTTP redirect handling (301, 302, etc.).
361+
362+
urllib follows redirects automatically. This tests if the instrumentation
363+
correctly handles redirect scenarios.
364+
"""
365+
try:
366+
# httpbin.org/redirect/n redirects n times before returning 200
367+
with urlopen("https://httpbin.org/redirect/2", timeout=10) as response:
368+
data = json.loads(response.read().decode("utf-8"))
369+
return jsonify({
370+
"final_url": response.geturl(),
371+
"status": response.status,
372+
"data": data,
373+
})
374+
except Exception as e:
375+
return jsonify({"error": str(e)}), 500
376+
377+
331378
if __name__ == "__main__":
332379
sdk.mark_app_as_ready()
333380
app.run(host="0.0.0.0", port=8000, debug=False)

drift/instrumentation/urllib/e2e-tests/src/test_requests.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,4 +54,11 @@
5454
# urlopen with data parameter
5555
make_request("POST", "/api/urlopen-with-data")
5656

57+
# Bug-exposing tests (these tests expose bugs in the instrumentation)
58+
# HTTP 404 error handling - tests HTTPError replay
59+
make_request("GET", "/test/http-404-error")
60+
61+
# HTTP redirect handling - tests geturl() after redirects
62+
make_request("GET", "/test/http-redirect")
63+
5764
print_request_summary()

drift/instrumentation/urllib/instrumentation.py

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -654,6 +654,9 @@ def _try_get_mock(
654654
655655
Returns:
656656
Mocked response object if found, None otherwise
657+
658+
Raises:
659+
urllib.error.HTTPError: If the recorded response was an HTTPError
657660
"""
658661
try:
659662
parsed_url = urlparse(url)
@@ -729,9 +732,22 @@ def _try_get_mock(
729732
if mock_response_output.response is None:
730733
logger.debug(f"Mock found but response data is None for {method} {url}")
731734
return None
732-
return self._create_mock_response(mock_response_output.response, url)
735+
736+
# Check if the recorded response was an error (HTTPError, URLError, etc.)
737+
response_data = mock_response_output.response
738+
error_name = response_data.get("errorName")
739+
740+
if error_name == "HTTPError":
741+
# The original request raised HTTPError - we need to raise it too
742+
self._raise_http_error_from_mock(response_data, url)
743+
744+
return self._create_mock_response(response_data, url)
733745

734746
except Exception as e:
747+
# Re-raise HTTPError (and other urllib errors) so they propagate correctly
748+
from urllib.error import HTTPError, URLError
749+
if isinstance(e, (HTTPError, URLError)):
750+
raise
735751
logger.error(f"Error getting mock for {method} {url}: {e}")
736752
return None
737753

@@ -785,6 +801,49 @@ def _create_mock_response(self, mock_data: dict[str, Any], url: str) -> MockHTTP
785801
url=url,
786802
)
787803

804+
def _raise_http_error_from_mock(self, mock_data: dict[str, Any], url: str) -> None:
805+
"""Raise an HTTPError from mocked error response data.
806+
807+
When the original request resulted in an HTTPError (4xx/5xx status codes),
808+
we need to raise the same error during replay so application code that
809+
catches HTTPError behaves the same way.
810+
811+
Args:
812+
mock_data: Mock response data containing errorName and errorMessage
813+
url: Request URL
814+
815+
Raises:
816+
urllib.error.HTTPError: Always raises this exception
817+
"""
818+
from email.message import Message
819+
from urllib.error import HTTPError
820+
821+
error_message = mock_data.get("errorMessage", "")
822+
823+
# Parse status code and reason from error message
824+
# Format: "HTTP Error 404: NOT FOUND"
825+
status_code = 500 # Default
826+
reason = "Internal Server Error"
827+
828+
if error_message.startswith("HTTP Error "):
829+
try:
830+
# Extract "404: NOT FOUND" part
831+
parts = error_message[len("HTTP Error "):].split(":", 1)
832+
status_code = int(parts[0].strip())
833+
if len(parts) > 1:
834+
reason = parts[1].strip()
835+
except (ValueError, IndexError):
836+
pass
837+
838+
# Create empty headers (HTTPError requires headers object)
839+
headers = Message()
840+
841+
# Create empty body
842+
body_fp = BytesIO(b"")
843+
844+
logger.debug(f"Raising HTTPError {status_code} for {url} (replayed from recording)")
845+
raise HTTPError(url, status_code, reason, headers, body_fp)
846+
788847
def _finalize_span(
789848
self,
790849
span: Span,

0 commit comments

Comments
 (0)