Phase 4: Stream binary pass-through responses via send_to_client()

[aram356]

Summary

Stream non-processable 2xx responses (images, fonts, video) directly to the client instead of buffering the entire body in memory. Uses send_to_client() with the unmodified body to preserve Content-Length and avoid chunked encoding overhead.

Closes #592, closes #593.
Part of epic #563. Depends on Phase 3 (#591).

Performance results (staging vs production, median over 8 runs, Chrome 1440x900)

Metric	Production (v135, buffered)	Staging (v141, streaming+passthrough)	Delta
TTFB	57 ms	28 ms	-29 ms (-51%)
First Paint	252 ms	182 ms	-70 ms (-28%)
First Contentful Paint	252 ms	182 ms	-70 ms (-28%)
DOM Content Loaded	322 ms	248 ms	-74 ms (-23%)
DOM Complete	678 ms	582 ms	-96 ms (-14%)

Measured on getpurpose.ai via Chrome DevTools Protocol with --host-resolver-rules to route to staging Fastly edge.

How it works

The streaming gate in handle_publisher_request now has three outcomes:

is_success (2xx, not 204/205)
├── should_process && (!is_html || !has_post_processors) → Stream (pipeline)
├── should_process && is_html && has_post_processors     → Buffered (post-processors)
└── !should_process                                      → PassThrough (send_to_client)

204 No Content, 205 Reset Content, or !is_success
└── any content type                                     → Buffered

PassThrough reattaches the unmodified body and sends via send_to_client(), preserving Content-Length. This avoids both WASM memory buffering and chunked encoding overhead.

What changed

File	What
publisher.rs	PassThrough variant, gate logic, 204/205 exclusion, debug logging, tests
main.rs	Handle PassThrough: reattach body, return for send_to_client()

Behavioral changes worth calling out

• Empty-host + non-processable 2xx now PassThrough instead of Buffered. Before this PR, requests with an empty request_host always took the buffered path. URL rewriting is skipped either way, so this is functionally equivalent — but observable as a different code path.
• Buffered path now sets Content-Length to the processed output length. Previously the buffered path called response.remove_header(CONTENT_LENGTH) after processing; this PR sets it to output.len() instead. Browsers that previously fell back to chunked encoding for processed HTML now get an explicit length.

Tests added

• pass_through_gate_streams_non_processable_2xx — images/fonts return PassThrough
• pass_through_gate_buffers_non_processable_error — 4xx stays Buffered
• pass_through_gate_does_not_apply_to_processable_content — HTML goes through Stream
• pass_through_gate_excludes_204_no_content — 204 stays Buffered (RFC 9110 §15.3.5)
• pass_through_gate_excludes_205_reset_content — 205 stays Buffered (RFC 9110 §15.3.6)
• pass_through_gate_applies_with_empty_request_host — empty host still passes through
• pass_through_preserves_body_and_content_length — byte-for-byte identity + CL preserved

Verification

• cargo test --workspace — all passing (819 unit + integration)
• cargo clippy --workspace --all-targets --all-features -- -D warnings — clean
• cargo fmt --all -- --check — clean
• cargo build --release --target wasm32-wasip1 — success

Test plan

• Pass-through gate unit tests pass (7 tests)
• Byte-level body + Content-Length preservation test passes
• 204 No Content and 205 Reset Content exclusion tests pass
• All existing streaming/buffered tests pass
• Full workspace tests pass
• WASM build succeeds
• Staging performance verified (51% TTFB, 28% FCP, 14% DOM Complete improvement)

Dismissing to resubmit with complete review body — inline comments and body were split across two API calls due to tooling issue.

[prk-Jr]

Summary

Phase 4 is mechanically correct: the PassThrough variant cleanly separates binary pass-through from streaming/buffered paths, Content-Length is preserved via send_to_client(), the 204 exclusion is properly handled, and the once_cell → LazyLock migration reduces the dependency surface. Two items need to be addressed before merge.

Blocking

🔧 wrench

• Stale PR title: The title says "via io::copy" but the final implementation uses response.set_body(body) + send_to_client(). Commit 3aa88fe replaced the io::copy approach specifically to preserve Content-Length. The title should read something like "Phase 4: Pass-through binary responses with Content-Length preservation via send_to_client".

Non-blocking

🤔 thinking

• Empty-host + non-processable 2xx behavioral change is undocumented: Before Phase 4, empty-host requests always went Buffered. Now non-processable 2xx responses with an empty host take the PassThrough path instead. Intentional and correct (URL rewriting is skipped either way), but not mentioned anywhere in the PR description. Worth a one-liner: "Non-processable 2xx with empty request host now PassThrough instead of Buffered — URL rewriting is skipped either way."

📝 note

• Content-Length fix in buffered path not mentioned: publisher.rs:449 — Phase 3 left the buffered path calling response.remove_header(CONTENT_LENGTH) after processing; this PR correctly sets it to the processed output length. The fix is right but it changes observable response headers and isn't mentioned in the description or commit messages.

👍 praise

