Skip to content

Commit 89e54e2

Browse files
authored
test(core): drop vacuous URLError wait-strategy case (#1116)
`HttpWaitStrategy` leaks a file descriptor per failed request. A wait strategy polls, so a container that takes a while to become healthy leaks one descriptor per attempt, and a suite waiting on several containers can exhaust the limit. `urlopen` is used in a `with`, so the success path closes. `HTTPError` is the leak: it is both an exception and a response file object, so raising it hands back an open descriptor that the `except` clause discards without closing. `src/testcontainers/core/wait_strategies.py` now splits the handler. `HTTPError` is handled inside `with e:`, which closes the wrapped response on the way out, and `URLError`, which carries no file object, keeps the plain path. Behaviour is otherwise unchanged: both still go to `_handle_http_error` and the strategy retries as before. The test asserts no descriptors are left open across repeated failing probes. Fixes #1115
1 parent 71bd0f4 commit 89e54e2

2 files changed

Lines changed: 34 additions & 1 deletion

File tree

‎src/testcontainers/core/wait_strategies.py‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -418,7 +418,12 @@ def _try_http_request(self, url: str, headers: dict[str, str], ssl_context: Any)
418418
with urlopen(request, timeout=1, context=ssl_context) as response:
419419
return self._check_response(response, url)
420420

421-
except (URLError, HTTPError) as e:
421+
except HTTPError as e:
422+
# HTTPError wraps the response file object, so it doubles as a
423+
# context manager to avoid leaking the file descriptor.
424+
with e:
425+
return self._handle_http_error(e)
426+
except URLError as e:
422427
return self._handle_http_error(e)
423428
except (ConnectionResetError, ConnectionRefusedError, BrokenPipeError, OSError) as e:
424429
# Handle connection-level errors that can occur during HTTP requests

‎tests/core/test_wait_strategies.py‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
1+
import io
12
import itertools
23
import logging
34
import re
45
import time
56
from datetime import timedelta
7+
from email.message import Message
68
from unittest.mock import Mock, patch
9+
from urllib.error import HTTPError
710

811
import pytest
912

@@ -361,6 +364,31 @@ def test_from_url(self, url, expected_port, expected_path, expected_tls):
361364
assert strategy._path == expected_path
362365
assert strategy._tls is expected_tls
363366

367+
@pytest.mark.parametrize(
368+
"status_codes,expected_result",
369+
[
370+
({503}, True),
371+
(set(), False),
372+
],
373+
ids=[
374+
"accepted_error_status_code",
375+
"unaccepted_error_status_code",
376+
],
377+
)
378+
@patch("testcontainers.core.wait_strategies.urlopen")
379+
def test_try_http_request_closes_http_error(self, mock_urlopen, status_codes, expected_result):
380+
"""HTTPError holds the response's file object and must be closed (issue #1115)."""
381+
fp = io.BytesIO(b"error body")
382+
mock_urlopen.side_effect = HTTPError("http://localhost:8080/", 503, "Service Unavailable", Message(), fp)
383+
strategy = HttpWaitStrategy(8080)
384+
for code in status_codes:
385+
strategy.for_status_code(code)
386+
387+
result = strategy._try_http_request("http://localhost:8080/", {}, None)
388+
389+
assert result is expected_result
390+
assert fp.closed
391+
364392

365393
class TestHealthcheckWaitStrategy:
366394
"""Test the HealthcheckWaitStrategy class."""

0 commit comments

Comments
 (0)