Stream publisher origin bodies end-to-end on Fastly#867
Conversation
Publisher pages were fully buffered before the first byte reached the client: the platform client materialized the origin body (10 MiB cap), the rewrite pipeline ran over an in-memory cursor, and the EdgeZero finalize buffered the assembled response while awaiting auction collection. TTFB therefore tracked full origin transfer plus the auction instead of origin first byte. - Add supports_streaming_responses() to PlatformHttpClient (default false, Fastly true) and request with_stream_response() on the publisher origin fetch only where honored - Teach the pipeline to consume Body::Stream asynchronously: BodyChunkSource (cumulative raw-byte cap via publisher.max_buffered_body_bytes), push-style BodyStreamDecoder/BodyStreamEncoder in streaming_processor - Replace the Fastly buffered finalize with publisher_response_into_streaming_response: a lazy Body::Stream that commits headers at origin first byte, streams rewritten chunks, and holds only the </body> tail for auction collection; bids still inject before body close - Share one hold implementation (hold_step_decoded_chunk / hold_finish_segments) between the lazy body and the writer-driven loop so the paths cannot drift; collect_non_html_auction dedupes the collect-before-stream path - Finalize brotli decode with close() so truncated origin streams error instead of silently truncating; decode failures emit stream_decode_error telemetry - Guard bodiless (HEAD/204/304) responses and log wasted auction dispatch, matching the buffered finalizer Local A/B on a 183 KB gzip publisher page with a live 3-slot auction (release builds, 20 interleaved rounds): TTFB median 741 ms buffered vs 161 ms streamed (-78%); guest wall time and wasm heap unchanged.
Address the deep-review findings on the streaming cutover: - Cap cumulative decoded bytes in BodyStreamDecoder against publisher.max_buffered_body_bytes: the chunk source only bounds raw compressed bytes, so a decompression bomb could expand ~1000x past it and push unbounded decoded volume through the rewrite pipeline - Detect truncated deflate streams: write::ZlibDecoder::try_finish accepts truncated input silently, so the deflate arm now drives flate2::Decompress directly and requires Status::StreamEnd at finalization; trailing bytes after the end marker stay ignored. Add truncated-gzip and truncated-deflate regression tests - Make BodyChunkSource::next_chunk cancellation-safe by polling the body in place instead of moving it out across an await; a cancelled pull no longer turns into a silent EOF - Log dispatched auctions dropped uncollected (client disconnect mid-stream or never-polled body) via DispatchedAuctionGuard; the guard is created before the lazy stream so unpolled drops log too - Share the pull+decode step between the lazy publisher body and the write-sink drivers (hold_step_next_chunk / passthrough_step), removing the unreachable!() error plumbing and the triplicated processor selection; document body_close_hold_loop_stream as groundwork for the buffered adapters' streaming cutover - Pass identity-encoded chunks through zero-copy and finish encoders by consuming them instead of allocating a throwaway replacement - Add a Fastly dispatch test asserting the publisher fallback returns Body::Stream without a stale Content-Length, plus a comment on why the publisher fetch gates streaming on capability while the asset path does not Behavior note: gzip bodies with trailing garbage after the trailer now error mid-stream; the old read-path decoder ignored them.
The buffered finalizer abandons a dispatched auction with processor_init_error telemetry when HTML processor construction fails; the streaming finalizer dropped the in-flight SSP responses silently. Make publisher_response_into_streaming_response async and emit the same abandonment before returning the construction error.
ChristianPavilonis
left a comment
ChristianPavilonis
left a comment
There was a problem hiding this comment.
Automated review:
Review Summary
Reviewed PR #867 against main, focusing on the new Fastly end-to-end publisher streaming path, body/HTTP semantics, decoder limits, adapter contracts, and regression coverage. The change is well-tested and CI is green, but I found two correctness/resource-safety issues that should be addressed before relying on this path in production.
Findings
See the inline comments for the detailed findings.
CI / Existing Reviews
gh pr checks 867 reports all checks passing, including Rust/JS tests, formatting, CodeQL, browser integration, and cross-adapter parity. No existing PR reviews or inline comments were present when this automated review ran.
| services: RuntimeServices, | ||
| ) -> Result<Response<EdgeBody>, Report<TrustedServerError>> { | ||
| match publisher_response { | ||
| PublisherResponse::Buffered(response) => Ok(response), |
There was a problem hiding this comment.
Automated review: P1 — Buffered streaming responses can leak bodies on bodiless responses
Fastly now requests with_stream_response() before the origin response is classified. Any response routed to BufferedUnmodified can therefore still carry an EdgeBody::Stream, and this arm returns it unchanged. The Fastly entry point later streams any EdgeBody::Stream, so HEAD, 204, 205, and unprocessable 304 responses can send body bytes even though HTTP requires them to be bodiless/preserve metadata only.
Please normalize the Buffered arm for the streaming finalizer (and ideally the common buffered finalizer) by clearing the body when !response_carries_body(method, response.status()), while preserving headers such as Content-Length; also include StatusCode::RESET_CONTENT in response_carries_body. Add regression tests with streaming bodies for HEAD, 204, 205, and 304.
| .change_context(TrustedServerError::Proxy { | ||
| message: "Failed to decode gzip publisher body chunk".to_string(), | ||
| })?; | ||
| bytes::Bytes::from(std::mem::take(decoder.get_mut())) |
There was a problem hiding this comment.
Automated review: P1 — Decoded body cap is checked only after a compressed chunk fully expands
The streaming decoder writes a whole compressed chunk into the codec, drains the codec's internal Vec, and only then calls track_decoded(). A small compressed chunk can expand far beyond publisher.max_buffered_body_bytes before the cap is checked, so the configured limit is not a hard Wasm-heap ceiling. The current tests demonstrate this shape already: a sub-1 KiB gzip body expands to 64 KiB and is rejected only after that allocation has happened.
Please enforce the remaining decoded budget during decompression instead of after it, e.g. by decoding into a bounded sink/buffer that errors as soon as remaining + 1 bytes would be emitted, or by driving each codec with explicit output buffers sized to the remaining limit.
2 participants
Summary
Body::Streamso headers commit at origin first byte while the auction rides the transfer — only the</body>tail is held so bids still inject before body close.fastly compute serve, same seeded config, live origin, 183 KB gzip page, live 3-slot server-side auction, 20 interleaved rounds): TTFB median 741 ms buffered → 161 ms streamed (−78%, 4.6×); guest wall time (738 vs 744 ms) and wasm heap (4.3 vs 4.2 MB) unchanged — the win is purely overlapping delivery with transfer, and >10 MiB pages now stream instead of erroring. Buffered adapters (Axum/Cloudflare/Spin) keep the bounded buffered finalizer; their platform clients don't produce streams yet.Changes
crates/trusted-server-core/src/platform/http.rssupports_streaming_responses()toPlatformHttpClient(defaultfalse) so callers only request the streaming contract where the adapter honors itcrates/trusted-server-adapter-fastly/src/platform.rssupports_streaming_responses() = truecrates/trusted-server-core/src/publisher.rswith_stream_response()when supported;BodyChunkSource(async chunk pull + cumulative raw-byte cap);publisher_response_into_streaming_responsebuilds the lazy processedBody::Stream(auction hold inside the generator); sharedhold_step_decoded_chunk/hold_finish_segmentsused by both async hold paths;collect_non_html_auctiondedupes collect-before-stream; bodiless (HEAD/204/304) guard + wasted-dispatch warning;body_as_readernow errors on stream bodies instead of silently emptying themcrates/trusted-server-core/src/streaming_processor.rsBodyStreamDecoder/BodyStreamEncoder(write-based flate2/brotli codecs; brotli finalize usesclose()so truncated input errors instead of silently truncating); sharedSTREAM_CHUNK_SIZEcrates/trusted-server-adapter-fastly/src/app.rsbuffer_publisher_response_asynccrates/trusted-server-adapter-fastly/src/main.rssend_edgezero_responsedoc: streaming now covers publisher bodies, not just assetscrates/trusted-server-core/src/proxy.rsstream_asset_bodydocs generalized — it now bridges publisher streams toocrates/trusted-server-core/src/platform/test_support.rscrates/trusted-server-core/src/settings.rsmax_buffered_body_bytesdoc: also the cumulative raw-byte cap on the streaming path; cap trip after headers truncates rather than 5xxCargo.toml,crates/trusted-server-core/Cargo.toml,Cargo.lockasync-stream(already in the tree via edgezero-core) for the lazy body generatordocs/superpowers/plans/2026-07-08-true-origin-streaming-fastly.mdCloses
Closes #849
Test plan
cargo test-fastly && cargo test-axum(core: 1642 passed; pluscargo test-cloudflare && cargo test-spin)cargo clippy-fastly && cargo clippy-axum(plusclippy-cloudflare,clippy-cloudflare-wasm,clippy-spin-native,clippy-spin-wasm, all on 1.95.0)cargo fmt --all -- --checkcd crates/trusted-server-js/lib && npx vitest runcd crates/trusted-server-js/lib && npm run formatcd docs && npm run formatcargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1fastly compute serve: side-by-side A/B vsmain(20 interleaved rounds, live origin + live 3-slot auction) — TTFB 741 → 161 ms median,transfer-encoding: chunkedwith gzip preserved, bids injected before</body>on both builds, byte-parity on body size</body>survives the auction hold; truncated brotli stream errors; cumulative cap enforcement; Stream-vs-Once parity across gzip/deflate/brotli/identity; stream-flag gating on/offChecklist
unwrap()in production code — useexpect("should ...")tracingmacros (notprintln!)