Skip to content

Commit 886e893

Browse files
author
Alex Wang
committed
fix(insight): make HttpExporter timeout_ms a request deadline
- http_send: build the opener per request with handlers that create deadline-aware connections; a timer shuts the live socket down when the deadline expires and TimeoutError is raised, so a peer trickling bytes can no longer keep a request alive past timeout_ms - every TCP attempt is budgeted with the remaining time and registered with the deadline, so a stalled connect is aborted on time; expiry is judged by the monotonic clock after the request and after the error body read, so a late response is never reported as success - the urllib transport is otherwise unchanged (redirects refused, header merging, IPv6 hosts, proxy settings, default TLS context) - tests: trickled status line, trickled error body, slow name resolution, stalled connect, header case merge, unsupported scheme
1 parent 15fa221 commit 886e893

3 files changed

Lines changed: 384 additions & 12 deletions

File tree

packages/aws-durable-execution-sdk-python-insight/src/aws_durable_execution_sdk_python_insight/exporters/_common.py

Lines changed: 222 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,16 @@
1111
from __future__ import annotations
1212

1313
import datetime
14+
import http.client
1415
import json
1516
import re
17+
import socket
18+
import threading
19+
import time
1620
import urllib.error
1721
import urllib.request
1822
from typing import Any
23+
from urllib.parse import urlsplit
1924

2025

