Skip to content

Commit 24dd614

Browse files
authored
fix: prevent Actor log-streaming thread from crashing on stream timeout (#944)
## Summary After a successful `ActorClient.call()`, the background log-streaming thread could crash with an uncaught `impit.TimeoutException`, printing a traceback even though the run finished fine. ## Issue Closes: #945 ## Root cause The log stream was requested with a bounded 30s timeout. impit applies `timeout` to the *whole* request, including the streamed body, so any run streaming logs longer than that tripped `impit.TimeoutException` mid-stream — unhandled in the sync thread, and a spurious `ERROR` + traceback in the async twin. Raising the value wouldn't help: every tier is capped at `DEFAULT_TIMEOUT_MAX` (360s). ## Fix - Request the stream with `no_timeout` (impit's ~24h ceiling), so it runs until the server closes it with EOF at run finish. Matches the JS client. - Catch `impit.TimeoutException` in both paths and log a `WARNING` instead of crashing / error-logging; other failures still error-log. - Run the sync streaming thread as a daemon so a stalled read can't block interpreter shutdown. ## Known limitation A parked sync `iter_bytes()` read can't be interrupted from another thread, so with `no_timeout` a manual `StreamedLog.stop()` on a momentarily silent stream waits for the next chunk or EOF rather than the old 30s bound. The `ActorClient.call()` path is unaffected (run finish sends EOF). Matches the JS client.
1 parent 106afff commit 24dd614

2 files changed

Lines changed: 182 additions & 25 deletions

File tree

src/apify_client/_streamed_log.py

Lines changed: 39 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5,23 +5,34 @@
55
import re
66
import threading
77
from asyncio import Task
8-
from datetime import UTC, datetime, timedelta
8+
from datetime import UTC, datetime
99
from threading import Thread
1010
from typing import TYPE_CHECKING, ClassVar, Self, cast
1111

12+
import impit
13+
1214
from apify_client._docs import docs_group
1315

1416
if TYPE_CHECKING:
1517
from types import TracebackType
1618

1719
from apify_client._resource_clients import LogClient, LogClientAsync
20+
from apify_client.types import Timeout
1821

1922

2023
class StreamedLogBase:
2124
"""Base class for streaming and buffering chunked Actor run logs."""
2225

23-
# Test related flag to enable propagation of logs to the `caplog` fixture during tests.
2426
_force_propagate = False
27+
"""Test related flag to enable propagation of logs to the `caplog` fixture during tests."""
28+
29+
_stream_timeout: ClassVar[Timeout] = 'no_timeout'
30+
"""Timeout for the log-stream long-poll request, which stays open for the whole Actor run.
31+
32+
impit applies its `timeout` to the whole request including the streamed body, so any bounded value truncates a
33+
longer run mid-stream and raises `impit.TimeoutException` (#1040). `no_timeout` maps to impit's ~24h cap, which
34+
is effectively unbounded for real runs and mirrors the JS client.
35+
"""
2536

2637
def __init__(self, to_logger: logging.Logger, *, from_start: bool = True) -> None:
2738
if self._force_propagate:
@@ -90,10 +101,6 @@ class StreamedLog(StreamedLogBase):
90101
call `start` and `stop` manually. Obtain an instance via `RunClient.get_streamed_log`.
91102
"""
92103

93-
# Caps how long `iter_bytes()` can block on a silent stream so `stop()` can unblock within
94-
# this window instead of waiting for the long-polling default.
95-
_read_timeout: ClassVar[timedelta] = timedelta(seconds=30)
96-
97104
def __init__(self, log_client: LogClient, *, to_logger: logging.Logger, from_start: bool = True) -> None:
98105
"""Initialize `StreamedLog`.
99106
@@ -117,7 +124,8 @@ def start(self) -> Thread:
117124
if self._streaming_thread:
118125
raise RuntimeError('Streaming thread already active')
119126
self._stop_logging = False
120-
self._streaming_thread = threading.Thread(target=self._stream_log)
127+
# A daemon thread so a stream still blocked on a read can never hold up interpreter shutdown.
128+
self._streaming_thread = threading.Thread(target=self._stream_log, daemon=True)
121129
self._streaming_thread.start()
122130
return self._streaming_thread
123131

@@ -142,17 +150,25 @@ def __exit__(
142150
self.stop()
143151

144152
def _stream_log(self) -> None:
145-
with self._log_client.stream(raw=True, timeout=self._read_timeout) as log_stream:
146-
if not log_stream:
147-
return
148-
try:
149-
for data in log_stream.iter_bytes():
150-
self._process_new_data(data)
151-
if self._stop_logging:
152-
break
153-
finally:
154-
# Flush the last buffered part even if the read timed out or was stopped.
155-
self._log_buffer_content(include_last_part=True)
153+
try:
154+
with self._log_client.stream(raw=True, timeout=self._stream_timeout) as log_stream:
155+
if not log_stream:
156+
return
157+
try:
158+
for data in log_stream.iter_bytes():
159+
self._process_new_data(data)
160+
if self._stop_logging:
161+
break
162+
finally:
163+
# Flush the last buffered part even if the read timed out or was stopped.
164+
self._log_buffer_content(include_last_part=True)
165+
except impit.TimeoutException:
166+
# With `no_timeout` this fires only if the run outlives impit's ~24h cap or the connection stalls.
167+
# The stream cannot continue, so warn and let the thread end instead of leaking a traceback (#1040).
168+
self._to_logger.warning('Log streaming stopped: the log stream request timed out.')
169+
except Exception:
170+
# Any other failure in log redirection must not escape the background thread; log it instead.
171+
self._to_logger.exception('Log redirection stopped due to unexpected error:')
156172

157173

158174
@docs_group('Other')
@@ -216,7 +232,7 @@ async def __aexit__(
216232

217233
async def _stream_log(self) -> None:
218234
try:
219-
async with self._log_client.stream(raw=True) as log_stream:
235+
async with self._log_client.stream(raw=True, timeout=self._stream_timeout) as log_stream:
220236
if not log_stream:
221237
return
222238
try:
@@ -225,6 +241,10 @@ async def _stream_log(self) -> None:
225241
finally:
226242
# Flush the last buffered part even if the task is cancelled by `stop()`.
227243
self._log_buffer_content(include_last_part=True)
244+
except impit.TimeoutException:
245+
# As in `StreamedLog._stream_log`, impit's whole-request timeout on the long-lived stream is an
246+
# expected terminal condition, not an error, so log a warning and end the task instead of a traceback.
247+
self._to_logger.warning('Log streaming stopped: the log stream request timed out.')
228248
except Exception:
229249
# Exception in log redirection should not propagate further.
230250
self._to_logger.exception('Log redirection stopped due to unexpected error:')

tests/unit/test_logging.py

Lines changed: 143 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
from apify_client import ApifyClient, ApifyClientAsync
1616
from apify_client._logging import RedirectLogFormatter
1717
from apify_client._status_message_watcher import StatusMessageWatcherBase
18-
from apify_client._streamed_log import StreamedLog, StreamedLogBase
18+
from apify_client._streamed_log import StreamedLog, StreamedLogAsync, StreamedLogBase
1919

2020
if TYPE_CHECKING:
2121
from collections.abc import Iterator
@@ -820,13 +820,13 @@ def generate_logs() -> Iterator[bytes]:
820820
assert any(_TAIL_SECOND_MESSAGE in m for m in messages), f'Buffered tail dropped on async stop(). Got: {messages}'
821821

822822

823-
def test_streamed_log_sync_stop_does_not_hang_on_silent_stream(
823+
def test_streamed_log_sync_stop_unblocks_on_finite_stream_timeout(
824824
httpserver: HTTPServer,
825825
monkeypatch: pytest.MonkeyPatch,
826826
) -> None:
827-
"""Verify `stop()` returns promptly even when the underlying stream is silent (no chunks)."""
828-
# Shorten the read timeout so the test doesn't wait for the production default.
829-
monkeypatch.setattr(StreamedLog, '_read_timeout', timedelta(seconds=1))
827+
"""A finite `_stream_timeout` bounds how long `stop()` waits on a silent stream, since the blocking read cannot
828+
otherwise be interrupted (the production default is `no_timeout`, so the test configures a short finite one)."""
829+
monkeypatch.setattr(StreamedLog, '_stream_timeout', timedelta(seconds=1))
830830

831831
release_server = threading.Event()
832832

@@ -857,6 +857,143 @@ def generate_logs() -> Iterator[bytes]:
857857
stop_thread = threading.Thread(target=streamed_log.stop)
858858
stop_thread.start()
859859
stop_thread.join(timeout=5)
860-
assert not stop_thread.is_alive(), 'stop() hangs when the underlying stream is silent'
860+
assert not stop_thread.is_alive(), 'stop() did not unblock within the finite stream timeout'
861861
finally:
862862
release_server.set()
863+
864+
865+
@pytest.mark.usefixtures('propagate_stream_logs')
866+
def test_streamed_log_sync_does_not_leak_exception_on_stream_timeout(
867+
caplog: LogCaptureFixture,
868+
httpserver: HTTPServer,
869+
monkeypatch: pytest.MonkeyPatch,
870+
) -> None:
871+
"""The streaming thread ends quietly when the log-stream request hits its total timeout (regression #1040)."""
872+
# impit enforces a whole-request timeout, so a still-running Actor whose run outlives the timeout makes
873+
# `iter_bytes()` raise `impit.TimeoutException`. Shorten the timeout to trigger this quickly.
874+
monkeypatch.setattr(StreamedLog, '_stream_timeout', timedelta(seconds=1))
875+
876+
release_server = threading.Event()
877+
878+
def _slow_handler(_request: Request) -> Response:
879+
def generate_logs() -> Iterator[bytes]:
880+
# Emit one complete line, then keep the connection open (as a running Actor would) past the
881+
# client-side total timeout without sending anything more.
882+
yield b'2025-05-13T07:24:12.588Z ACTOR: still running\n'
883+
release_server.wait(timeout=30)
884+
885+
return Response(response=generate_logs(), status=200, mimetype='application/octet-stream')
886+
887+
httpserver.expect_request(
888+
f'/v2/actor-runs/{_MOCKED_RUN_ID}/log', method='GET', query_string='stream=true&raw=true'
889+
).respond_with_handler(_slow_handler)
890+
_register_run_and_actor_endpoints(httpserver)
891+
892+
api_url = httpserver.url_for('/').removesuffix('/')
893+
run_client = ApifyClient(token='mocked_token', api_url=api_url).run(run_id=_MOCKED_RUN_ID)
894+
streamed_log = run_client.get_streamed_log()
895+
logger_name = f'apify.{_MOCKED_ACTOR_NAME}-{_MOCKED_RUN_ID}'
896+
897+
thread_exceptions: list[threading.ExceptHookArgs] = []
898+
monkeypatch.setattr(threading, 'excepthook', thread_exceptions.append)
899+
900+
try:
901+
with caplog.at_level(logging.DEBUG, logger=logger_name):
902+
thread = streamed_log.start()
903+
# Wait past the 1s total timeout so the streaming request fails inside the thread.
904+
thread.join(timeout=5)
905+
assert not thread.is_alive(), 'streaming thread did not end after the stream timed out'
906+
finally:
907+
release_server.set()
908+
streamed_log.stop()
909+
910+
leaked = [args.exc_type.__name__ for args in thread_exceptions]
911+
assert not leaked, f'streaming thread leaked an uncaught exception: {leaked}'
912+
# The timeout is expected, so it must be swallowed quietly, not funnelled through the generic error handler.
913+
error_records = [r for r in caplog.records if r.levelno >= logging.ERROR and 'Log redirection stopped' in r.message]
914+
assert not error_records, f'sync thread logged an error on stream timeout: {[r.message for r in error_records]}'
915+
# The line received before the timeout must still have been redirected.
916+
assert any('ACTOR: still running' in record.message for record in caplog.records)
917+
918+
919+
@pytest.mark.usefixtures('propagate_stream_logs')
920+
def test_streamed_log_sync_requests_stream_with_no_timeout(
921+
httpserver: HTTPServer,
922+
monkeypatch: pytest.MonkeyPatch,
923+
) -> None:
924+
"""The log stream is requested with `no_timeout`, so a long run is not truncated mid-stream (#1040)."""
925+
httpserver.expect_request(
926+
f'/v2/actor-runs/{_MOCKED_RUN_ID}/log', method='GET', query_string='stream=true&raw=true'
927+
).respond_with_data(b'2025-05-13T07:24:12.588Z ACTOR: done\n', content_type='application/octet-stream')
928+
_register_run_and_actor_endpoints(httpserver)
929+
930+
api_url = httpserver.url_for('/').removesuffix('/')
931+
run_client = ApifyClient(token='mocked_token', api_url=api_url).run(run_id=_MOCKED_RUN_ID)
932+
933+
# Capture the timeout the log stream is requested with. impit applies it to the whole request (body included),
934+
# so anything but `no_timeout` would cut a long run off mid-stream, which is the root cause of #1040.
935+
log_stream_timeouts: list[object] = []
936+
original_call = run_client._http_client.call
937+
938+
def _recording_call(**kwargs: object) -> object:
939+
if str(kwargs.get('url', '')).endswith('/log'):
940+
log_stream_timeouts.append(kwargs.get('timeout'))
941+
return original_call(**kwargs)
942+
943+
monkeypatch.setattr(run_client._http_client, 'call', _recording_call)
944+
945+
streamed_log = run_client.get_streamed_log()
946+
thread = streamed_log.start()
947+
thread.join(timeout=5)
948+
streamed_log.stop()
949+
950+
assert log_stream_timeouts == ['no_timeout'], (
951+
f'log stream requested with timeout={log_stream_timeouts}, expected no_timeout so long runs are not truncated'
952+
)
953+
954+
955+
@pytest.mark.usefixtures('propagate_stream_logs')
956+
async def test_streamed_log_async_does_not_error_on_stream_timeout(
957+
caplog: LogCaptureFixture,
958+
httpserver: HTTPServer,
959+
monkeypatch: pytest.MonkeyPatch,
960+
) -> None:
961+
"""The async streaming task ends quietly on a stream-request timeout, matching the sync regression for #1040."""
962+
monkeypatch.setattr(StreamedLogAsync, '_stream_timeout', timedelta(seconds=1))
963+
964+
release_server = threading.Event()
965+
966+
def _slow_handler(_request: Request) -> Response:
967+
def generate_logs() -> Iterator[bytes]:
968+
# Emit one complete line, then keep the connection open past the client-side total timeout.
969+
yield b'2025-05-13T07:24:12.588Z ACTOR: still running\n'
970+
release_server.wait(timeout=30)
971+
972+
return Response(response=generate_logs(), status=200, mimetype='application/octet-stream')
973+
974+
httpserver.expect_request(
975+
f'/v2/actor-runs/{_MOCKED_RUN_ID}/log', method='GET', query_string='stream=true&raw=true'
976+
).respond_with_handler(_slow_handler)
977+
_register_run_and_actor_endpoints(httpserver)
978+
979+
api_url = httpserver.url_for('/').removesuffix('/')
980+
run_client = ApifyClientAsync(token='mocked_token', api_url=api_url).run(run_id=_MOCKED_RUN_ID)
981+
streamed_log = await run_client.get_streamed_log()
982+
logger_name = f'apify.{_MOCKED_ACTOR_NAME}-{_MOCKED_RUN_ID}'
983+
984+
try:
985+
with caplog.at_level(logging.DEBUG, logger=logger_name):
986+
task = streamed_log.start()
987+
# The 1s total timeout fails the request inside the task; it must end on its own without our help.
988+
done, _pending = await asyncio.wait({task}, timeout=5)
989+
assert task in done, 'async streaming task did not end after the stream timed out'
990+
finally:
991+
release_server.set()
992+
await streamed_log.stop()
993+
994+
assert not task.cancelled()
995+
assert task.exception() is None, f'async streaming task raised on stream timeout: {task.exception()!r}'
996+
error_records = [r for r in caplog.records if r.levelno >= logging.ERROR and 'Log redirection stopped' in r.message]
997+
assert not error_records, f'async task logged an error on stream timeout: {[r.message for r in error_records]}'
998+
# The line received before the timeout must still have been redirected.
999+
assert any('ACTOR: still running' in record.message for record in caplog.records)

0 commit comments

Comments
 (0)