Skip to content

fix: correct deadline logic in _wait_for_finish#749

Open
vdusek wants to merge 1 commit intomasterfrom
fix/wait-for-finish-deadline-logic
Open

fix: correct deadline logic in _wait_for_finish#749
vdusek wants to merge 1 commit intomasterfrom
fix/wait-for-finish-deadline-logic

Conversation

@vdusek
Copy link
Copy Markdown
Contributor

@vdusek vdusek commented Apr 22, 2026

Summary

Fixes two bugs in ResourceClient._wait_for_finish / ResourceClientAsync._wait_for_finish:

  1. not_found_deadline anchored at function entry. The 3-second "job not found" grace window was computed once at entry and never reset. A single transient 404 (e.g. replica lag) during a long poll could terminate the wait and return None even though the job was live. The deadline is now lazy — it starts on the first observed 404 and resets after any successful response, so each run of 404s gets its own fresh window.

  2. User wait_duration not checked on 404. When wait_duration was shorter than DEFAULT_WAIT_WHEN_JOB_NOT_EXIST (3s) and the API returned persistent 404s, the loop could overrun the documented contract by up to ~3s. The 404 branch now checks the user deadline first.

Applied symmetrically to the sync and async mirrors.

@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 Apr 22, 2026
@vdusek vdusek self-assigned this Apr 22, 2026
@github-actions github-actions Bot added this to the 139th sprint - Tooling team milestone Apr 22, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 22, 2026

Codecov Report

❌ Patch coverage is 33.33333% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.13%. Comparing base (9ab096a) to head (122f101).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
...apify_client/_resource_clients/_resource_client.py 33.33% 12 Missing ⚠️

❌ Your patch check has failed because the patch coverage (33.33%) is below the target coverage (50.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #749      +/-   ##
==========================================
- Coverage   95.22%   95.13%   -0.09%     
==========================================
  Files          45       45              
  Lines        5154     5164      +10     
==========================================
+ Hits         4908     4913       +5     
- Misses        246      251       +5     
Flag Coverage Δ
integration 95.13% <33.33%> (-0.09%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 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 April 22, 2026 07:48
Make the not-found deadline lazy (starts on the first observed 404 and
resets after any successful response) and honor the user-provided
wait_duration in the 404 branch. This prevents a single transient 404
from aborting a long poll early, and prevents overruns when the user's
deadline is shorter than DEFAULT_WAIT_WHEN_JOB_NOT_EXIST.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@vdusek vdusek force-pushed the fix/wait-for-finish-deadline-logic branch from b9a24dc to 122f101 Compare April 22, 2026 09:23
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.

2 participants