From 427e175f15e939287837b140c3073f0274f47de6 Mon Sep 17 00:00:00 2001 From: Imran Siddique Date: Sun, 6 Sep 2026 10:49:52 -0700 Subject: [PATCH] test: let the drain-ceiling test survive the reset it documents test_far_oversized_request_beyond_the_drain_ceiling_still_gets_a_clean_response fails about one run in four on an unmodified tree, and it took main red after the 0.5.0 release with WinError 10053 on windows-latest. Its own comment already said the right thing: above the drain ceiling the server stops draining and closes, so a connection-level failure is an acceptable client-side outcome rather than a defect. Only sendall was actually allowed one. The recv was not, so when the reset arrived before the buffered 413 could be read the test failed on a behaviour it was written to permit. recv now gets the same treatment, and the 413 assertion applies only when bytes actually came back. That alone would weaken the test to passing on silence, so it gains the assertion that carries the real contract: a 15 MB body must not wedge the server. Whichever path the oversized request took, a following valid request is still served. That holds on both sides of the race and is the property worth protecting. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01XbDBXDWWvMFa7c2jGgyq9t --- tests/unit/test_mock_upstream_gate.py | 28 +++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_mock_upstream_gate.py b/tests/unit/test_mock_upstream_gate.py index f953832..c0a0c1b 100644 --- a/tests/unit/test_mock_upstream_gate.py +++ b/tests/unit/test_mock_upstream_gate.py @@ -226,8 +226,32 @@ def test_far_oversized_request_beyond_the_drain_ceiling_still_gets_a_clean_respo # a defect. with contextlib.suppress(BrokenPipeError, ConnectionResetError): sock.sendall(body) - raw_response = sock.recv(65536) - assert b"413" in raw_response.split(b"\r\n", 1)[0] + # The same applies to the read. Once the server has stopped draining + # and closed, the reset can arrive before the buffered 413 is read, + # and on Windows that surfaces as ConnectionAbortedError (WinError + # 10053) rather than an empty read. The comment above always said a + # connection-level failure was acceptable here; only sendall was + # actually allowed one, which made this test fail about one run in + # four on an unmodified tree. + raw_response = b"" + with contextlib.suppress( + ConnectionResetError, ConnectionAbortedError, TimeoutError, OSError + ): + raw_response = sock.recv(65536) + + # If a response did come back it must be the rejection, never an + # acceptance. If the connection was torn down first, that is the + # give-up-and-close path this test exists to cover. + if raw_response: + assert b"413" in raw_response.split(b"\r\n", 1)[0] + + # The contract that holds either way: a 15 MB body must not wedge the + # server. Whichever path the request above took, the next one is served + # normally. This is what makes the test meaningful when the connection is + # reset before any bytes are read. + status, resp = _post(upstream, VALID_REQUEST) + assert status == 200 + assert resp["id"] == "req-1" def test_request_at_the_limit_is_accepted(upstream):