Skip to content

Commit cf4d27f

Browse files
Alex Wangwangyb-A
authored andcommitted
docs(insight): state HttpExporter timeout_ms scope
The HttpExporter docstring said timeout_ms "bounds the whole request". It does not: http_send passes it to urllib, which applies it to each blocking socket operation. An endpoint that stops reading or goes silent fails at timeout_ms, but one that keeps consuming or sending bytes slowly can hold the request open for longer (measured in review of #720: 37 s with timeout_ms=2000 against a 1 byte/s server). - HttpExporter docstring: name the phases the timeout applies to (the connect, each write of the request, each read of the status line, headers, and a non-2xx error body; a 2xx body is never read), note that a future release may enforce timeout_ms as a whole-request deadline, and tell callers to size the function timeout with this in mind - http_send docstring: same per-operation description - README HttpExporter section: same caveat and reservation - rename test_timeout_is_enforced to test_timeout_applies_to_a_silent_peer, which is the case it exercises; no assertion changed No behaviour change.
1 parent 86ea45c commit cf4d27f

4 files changed

Lines changed: 23 additions & 5 deletions

File tree

packages/aws-durable-execution-sdk-python-insight/README.md

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,12 @@ OTelExporter(endpoint="https://otlp.vendor.com/v1/logs", headers={"x-api-key": "
183183

184184
`POST` (or `method="PUT"`) the record as JSON to `url` with
185185
`Content-Type: application/json` plus `headers`; a non-2xx status raises.
186-
`timeout_ms` defaults to 10000. No IAM.
186+
`timeout_ms` defaults to 10000 and currently applies to each socket operation
187+
(the connect, each write of the request, and each read of the status line,
188+
headers, and a non-2xx error body; a 2xx body is never read) rather than to the
189+
whole request, so an endpoint that consumes or responds slowly can hold the
190+
export open longer than `timeout_ms`; a future release may enforce it as a
191+
whole-request deadline. No IAM.
187192

188193
```python
189194
HttpExporter(url="https://hooks.example.com/insight", headers={"Authorization": "Bearer ..."})

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,11 @@ def http_send(
8989
message. Redirects are not followed: a 3xx is returned like any other
9090
failure. ``error_text`` is the first ``_MAX_ERROR_BODY_BYTES`` of a non-2xx
9191
response body and empty on success; a success body is never read. Network
92-
errors and timeouts propagate.
92+
errors and timeouts propagate. ``timeout`` is ``urllib``'s per-socket-
93+
operation timeout: it bounds the connect, each write of the request, and
94+
each read of the status line, headers, and any error body separately, not
95+
the request as a whole. A peer that keeps consuming or sending bytes
96+
slowly can outlive it.
9397
"""
9498
request = urllib.request.Request(url, data=body, method=method)
9599
for key, value in headers.items():

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

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,17 @@ class HttpExporter:
3434
"""Sends each record as a JSON body to any HTTP endpoint.
3535
3636
The endpoint must answer 2xx; any other status raises. ``timeout_ms``
37-
bounds the whole request (default 10 seconds). ``max_record_size_bytes``
38-
has no default because a generic endpoint has no known limit.
37+
(default 10 seconds) is currently applied to each blocking socket
38+
operation, not to the request as a whole: the connect, each write while
39+
sending the request (headers and the record body), and each read of the
40+
status line and headers, plus of the error body on a non-2xx response. A
41+
successful response body is never read. An endpoint that stops reading or
42+
goes silent fails at ``timeout_ms``, but one that keeps consuming or
43+
sending bytes slowly can hold the request open for longer. A future
44+
release may enforce ``timeout_ms`` as a deadline for the whole request, so
45+
do not rely on a request being allowed to exceed it. Size the function
46+
timeout with this in mind. ``max_record_size_bytes`` has no default because
47+
a generic endpoint has no known limit.
3948
"""
4049

4150
def __init__(

packages/aws-durable-execution-sdk-python-insight/tests/test_http_exporter.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ def test_non_2xx_response_raises(http_capture: HttpCapture) -> None:
181181
exporter.export(_record())
182182

183183

184-
def test_timeout_is_enforced(http_capture: HttpCapture) -> None:
184+
def test_timeout_applies_to_a_silent_peer(http_capture: HttpCapture) -> None:
185185
http_capture.delay_seconds = 1.0
186186
exporter = HttpExporter(url=http_capture.url, timeout_ms=100)
187187
with pytest.raises(TimeoutError):

0 commit comments

Comments
 (0)