Repository navigation
fix: stop polling sleeps at the remaining deadline - #283
Conversation
NianJiuZst
left a comment
NianJiuZst
left a comment
There was a problem hiding this comment.
Reviewed commit 78469b689b381ab890cf04a522f87d55e4540f43. No blocking findings; I recommend merging.
The change correctly caps the delay between polling attempts at the remaining deadline. It preserves the existing completed/failed response handling and does not change the timeout of an in-flight HTTP request.
I independently reproduced the regression: with a 50 ms polling timeout and a 1.5 s interval, unchanged main (8668aa5) took approximately 1,506 ms to reject, while this patch took approximately 52 ms. All 6 polling tests pass. Type checking and lint pass, with the existing unrelated warning in test/sdk/speech.test.ts.
The CI workflow for this exact head now passes, so the earlier CI hold is resolved.
2 participants
When a pending task is near its polling deadline,
poll()still sleeps for the entire configured interval. For example, a 50 ms timeout with a 1.5 s interval takes about 1.5 s to reportPolling timed out. The CLI's default video polling interval is 5 s, so short timeouts can substantially overrun while waiting between requests.Cap that sleep by the time remaining before the existing deadline. This is limited to the interval between requests: it does not change HTTP request timeouts or how completed and failed task responses are handled.
The regression fails on unchanged source (about 1504 ms) and passes with the one-line fix (about 51 ms). A separate real HTTP check against a local pending-task server took about 1543 ms on unmodified upstream and 54 ms with the patch, with one request in both cases.
Validation with Bun 1.4.0:
test/sdk/speech.test.ts.a965e72produces the same 3 failures (596 passed): two region-detection/auth tests exceed their existing 5 s test timeouts, and a FileSDK upload assertion sees a loopback base URL instead of the global endpoint. A second run with process-local loopback proxy bypass had 596 passed and 4 failed, adding an existing ImageSDK validation/network test timeout; that extra timeout is not diagnosed here. The full suite is not green. No tests were skipped or timeouts/assertions relaxed. OAuth tests used a temporary process-local no-opxdg-opento prevent launching a real browser; source and tests were otherwise unchanged.Codex assisted with investigation, implementation, testing and the automated reviews. These reviews are not independent human code review.