2126
_SQL_IDENTIFIER = re.compile(r"^[a-zA-Z_][a-zA-Z0-9_]*$")
@@ -36,7 +41,179 @@ def redirect_request( # type: ignore[override] # stdlib signature has no hints
3641
return None
3742

3843

39-
_OPENER = urllib.request.build_opener(_NoRedirect())
44+
class _Deadline:
45+
"""A whole-request time budget shared by the timer and the connection.
46+
47+
``expired()`` is judged by the monotonic clock, so a response that lands
48+
after the budget is never accepted even if the timer callback runs late.
49+
The timer's job is only to wake a blocked socket.
50+
"""
51+
52+
def __init__(self, seconds: float) -> None:
53+
self.seconds = seconds
54+
self.expires_at = time.monotonic() + seconds
55+
self._fired = threading.Event()
56+
self._lock = threading.Lock()
57+
self._sockets: list[socket.socket] = []
58+
59+
def expired(self) -> bool:
60+
return self._fired.is_set() or time.monotonic() >= self.expires_at
61+
62+
def remaining(self) -> float:
63+
# Never hand the socket layer zero or a negative value: those mean
64+
# non-blocking / blocking, not "no time left".
65+
return max(self.expires_at - time.monotonic(), 0.001)
66+
67+
def register(self, sock: socket.socket) -> None:
68+
"""Make ``sock`` reachable by ``fire`` (an in-flight connect or the live socket)."""
69+
with self._lock:
70+
self._sockets.append(sock)
71+
already_fired = self._fired.is_set()
72+
if already_fired:
73+
_shutdown(sock)
74+
75+
def fire(self) -> None:
76+
self._fired.set()
77+
with self._lock:
78+
sockets = list(self._sockets)
79+
for sock in sockets:
80+
_shutdown(sock)
81+
82+
83+
def _shutdown(sock: socket.socket) -> None:
84+
try:
85+
sock.shutdown(socket.SHUT_RDWR)
86+
except OSError:
87+
pass
88+
89+
90+
def _connect_within(
91+
deadline: _Deadline,
92+
address: tuple[str, int],
93+
timeout: float | None,
94+
source_address: tuple[str, int] | None = None,
95+
) -> socket.socket:
96+
"""``socket.create_connection`` with the remaining budget per attempt.
97+
98+
Every candidate socket is registered with the deadline before connecting,
99+
so the timer can abort an attempt that is still waiting for the peer. Name
100+
resolution itself cannot be interrupted.
101+
"""
102+
del timeout # the deadline, not the connection's static timeout, rules here
103+
host, port = address
104+
if deadline.expired():
105+
msg = "deadline expired before connecting"
106+
raise TimeoutError(msg)
107+
last_error: OSError | None = None
108+
for family, kind, proto, _, sockaddr in socket.getaddrinfo(
109+
host, port, 0, socket.SOCK_STREAM
110+
):
111+
if deadline.expired():
112+
msg = "deadline expired while connecting"
113+
raise TimeoutError(msg)
114+
sock = socket.socket(family, kind, proto)
115+
try:
116+
sock.settimeout(deadline.remaining())
117+
if source_address:
118+
sock.bind(source_address)
119+
deadline.register(sock)
120+
sock.connect(sockaddr)
121+
except OSError as exc:
122+
sock.close()
123+
last_error = exc
124+
continue
125+
return sock
126+
if last_error is not None:
127+
raise last_error
128+
msg = f"getaddrinfo returned no addresses for {host!r}"
129+
raise OSError(msg)
130+
131+
132+
def _bind_deadline(
133+
conn: http.client.HTTPConnection, deadline: _Deadline | None
134+
) -> None:
135+
# ``_create_connection`` is the connection's socket factory; swapping it
136+
# keeps the stdlib connect (TLS wrapping, ALPN, tunnelling) intact while
137+
# every TCP attempt is budgeted and interruptible.
138+
if deadline is not None and hasattr(conn, "_create_connection"):
139+
conn._create_connection = ( # type: ignore[attr-defined] # stdlib hook
140+
lambda address, timeout=None, source_address=None: _connect_within(
141+
deadline, address, timeout, source_address
142+
)
143+
)
144+
145+
146+
def _after_connect(
147+
conn: http.client.HTTPConnection, deadline: _Deadline | None
148+
) -> None:
149+
if deadline is None:
150+
return
151+
if conn.sock is not None:
152+
deadline.register(conn.sock) # the (possibly TLS-wrapped) live socket
153+
if deadline.expired():
154+
conn.close()
155+
msg = "deadline expired while connecting"
156+
raise TimeoutError(msg)
157+
158+
159+
class _HTTPConnection(http.client.HTTPConnection):
160+
def __init__(self, *args: Any, deadline: _Deadline | None, **kwargs: Any) -> None:
161+
super().__init__(*args, **kwargs)
162+
self._deadline = deadline
163+
_bind_deadline(self, deadline)
164+
165+
def connect(self) -> None:
166+
super().connect()
167+
_after_connect(self, self._deadline)
168+
169+
170+
class _HTTPSConnection(http.client.HTTPSConnection):
171+
def __init__(self, *args: Any, deadline: _Deadline | None, **kwargs: Any) -> None:
172+
super().__init__(*args, **kwargs)
173+
self._deadline = deadline
174+
_bind_deadline(self, deadline)
175+
176+
def connect(self) -> None:
177+
super().connect()
178+
_after_connect(self, self._deadline)
179+
180+
181+
class _HTTPHandler(urllib.request.HTTPHandler):
182+
def __init__(self, deadline: _Deadline | None) -> None:
183+
super().__init__()
184+
self._deadline = deadline
185+
186+
def http_open(self, req: urllib.request.Request) -> http.client.HTTPResponse:
187+
deadline = self._deadline
188+
189+
def factory(*args: Any, **kwargs: Any) -> _HTTPConnection:
190+
return _HTTPConnection(*args, deadline=deadline, **kwargs)
191+
192+
return self.do_open(factory, req) # type: ignore[arg-type] # factory, not class
193+
194+
195+
class _HTTPSHandler(urllib.request.HTTPSHandler):
196+
def __init__(self, deadline: _Deadline | None) -> None:
197+
super().__init__()
198+
self._deadline = deadline
199+
200+
def https_open(self, req: urllib.request.Request) -> http.client.HTTPResponse:
201+
deadline = self._deadline
202+
203+
def factory(*args: Any, **kwargs: Any) -> _HTTPSConnection:
204+
return _HTTPSConnection(*args, deadline=deadline, **kwargs)
205+
206+
# No context is configured on this handler, so the connection builds the
207+
# stdlib default (certificate verification, ALPN http/1.1).
208+
return self.do_open(factory, req) # type: ignore[arg-type] # factory, not class
209+
210+
211+
def _opener(deadline: _Deadline | None) -> urllib.request.OpenerDirector:
212+
# build_opener keeps the default handlers (proxy discovery, header merging,
213+
# IPv6 hosts, error handling) and swaps in ours where classes overlap.
214+
return urllib.request.build_opener(
215+
_NoRedirect(), _HTTPHandler(deadline), _HTTPSHandler(deadline)
216+
)
40217

41218

42219
def compact_dumps(value: Any) -> str:
@@ -89,17 +266,53 @@ def http_send(
89266
message. Redirects are not followed: a 3xx is returned like any other
90267
failure. ``error_text`` is the first ``_MAX_ERROR_BODY_BYTES`` of a non-2xx
91268
response body and empty on success; a success body is never read. Network
92-
errors and timeouts propagate.
269+
errors propagate.
270+
271+
``timeout`` (seconds) is a deadline for the whole request: connecting,
272+
sending, and receiving the status, headers and any error body. On expiry
273+
the live socket is shut down and ``TimeoutError`` is raised, even against
274+
a peer that keeps the connection alive by trickling bytes, and a response
275+
that completes after the deadline is never reported as success. Name
276+
resolution cannot be interrupted. ``None`` means no limit.
93277
"""
278+
scheme = urlsplit(url).scheme
279+
if scheme not in ("http", "https"):
280+
msg = f"Unsupported URL scheme {scheme!r} in {url!r} (need http or https)"
281+
raise ValueError(msg)
94282
request = urllib.request.Request(url, data=body, method=method)
95283
for key, value in headers.items():
96284
request.add_header(key, value)
285+
286+
deadline = _Deadline(timeout) if timeout is not None else None
287+
timer: threading.Timer | None = None
288+
if deadline is not None:
289+
timer = threading.Timer(deadline.seconds, deadline.fire)
290+
timer.daemon = True
291+
timer.start()
292+
timed_out = f"request to {url} exceeded {timeout}s"
97293
try:
98-
with _OPENER.open(request, timeout=timeout) as response: # noqa: S310
99-
return int(response.status), str(response.reason or ""), ""
100-
except urllib.error.HTTPError as exc:
101294
try:
102-
detail = exc.read(_MAX_ERROR_BODY_BYTES).decode("utf-8", errors="replace")
103-
except Exception: # noqa: BLE001 - the body is best-effort detail only
104-
detail = ""
105-
return int(exc.code), str(exc.reason or ""), detail
295+
with _opener(deadline).open(request, timeout=timeout) as response: # noqa: S310
296+
status = int(response.status)
297+
reason = str(response.reason or "")
298+
detail = ""
299+
except urllib.error.HTTPError as exc:
300+
status = int(exc.code)
301+
reason = str(exc.reason or "")
302+
try:
303+
detail = exc.read(_MAX_ERROR_BODY_BYTES).decode(
304+
"utf-8", errors="replace"
305+
)
306+
except Exception: # noqa: BLE001 - the body is best-effort detail only
307+
detail = ""
308+
except Exception as exc:
309+
if deadline is not None and deadline.expired():
310+
raise TimeoutError(timed_out) from exc
311+
raise
312+
finally:
313+
if timer is not None:
314+
timer.cancel()
315+
# Whatever arrived after the deadline is not a delivery.
316+
if deadline is not None and deadline.expired():
317+
raise TimeoutError(timed_out)
318+
return status, reason, detail

packages/aws-durable-execution-sdk-python-insight/src/aws_durable_execution_sdk_python_insight/exporters/http_exporter.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,11 @@ class HttpMethod(StrEnum):
3333
class HttpExporter:
3434
"""Sends each record as a JSON body to any HTTP endpoint.
3535
36-
The endpoint must answer 2xx; any other status raises. ``timeout_ms``
37-
bounds the whole request (default 10 seconds). ``max_record_size_bytes``
36+
The endpoint must answer 2xx; any other status raises. ``timeout_ms`` is a
37+
deadline for the whole request (connect, send, response headers and any
38+
error body; default 10 seconds); on expiry the connection is shut down and
39+
``TimeoutError`` is raised. Name resolution is not interruptible.
40+
``max_record_size_bytes``
3841
has no default because a generic endpoint has no known limit.
3942
"""
4043

0 commit comments

Comments
 (0)