Send TUS uploads as bounded 32 MiB chunks - #108
Merged
Conversation
Re-lands the chunked PATCH loop (previously reverted in da1f0f6) with the chunk size retuned from 10 MiB to 32 MiB. Behind an edge that buffers the full request body before proxying, an interrupted single PATCH commits nothing: the server never sees a byte until the body completes, so every drop resumes from offset 0 and large uploads on flaky networks can loop forever. Bounded chunks are the standard TUS answer (tus-js-client exposes chunkSize for exactly this): each completed chunk is durably on the server, so a drop loses at most one chunk. The original 10 MiB default measurably hurt throughput, which is why it was reverted. Measured against the production edge (64 MB file): single PATCH to EOF 20.4 MB/s 32 MB chunks, kept-alive conn 20.5 MB/s 16 MB chunks, kept-alive conn 16.3 MB/s 8 MB chunks, kept-alive conn 10.1 MB/s 8 MB chunks, fresh TLS per chunk 5.1 MB/s Below ~32 MB the fixed per-chunk dead time (~0.4 s while the proxy spools each body) dominates; at 32 MB it is measurement noise, so this keeps single-PATCH throughput while making progress durable. Details: - Transfer loop sends chunkSizeBytes-bounded PATCHes and advances only on each 204's Upload-Offset — the server stays the sole source of truth and the happy path pays no per-chunk HEAD. Any failure or a 204 without forward progress hands control back to the retry wrapper, which re-HEADs for the authoritative offset before moving more bytes. - prepareChunkSource stages one bounded chunk via FileHandle seek+read, capping the per-resume staging copy at the chunk size — the previous code read the entire remainder into memory at once, unbounded by anything but file size. A file that fits in one chunk from offset 0 still uploads directly with no copy. - chunkSizeBytes stays configurable so deployments whose edge streams request bodies can raise it toward a single remainder. Fixes #89
There was a problem hiding this comment.
Pull request overview
Implements always-chunked TUS uploads (default 32 MiB) so uploads make durable progress when an edge proxy buffers request bodies, while preserving throughput via larger chunk sizes and connection reuse.
Changes:
- Add bounded-chunk upload loop driven by server
Upload-OffsetinuploadViaTus, with a configurablechunkSizeBytesdefaulting to 32 MiB. - Update the native uploader to stage exactly one chunk (bounded temp copy) and expose it as
uploadChunkNative. - Expand/adjust Jest coverage to validate chunk sequencing, offset discipline, default chunk size, and retry/re-HEAD resume behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/features/upload/tus-client.ts | Switch TUS transfer logic to bounded chunk PATCHes and add chunk-size configuration + offset parsing utilities. |
| src/features/upload/tus-client.test.ts | Update tests to assert chunk sequencing, default 32 MiB chunk size, and retry/re-HEAD offset discipline. |
| src/features/upload/transports/tus-server-transport.ts | Wire transport to the new uploadChunkNative injection point. |
| src/features/upload/native-chunk-upload.ts | Replace remainder-staging with per-chunk staging (prepareChunkSource) and export uploadChunkNative. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ress Review follow-ups: - Reject a non-positive/non-finite/fractional chunkSizeBytes up front with a non-retryable TusUploadError instead of entering a non-advancing upload loop that retries forever (covered by new tests). - Drop the in-flight per-chunk onProgress forwarding: behind a body-buffering proxy, bytes sent are not bytes committed, so progress now advances only on each 204's server-acknowledged Upload-Offset, matching the documented offset discipline. The unused per-chunk onProgress hook is removed from the UploadChunk contract and uploadChunkNative. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
morepriyam
added a commit
that referenced
this pull request
Jul 22, 2026
Send TUS uploads as bounded 32 MiB chunks
This was referenced Jul 22, 2026
morepriyam
added a commit
that referenced
this pull request
Jul 22, 2026
…ing as a knob Reverts the 32 MiB always-chunked default from #108 to the standard TUS shape: one PATCH from the current offset to EOF. In practice the chunk loop was what made big uploads crawl on device — a 32 MiB temp copy per chunk, a fresh background-URLSession task per chunk, the edge's spool dead time paid per chunk, and the JS loop freezing between chunks the moment the screen locks (progress then creeps at <=32 MiB per rare BG wake). A single PATCH is one native background transfer that streams straight from the source file and survives lock/backgrounding. chunkSizeBytes stays as an explicit opt-in escape hatch for deployments behind a body-buffering edge, with the 32 MiB benchmark numbers kept in its doc. Durability through the buffering proxy is deferred to the server-side fix (mieweb/opensource-server#395). Also: - restore smooth in-flight progress: native task ticks are forwarded as offset + bytesSentThisAttempt (clamped), re-anchored on each 204's server-acknowledged Upload-Offset, and honestly stepped back to the re-HEAD offset after a failure - resume staging is now memory-bounded: the remainder temp copy streams through 8 MiB FileHandle reads instead of one whole-remainder readBytes allocation, so remainder-sized PATCHes do not reintroduce the OOM half of #92
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
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.
Fixes #89 — successor to #87 (whose fix, #88, was reverted in da1f0f6;
maincurrently has neither the durability nor the behavior #87 was closed for).Problem
tus-client.tssends one PATCH from the current offset to EOF. The opensource-server edge buffers the entire request body beforeproxy_pass(mieweb/opensource-server#395), so the upstream never sees a byte until the body completes — an interrupted PATCH commits zero bytes. Confirmed against the production container: 31.5 MB sent →HEADreturnsUpload-Offset: 0. Large uploads on flaky networks can loop from byte 0 forever.Bounded chunks are the standard TUS client answer to body-buffering middleboxes — it's why
tus-js-clientexposeschunkSize("a good example of when to use it: if the server or proxy has a limit on the request body size"), and the tus resumable upload protocol makes every completed PATCH durable viaUpload-Offset— so a drop loses at most one chunk instead of everything.Why 32 MiB (and why the 10 MiB version was rightly reverted)
The reverted change defaulted to 10 MiB, which measurably hurt throughput. Benchmarked against the production edge (64 MB file):
main)Small chunks pay a fixed ~0.4 s per-chunk dead time (the proxy holds each body until complete) plus TLS setup when connections aren't reused. At 32 MiB the overhead is measurement noise — durability without giving up single-PATCH throughput.
chunkSizeBytesstays configurable so it can be raised toward a single remainder once the edge streams bodies (mieweb/opensource-server#395'smodsecurity offfix).Design
Upload-Offset(no per-chunk HEAD on the happy path). Any failure — or a 204 without forward progress — falls back to the retry wrapper, whose next attempt re-HEADs for the authoritative offset before sending more bytes, per the protocol's offset semantics.prepareChunkSourcestages exactly one chunk viaFileHandleseek + bounded read, capping the resume-copy memory spike at 32 MiB — the current code reads the entire remainder into memory on every resume, unbounded by anything but the file size. A file that fits in one chunk from offset 0 still uploads directly with no copy.expo-file-system's native upload task (File.upload,BINARY_CONTENT), since React Native'sfetch/Blobcannot carry raw byte bodies. Progress reporting only advances on server-acknowledged offsets, so displayed progress is always durable progress.Tests
tus-client.test.tscovers: sequential bounded chunk sequencing with correctUpload-Offsetheaders, the 32 MiB default, advance-on-204 without per-chunk HEADs, a 204 lacking a usableUpload-Offsettreated as transient (re-HEAD, continue from authoritative offset), mid-transfer failure resuming from the server's offset, and the existing create/resume/redirect/origin-pinning suite. All logic is dependency-injected and runs under the pure-logic jest config.Verification
npx tsc --noEmitcleannpx eslintcleannpx jest— 10 suites, 78 tests pass