fix(cmdutil): cap poll sleep at the deadline so --timeout stops on time - #47
Merged
Conversation
Poll always slept the full interval between checks and only tested the deadline after honoring done, so --timeout could overshoot by a full interval (plus the in-flight request): a 5s interval with --timeout 1 resolved at t≈5s, as success or as exit 5, never at ~1s. The sleep is now min(interval, time-until-deadline), so the final check lands on the deadline and no new check starts past it. A check that starts on time and succeeds still returns its result — the operation genuinely completed; --timeout bounds when polling stops, not total wall-clock duration. Overshoot is now bounded by one in-flight request instead of one interval. AGENTS.md's interim description of the loose behavior is updated to match. Fixes Bandwidth#35.
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
kshahbw
approved these changes
Aug 25, 2026
joshraub-bw
approved these changes
Aug 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #35.
What was wrong
Pollininternal/cmdutil/poll.goalways slept the full--intervalbetween checks and only tested the deadline after honoringdone. So--timeoutcould overshoot by a full interval plus the in-flight request: with a 5s interval and--timeout 1, the command resolved at t≈5s — as exit 0 if the second poll succeeded, or exit 5 if it didn't — never at ~1s. This affected every--waitcommand.The fix
The sleep is now
min(interval, time-until-deadline), so the final wait lands on the deadline: one last check runs there, and no new check starts past it. Overshoot is now bounded by one in-flight request instead of one interval.On the open question from the issue — success on a check that started on time but lands at/after the deadline — this keeps exit 0 with the result, as the issue argues: the operation genuinely completed, and discarding that is worse than being slightly late.
--timeoutbounds when polling stops, not total wall-clock duration; the doc comment onPolland the AGENTS.md section now say exactly that (the old AGENTS.md text described the loose behavior and is replaced with the tightened bound).Tests
Two regression tests, written failing-first against the old loop (both failed at ~250ms, now pass at ~20ms):
TestPollTimeoutDoesNotOvershootByFullInterval— 250ms interval, 20ms timeout must reportErrPollTimeout(exit 5) at ~20ms, not ~250ms.TestPollFinalCheckAtDeadlineCanSucceed— a check that succeeds at the deadline returns the result at ~deadline, not a full interval later.Verified:
go build,go vet, full suite,gofmtclean; poll tests race-clean over-count=5.