Fix 429 retry#1051
Merged
Merged
Conversation
Signed-off-by: Peter Wielander <mittgfu@gmail.com>
🦋 Changeset detectedLatest commit: f8fc681 The changes in this PR will be included in the next version bump. This PR includes changesets to release 14 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 |
Contributor
Contributor
🧪 E2E Test Results
⏳ Tests are running... Started at: 2026-02-14T00:26:38Z ❌ Some tests failed Summary
❌ Failed Tests🌍 Community Worlds (42 failed)turso (42 failed):
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
❌ 🌍 Community Worlds
✅ 📋 Other
|
Contributor
📊 Benchmark Results
workflow with no steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) workflow with 1 step💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) | Nitro workflow with 10 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) workflow with 25 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) workflow with 50 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) Promise.all with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) | Nitro Promise.all with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) | Express Promise.all with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) | Express Promise.race with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) Promise.race with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) Promise.race with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Express | Nitro Stream Benchmarks (includes TTFB metrics)workflow with stream💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) | Nitro SummaryFastest Framework by WorldWinner determined by most benchmark wins
Fastest World by FrameworkWinner determined by most benchmark wins
Column Definitions
Worlds:
|
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
pranaygp
requested changes
Feb 13, 2026
Contributor
pranaygp
left a comment
pranaygp
left a comment
There was a problem hiding this comment.
This PR has the right intent but the fix has two structural bugs — the 429 path is still unreachable, and non-5xx errors are now silently swallowed. See inline comments.
| WorkflowAPIError.is(err) && | ||
| err.status !== undefined && | ||
| err.status >= 500 | ||
| ) { |
Contributor
There was a problem hiding this comment.
Bug: Non-5xx errors are now silently swallowed. The old code used a generic else block, so all non-suspension errors (user code errors, 4xx, network errors, etc.) would fall through to the run_failed handling below. By changing this to else if (status >= 500), the run_failed code (lines 321-372) is now inside this block and only executes for 5xx errors. Any other error type silently falls through the catch without recording run_failed.
Suggested structure:
} else if (WorkflowAPIError.is(err) && err.status === 429) {
// Re-throw to let withThrottleRetry handle it
throw err;
} else if (
WorkflowAPIError.is(err) &&
err.status !== undefined &&
err.status >= 500
) {
const retryCount = serverErrorRetryCount ?? 0;
const delaySecondSteps = [5, 30, 120];
if (retryCount < delaySecondSteps.length) {
// re-enqueue with backoff
...
return;
}
// Fall through to run_failed after exhausting retries
} else {
// Intentional fall-through for all other errors
}
// run_failed handling here (outside all branches)This way:
- 429 is handled first and re-thrown to
withThrottleRetry - 5xx gets exponential backoff retry, falling through to
run_failedwhen exhausted - All other errors (user code, network, etc.) still reach
run_failed
pranaygp
reviewed
Feb 13, 2026
Contributor
pranaygp
left a comment
pranaygp
left a comment
There was a problem hiding this comment.
One more note on testing:
pranaygp
reviewed
Feb 14, 2026
Contributor
pranaygp
left a comment
pranaygp
left a comment
There was a problem hiding this comment.
The latest revision (f8fc681) looks correct. The 429 else if is now a proper sibling to the 5xx check inside the generic else block, so:
- 429 errors correctly re-throw to
withThrottleRetry✓ - 5xx retry + fallthrough to
run_failedis preserved ✓ - All other errors (user code, network, etc.) still reach
run_failed✓
The structural bugs from the first commit (dead 429 branch, swallowed non-5xx errors) are resolved.
Suggestion: Consider adding a 429 fault injection test (similar to the existing 5xx installServerErrorFaultInjection pattern) to cover this integration path. That would prevent regressions like the original dead-code bug from shipping undetected.
pranaygp
approved these changes
Feb 14, 2026
Contributor
pranaygp
left a comment
pranaygp
left a comment
There was a problem hiding this comment.
The latest revision correctly fixes the 429 retry path. LGTM.
One suggestion for follow-up: consider adding a 429 fault injection e2e test (similar to the existing 5xx pattern) to prevent regressions.
This was referenced Feb 13, 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
2 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.
No description provided.