Add support for readable streams in workflows local dev#13145
Add support for readable streams in workflows local dev#13145Caio-Nogueira merged 1 commit intomainfrom
Conversation
🦋 Changeset detectedLatest commit: 1dd568d The changes in this PR will be included in the next version bump. 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 |
|
Codeowners approval required for this PR:
Show detailed file reviewers |
There was a problem hiding this comment.
I'm Bonk, and I've done a quick review of your PR.
This PR adds ReadableStream<Uint8Array> support to the local dev workflows engine by chunking streams into SQL storage, replaying them on cache hit, and handling various error/timeout/rollback scenarios.
Issues
-
streamOutputRolledBackonWorkflowTimeoutErroris never set (low severity) — TheStreamWriteTimeoutErrortype definesstreamOutputRolledBack?: booleanandcontext.ts:536-541reads it from the thrown error, but no code path ever sets this property totrueon any error object. The read always evaluates toundefined(falsy), so the guard at line 704 is effectively dead. This is harmless sincerollbackStreamOutputis idempotent, but the plumbing is misleading. If this is forward-looking (for production parity), a comment would help. Otherwise it can be removed. -
Missing changeset (low severity) — The changeset bot flagged this. Since this changes user-visible behavior (stream outputs in local dev), a changeset for
workflows-sharedis likely needed unless theno-changeset-requiredlabel applies.
|
Review posted on PR #13145. The PR is well-structured overall -- the stream chunking/replay infrastructure in I flagged two low-severity items:
|
create-cloudflare
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-editor-shared
wrangler
commit: |
9820806 to
6470c27
Compare
Changeset ReviewFile:
|
6470c27 to
8e4a68d
Compare
79d8cd7 to
90665df
Compare
90665df to
a55b2b5
Compare
d106ced to
132fe45
Compare
132fe45 to
ba9269e
Compare
ba9269e to
864c356
Compare
864c356 to
28d6ed5
Compare
penalosa
left a comment
There was a problem hiding this comment.
Approving from a Wrangler perspective. I haven't closely reviewed the workflows-shared code
28d6ed5 to
1dd568d
Compare
|
Codeowners approval required for this PR:
Show detailed file reviewers |
workers-devprod
left a comment
There was a problem hiding this comment.
Codeowners reviews satisfied
We recently added workflows support for
ReadableStream<Uint8Array>. We want to mirror behavior on local dev as wellA picture of a cute animal (not mandatory, but encouraged)