Skip to content

fix: prevent infinite loop in _wait_for_finish on persistent 404s#619

Merged
vdusek merged 1 commit intomasterfrom
fix/wait-for-finish-infinite-loop
Feb 17, 2026
Merged

fix: prevent infinite loop in _wait_for_finish on persistent 404s#619
vdusek merged 1 commit intomasterfrom
fix/wait-for-finish-infinite-loop

Conversation

@vdusek
Copy link
Contributor

@vdusek vdusek commented Feb 17, 2026

Summary

  • _wait_for_finish could spin indefinitely when polling a non-existent job
  • seconds_elapsed was only updated inside the try block (on success), so on persistent 404 errors the timeout check seconds_elapsed > 3 always evaluated as 0 > 3 = False
  • Moved the seconds_elapsed update into a finally block so it is always updated regardless of success or failure (both sync and async variants)

Test plan

  • Verify existing unit tests pass
  • Verify the fix by testing wait_for_finish on a non-existent run/build ID — should return None after ~3 seconds instead of looping forever

🤖 Generated with Claude Code

… in _wait_for_finish

When _wait_for_finish polls a job that consistently returns 404, seconds_elapsed
was never updated because the update was only inside the try block on success.
This caused the timeout check (seconds_elapsed > DEFAULT_WAIT_WHEN_JOB_NOT_EXIST_SEC)
to always evaluate as 0 > 3 = False, spinning the loop indefinitely at 250ms intervals.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vdusek vdusek added bug Something isn't working. adhoc Ad-hoc unplanned task added during the sprint. t-tooling Issues with this label are in the ownership of the tooling team. labels Feb 17, 2026
@vdusek vdusek self-assigned this Feb 17, 2026
@github-actions github-actions bot added this to the 134th sprint - Tooling team milestone Feb 17, 2026
@codecov
Copy link

codecov bot commented Feb 17, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.02%. Comparing base (e7ee8b3) to head (97a93ec).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #619   +/-   ##
=======================================
  Coverage   76.02%   76.02%           
=======================================
  Files          42       42           
  Lines        2482     2482           
=======================================
  Hits         1887     1887           
  Misses        595      595           
Flag Coverage Δ
integration 68.77% <50.00%> (ø)
unit 64.66% <100.00%> (ø)

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 February 17, 2026 14:35
Copy link
Contributor

@Pijukatel Pijukatel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer a fix based on this comment in your other PR, but this works as well:

@vdusek
Copy link
Contributor Author

vdusek commented Feb 17, 2026

I would prefer a fix based on #604 (comment) in your other PR, but this works as well:

For these last 2.x releases, let's go with this, in the 3.x I will check out your suggestions

@vdusek vdusek merged commit 00e8621 into master Feb 17, 2026
30 of 31 checks passed
@vdusek vdusek deleted the fix/wait-for-finish-infinite-loop branch February 17, 2026 15:31
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. bug Something isn't working. 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

Comments