Skip to content

fix: prevent Actor log-streaming thread from crashing on stream timeout#944

Merged
vdusek merged 3 commits into
masterfrom
fix/streamed-log-stream-timeout
Jul 13, 2026
Merged

fix: prevent Actor log-streaming thread from crashing on stream timeout#944
vdusek merged 3 commits into
masterfrom
fix/streamed-log-stream-timeout

Conversation

@vdusek

@vdusek vdusek commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

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.

@vdusek vdusek added adhoc Ad-hoc unplanned task added during the sprint. t-tooling Issues with this label are in the ownership of the tooling team. labels Jul 11, 2026
@vdusek vdusek self-assigned this Jul 11, 2026
@codecov

codecov Bot commented Jul 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 82.60870% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.54%. Comparing base (68c0e24) to head (e45d675).
⚠️ Report is 12 commits behind head on master.

Files with missing lines Patch % Lines
src/apify_client/_streamed_log.py 82.60% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #944      +/-   ##
==========================================
- Coverage   94.56%   94.54%   -0.03%     
==========================================
  Files          48       48              
  Lines        5119     5129      +10     
==========================================
+ Hits         4841     4849       +8     
- Misses        278      280       +2     
Flag Coverage Δ
integration 92.86% <65.21%> (-0.17%) ⬇️
unit 83.42% <82.60%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@vdusek vdusek requested a review from Pijukatel July 13, 2026 07:56
@vdusek vdusek marked this pull request as ready for review July 13, 2026 07:56
@vdusek vdusek merged commit 24dd614 into master Jul 13, 2026
30 checks passed
@vdusek vdusek deleted the fix/streamed-log-stream-timeout branch July 13, 2026 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

adhoc Ad-hoc unplanned task added during the sprint. t-tooling Issues with this label are in the ownership of the tooling team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

impit.TimeoutException in _streamed_log.py after successful actor completion

3 participants