You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Follow-up to #2318 (now merged), addressing @TooTallNate's review note on it.
#2318's forward-port made object-stream reconnect retry on reader.read() errors, but a transient failure of the reopen itself — connect() throwing inside reconnect() because the server is briefly unavailable during the reconnect window — was fatal. That's exactly the blip the wrapper exists to survive.
This makes the reopen retry against the reconnect budget instead of erroring the stream: each reopen attempt counts against the consecutive-failure cap (and the absolute backstop), so a server that stays down still terminates the stream, but a momentary blip no longer kills it. The cancel/reset of buffered state moves ahead of the retry loop so every attempt resumes from the same frame index.
Test
New case in reconnecting-framed-stream.test.ts: a reopen that throws once then succeeds recovers transparently (the failed reopen never surfaces to the consumer; resume index is unchanged). All 12 reconnect tests pass.
Notes
Scoped to @workflow/core; user-facing behavior is unchanged (object streams reconnect) — this hardens the path against the blip it targets.
Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.
This PR includes no changesets
When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types
VaguelySerious
changed the title
[core] Forward-port stream reconnect to getReadable level (budgeted reopen)[core] Retry stream reopen against the reconnect budgetJun 11, 2026
The reason will be displayed to describe this comment to others. Learn more.
[P1] Resolve failed stream correctness benchmark before merging
The PR’s Benchmark Vercel (nextjs-turbopack) check failed in packages/core/e2e/bench.bench.ts with Stream correctness failure: expected 1048576 bytes but got 917504. Because this PR changes stream reconnect behavior, a stream-truncation correctness failure on the Vercel benchmark is a blocker until the job reruns green or is clearly triaged as unrelated/flaky.
The reason will be displayed to describe this comment to others. Learn more.
(AI) Triaged as unrelated to this PR's change, and re-run triggered.
Root cause: the failure is a silent truncation — totalBytes came up 128 KiB short (917504 vs 1048576) while the stream was treated as cleanly done. That only happens on the clean-EOF path (result.done → controller.close()). This PR touches exactly one function, reconnect(), which is reached only on a read error, never on clean EOF — it makes the reopen more resilient (retry-against-budget) and cannot cause a stream to end early. So this change is not on the code path that produced the failure.
What it actually is: the pre-existing max-duration truncation on the live read — the server hits its 2-min stream cap, closes the body cleanly, and the reader sees EOF mid-stream. It surfaced on the slower nextjs-turbopack Vercel lane (the express and nitro-v3 Vercel lanes, and all Local/Postgres lanes, passed in the same run) because that lane's 1 MiB pipeline crosses the 2-min window. This is precisely the bug the sibling PRs fix — world-vercel reading from the v3 endpoint (#2424) against the server erroring the body on timeout so the reader reconnects.
I've re-run the failed benchmark jobs; will confirm green here. "Performance Benchmarks" is a non-required workflow regardless.
The reason will be displayed to describe this comment to others. Learn more.
(AI) Confirmed green: after rebasing onto current main (the failing run was 11 days / 91 commits stale), Benchmark Vercel (nextjs-turbopack) now passes, along with Unit Tests (ubuntu + windows) and Build Packages. As expected this was the timing-surfaced v2 max-duration truncation rather than anything in this PR's reopen-retry change.
Addresses review feedback on #2318: a transient failure of the reopen
itself (`connect()` throwing inside `reconnect()`) was fatal — only
`reader.read()` errors were retried. A brief server unavailability during
the reconnect window is exactly the blip this wrapper exists to survive,
so count it against the reconnect budget and retry rather than erroring
the stream. Budget exhaustion (a server that stays down) still terminates.
Adds a test: a reopen that throws once then succeeds recovers transparently.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…testing
* origin/main:
Trace /flow route initialization (#2592)
Version Packages (beta) (#2591)
feat(web): add trace step shortcut helper (#2582)
[web-shared] reskin json viewer (no duplicates, better colours and navigation) (#2434)
Send occurredAt with workflow events (#2580)
docs: use actual eve logo and tidy OSS nav dropdown (#2586)
Display occurredAt in trace details (#2581)
fix(next): discover root entrypoints (#2564)
[core] Turbo: skip the unused run_started event-log preload (#2569)
fix(next): prewarm SWC plugin cache (#2538)
[world-vercel] Use v3 stream endpoint (supports transparent reconnect on timeout) (#2424)
[core] Retry stream reopen against the reconnect budget (#2334)
Add Platformatic World to worlds-manifest.json (#1450)
docs: derive section landing-page cards from the page tree + lint drift (#2567)
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
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.
Follow-up to #2318 (now merged), addressing @TooTallNate's review note on it.
#2318's forward-port made object-stream reconnect retry on
reader.read()errors, but a transient failure of the reopen itself —connect()throwing insidereconnect()because the server is briefly unavailable during the reconnect window — was fatal. That's exactly the blip the wrapper exists to survive.This makes the reopen retry against the reconnect budget instead of erroring the stream: each reopen attempt counts against the consecutive-failure cap (and the absolute backstop), so a server that stays down still terminates the stream, but a momentary blip no longer kills it. The cancel/reset of buffered state moves ahead of the retry loop so every attempt resumes from the same frame index.
Test
New case in
reconnecting-framed-stream.test.ts: a reopen that throws once then succeeds recovers transparently (the failed reopen never surfaces to the consumer; resume index is unchanged). All 12 reconnect tests pass.Notes
@workflow/core; user-facing behavior is unchanged (object streams reconnect) — this hardens the path against the blip it targets.🤖 Generated with Claude Code