feat: support streaming file uploads#1970
Conversation
Resolves #418
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d7d60a5d5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
jbeckwith-oai
left a comment
There was a problem hiding this comment.
This is awesome, thank you!
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Comprehensive review completed against 85872a1d.
I traced this back to #418 and #1052 and reproduced the root cause locally with a 64 MiB Node stream: toFile() consumed all 64 chunks before returning and retained 64 MiB of ArrayBuffer storage (about +123 MiB RSS in that run). The new toStreamingFile() wrapper consumed zero chunks before request transmission and added no measurable ArrayBuffer/RSS usage before send.
I also exercised the change through a real local HTTP server rather than only materializing the generated body. For a four-chunk, 4 MiB async source delayed by 120 ms/chunk, the server received its first bytes at 13 ms while the producer finished at 499 ms; the complete request contained the expected filename, MIME type, scalar field, and 4,194,524-byte multipart envelope. This validates that the SDK/fetch path preserves bounded, incremental delivery.
Validation performed:
pnpm exec jest tests/form.test.ts tests/uploads.test.ts tests/index.test.ts --runInBand: 79/79 tests passed, including streaming multipart, Blob chunk, and no-retry coverage.pnpm exec tsc --noEmit: passed.- Prettier, ESLint, package build, generated CJS/ESM import checks, type-package validation, and publint all passed locally. The final JSR dry-run step could not write to the sandboxed Deno cache, an environment restriction after the code/package checks had already succeeded.
- Existing CI is green across Node 20/22/24/26, build, ecosystem, breaking-change, Agents SDK, CodeQL, and 237/237 OkTest cases.
Compatibility review:
- Existing non-streaming
File/Blob/Responseuploads retain the nativeFormDatapath, andtoFile()is unchanged. - Node streams, web
ReadableStreams, async iterables, nested/array form fields, filenames, MIME types, and plainBlobchunks are handled by the lazy encoder. - Streaming bodies are correctly marked one-shot and automatic retries (including workload-identity replay) are suppressed rather than reusing a disturbed body. This intentionally means existing direct
fs.ReadStreamuploads no longer get buffered/replayable retry behavior; that is the necessary reliability tradeoff for transparent streaming and the explicit helper makes the behavior clear for new callers. - Cancellation/backpressure use the existing stream shims, and Node fetch receives
duplex: "half".
I found no blocking correctness, API, type, runtime, or regression issue.
Summary
toStreamingFile()helper for web, Node, and cloud-storage streamstoFile()behavior unchanged for callers that need a real webFileRoot cause
toFile()creates a webFile, and theFileconstructor has to consume stream contents up front. The existing multipart path also converted async streams into aBlobbefore sending, so large uploads were fully buffered in memory.Impact
Callers can now provide a filename and optional MIME type without buffering the source stream:
The SDK emits a bounded, lazy
ReadableStreammultipart body and preserves the existingFormDatapath for non-streaming uploads.Validation
pnpm run formatpnpm exec tsc --noEmitpnpm exec jest tests/form.test.ts tests/uploads.test.ts --runInBandResolves #418
Resolves #1052