Repository navigation
Conversation
After the last retry, performCheck resolved with a final predicate call but did not return, so it kept polling every sleepTime forever. When the predicate is a network call (deleteQueue confirmation), the leftover loop outlives the caller and its rejections surface as unhandled once the endpoint goes away. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthrough
ChangesRetry handling
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to No actionable issue remains identified with stopping polling at the retry limit; the change is mergeable after normal checks. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Once
maxRetryCountis exceeded,waitAndRetryresolves with one lastpredicateFn()call but does notreturn.performChecktherefore keeps going and polls everysleepTimeforever, long after the caller has moved on.Impact
deleteQueue(..., waitForConfirmation)uses this to pollListQueuesuntil the queue is gone. If confirmation takes longer than the retry budget (15 × 20 ms),init()continues and recreates the queue. The leftover loop then never sees an empty list, so it keeps sendingListQueuesafterclose()and after the app shuts down.In ota-service's test suite, the queue is deleted on every
init()because ofdeleteIfExistsin tests. The tests run against an in-process fauxqs. When the suite stops fauxqs, the next poll fails withECONNREFUSED. That promise was passed to an already-settledresolve, so the rejection goes unhandled and Vitest fails an otherwise green run, intermittently.Change
returnafter the finalresolve(predicateFn()).test/utils/waitUtils.spec.ts, which covers a predicate that eventually succeeds, one that never does (exactlymaxRetryCount + 2calls, then none), and a final check that rejects.The two new exhaustion tests fail on
main(80 and 6 calls instead of 5) and pass with the fix. Thecoresuite passes: 24 files, 289 tests.biome checkandtscare clean. In ota-service, the same one-line patch took the spec from 8-12ListQueuescalls after teardown per run to 0 across 3 runs.🤖 Generated with Claude Code
Summary by CodeRabbit