• once_cell → std::sync::LazyLock migration (datadome.rs, google_tag_manager.rs, rsc.rs, shared.rs): Correct across all four sites, reduces the dependency surface, and all Regex::new expect() messages follow the "should ..." convention.
• 204 No Content exclusion: RFC 9110 §15.3.5 prohibits a message body on 204 — the guard placement before take_body() and the dedicated test both make this correct and visible.

🌱 seedling

• No end-to-end test for the PassThrough path: The gate-logic tests are good but none of them call handle_publisher_request() with a backend returning a binary content type. An integration-level test exercising the full path would catch regressions in the gate condition itself. Follow-up suggestion — does not block merge.

CI Status

• Integration tests: PASS
• fmt / clippy / cargo test: NOT RUN (CI gates only trigger on PRs targeting main; expected for phased branching strategy)

[ChristianPavilonis]

Review Summary

2 findings (1× P2, 1× P3) — both are suggestions for improvement, no blockers.

[prk-Jr]

Summary

Adds PublisherResponse::PassThrough for non-processable 2xx responses (images, fonts, video), streaming them directly to the client via send_to_client() instead of buffering in WASM memory. The implementation is correct: Content-Length is preserved, 204 No Content is properly excluded, and EC/consent headers are still applied. The 51% TTFB improvement is well-documented with real staging measurements.

Non-blocking

🤔 thinking

• PassThrough doc implies finalize_response() is the immediate caller's obligation — but in main.rs it's called by the outer route_request path, not the PassThrough arm itself. Current code is correct; the doc could mislead a future adapter implementor. (publisher.rs:277)

♻️ refactor

• Redundant get_status() call — is_success: bool is captured at line 455, then response.get_status() is called again at line 470 to produce status. The inner status.is_success() duplicates is_success. Hoist let status = response.get_status() before the outer if and replace let is_success = ... with let is_success = status.is_success(). (publisher.rs:470)

📝 note

• No debug log when returning PassThrough — the block logs "Skipping response processing..." at the top but has no log at the PassThrough return. A log::debug!("Pass-through: binary response, Content-Type: {}, status: {}") would help with production tracing. (publisher.rs:473)
• Gate tests verify boolean arithmetic, not handle_publisher_request() directly — consistent with the existing streaming_gate_* pattern and an accepted trade-off given the Fastly runtime can't be mocked. Noted for awareness. (publisher.rs:766)

⛏ nitpick

• 205 Reset Content not excluded — RFC 9110 §15.3.6 says 205 MUST NOT include message body content. Adding status != StatusCode::RESET_CONTENT alongside the 204 guard would be spec-complete, low risk. (publisher.rs:471)

👍 praise

• send_to_client() over stream_to_client() — correct call; stream_to_client() forces chunked encoding which would corrupt Content-Length for binary assets. (main.rs:247)
• 204 No Content excluded correctly — HTTP spec prohibits a body in 204; tested and documented. (publisher.rs:471)
• apply_ec_headers called before the PassThrough gate — EC/consent headers are correctly applied to pass-through responses, consistent with existing buffered behaviour.
• Staging performance results — 51% TTFB, 28% FCP improvement with methodology documented. Exemplary.

CI Status

• browser integration tests: PASS
• integration tests: PASS
• prepare integration artifacts: PASS

[prk-Jr]

⛏ nitpick — PR title references io::copy but the implementation uses send_to_client()

The title reads: Phase 4: Stream binary pass-through responses via io::copy

The first commit used io::copy + stream_to_client(), but that was superseded by commit ef576a19 ("Preserve Content-Length for PassThrough by using send_to_client"). The final implementation has no io::copy anywhere — send_to_client() is the right description.

Suggested title: Phase 4: Stream binary pass-through responses via send_to_client()

[aram356]

Addressed in 6ed2b5e:

• PR title — renamed to "Phase 4: Stream binary pass-through responses via send_to_client()" (no more io::copy reference).
• Redundant get_status() call (publisher.rs:454/470) — hoisted let status = response.get_status() once and derived is_success from it; the inner block now reuses both.
• PassThrough doc (publisher.rs:277) — reworded to make clear that finalize_response() and send_to_client() are the outer dispatcher's responsibility, not the per-arm caller's.
• 205 Reset Content (publisher.rs:471) — added status != StatusCode::RESET_CONTENT alongside the 204 guard with an RFC 9110 §15.3.6 reference; new pass_through_gate_excludes_205_reset_content test covers it.
• Debug log (publisher.rs:473) — added log::debug!("Pass-through binary response - Content-Type: '{}', status: {}", ...) before the PassThrough return.
• PR description — added a "Behavioral changes worth calling out" section covering (a) empty-host + non-processable 2xx now PassThrough, and (b) buffered path now sets Content-Length to processed length instead of removing it.

The end-to-end adapter integration test (🌱 seedling / P2 follow-up) is not in this PR — would need a Viceroy harness for binary content. Tracking as follow-up.

Verification: cargo test --workspace (819 passing), cargo clippy --workspace --all-targets --all-features -- -D warnings clean, cargo fmt --all -- --check clean, release wasm build succeeds.