~ knotless Search/
↑↓ move↵ open⇧↵ all results

chadtmiller.com/knotless

git that lives in your Atmosphere account

knotless.dev
1 collaborator

knotless 0.13.0 Latest 2 packages, 1 image

1
10

Pages are sent on as the app sends them, not once it is done

Merged
Merged chadtmiller.com asked to merge into main from chadtmiller.com/knotless:serve/stream-pages

What changes

Pages stream through knotless-serve. The pages proxy (crates/knotless-serve/src/web/pages.rs) used to read the SvelteKit app's whole answer (answer.bytes()) before sending any of it. It now sends each part on as the app sends it. A page's first byte no longer waits for its last, and what a page streams, as a directory's last commits (lastCommits in web/src/lib/data.ts), reaches the browser as soon as the app has it, as was meant.

Status, headers, the retry of a read, the outgoing log line and the request body limit are as before.

This branch starts from Pages go brotli, run their theme script inline, and have an icon, which puts brotli on the same file; its commit leaves this diff once that one lands and main is merged in.

Why

The usage logs had page loads whose own queries took ~170 ms spend over a second before their first byte, and the pages' streamed loads were never streamed at all past this proxy. Streaming here is the whole fix: the app already streams.

Compressed answers still stream: async-compression's encoder flushes what it has whenever its input is pending (generic/bufread/encoder.rs, State::Flushing), and the test below reads the first compressed part before the app sends the rest. Caddy's encode leaves an answer already encoded alone, and its reverse_proxy flushes at once a response with no length, as a streamed one is.

How it was checked

pages_drawn_elsewhere gains an app that sends a first part, then holds the rest until the test has read that part, plain and as brotli; before this change the request would wait for the rest and time out. Built and run by CI only.

Before, on prod, the repository home page's first byte came 240–310 ms after TLS from here (180 ms away), with the whole page done; the after number is for once it deploys.

Shortcomings

A failure of the app part way through a page now cuts the connection rather than answering 502, as the status has already gone.

🤖 Generated with Claude Code

Approved

This changes the pages proxy in pages.rs to send the app's body on part by part (Body::from_stream over chunk()) instead of waiting for answer.bytes(). I found nothing that must change.

  • Streaming code: the stream is correct. Status, headers, the retry and the body limit are untouched, and try_unfold ends on None. The reqwest error converts into the body's error type.
  • Compression: the brotli layer sits on top of the stream. Your note about the encoder flushing while its input is pending is plausible, and the br case of the new test checks it by reading the first part while the app holds the rest.
  • Test: the identity and br cases cover the change. Both depend on the runner, which I did not see run.
  • Failure mode: you already list the connection cut when the app fails part way through, in Shortcomings.
  • Diff scope: the diff also holds the brotli pull's commit and unrelated web changes: theme.js inlined, favicon, tabindex, h2. I took your word that the brotli commit leaves once that pull lands. I didn't review the web changes as part of this one.

Minor: the doc comment on handed now breaks mid-sentence (The app sees the host it was sits on its own short line). Reflow it.

Approved

The head is now a merge of main into the branch, and it changes nothing in the streaming code I reviewed. pages.rs still sends the app's body on part by part, and I found nothing that must change.

Last round I asked you to reflow the doc comment on handed. That isn't done: The app sees the host it was still sits on its own short line. It's cosmetic and doesn't block the merge.

I didn't re-review the merge's conflict resolutions in tests/web.rs and docs/design/serve.md, and I didn't see CI run.

crates/knotless-serve/src/web/pages.rsline 59Outdated
}
/// `req`, as the app at `web` answers it, its body sent on as it comes.
/// The app sees the host it was
review.knotless.dev

This doc comment still breaks mid-sentence after The app sees the host it was. Reflow it.

Merged as 17e7b1f