fix: respect maxElapsed in ExponentialBackoff.execute#3760
Conversation
🦋 Changeset detectedLatest commit: ddd000b The changes in this PR will be included in the next version bump. This PR includes changesets to release 32 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughThis PR modifies Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes ✨ 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 |
|
Hi @nikhil-shukl, thanks for your interest in contributing! This project requires that pull request authors are vouched, and you are not in the list of vouched users. This PR will be closed automatically. See https://github.com/triggerdotdev/trigger.dev/blob/main/CONTRIBUTING.md for more details. |
There was a problem hiding this comment.
🟡 maxElapsed wall-clock check is bypassed when callback throws AttemptTimeout
The new maxElapsed check at line 343 is placed after the try/catch/finally block. When the callback throws an AttemptTimeout (line 335-337), the continue statement skips all code after the try/catch/finally, including the new maxElapsed check. Although the finally block still executes and correctly updates elapsedMs, the actual check is never evaluated.
This means if a caller uses both attemptTimeoutMs and maxElapsed, and every attempt times out, the loop will never exit early due to maxElapsed. It will only stop when retryAsync's delay-based check (which doesn't account for callback execution time, packages/core/src/v3/apps/backoff.ts:127-130) or maxRetries terminates the generator. For example, with maxElapsed: 10 (seconds), attemptTimeoutMs: 3000, and maxRetries: 100, the loop could run for up to ~300 seconds instead of stopping after 10.
(Refers to lines 335-337)
Was this helpful? React with 👍 or 👎 to provide feedback.
Closes #3726
✅ Checklist
Testing
Ran the focused backoff test: