Implement Streaming Static/Proxy Rendering #4329

jridgewell · 2023-03-24T04:18:47Z

Description

Implements streaming rendering for static/proxy renders (I don't know why they're called static or proxy…) using an initial Headers message, N BodyChunk messages, and a final BodyEnd message.

Updating proxy rendering was very easy. It already implemented separate messages for header and body, but it buffered the full body before sending the full contents. But static rendering was a little more difficult, it has multiple paths that are immediately ready with a full body that I didn't want to break apart. So I kept the Response message for a full response that transforms into a ContentSourceContent::Static, and implemented a new enum type for streaming responses into a ContentSourceContent::HttpProxy.

These is some weirdness with different data types on the headers field of a ContentSourceContent::Static's StaticContent and a ContentSourceContent::HttpProxy's ProxyResultVc. One is a HeaderListVc and the other is a Vec<(String, String)> (and really should be a HeadersList), but I'm leaving this for a follow up.

Testing Instructions

Paired Next.js PR: vercel/next.js#47476

Re: WEB-27

vercel · 2023-03-24T04:19:03Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated
examples-native-web	🔄 Building (Inspect)			Mar 29, 2023 at 4:54PM (UTC)
examples-tailwind-web	🔄 Building (Inspect)			Mar 29, 2023 at 4:54PM (UTC)

9 Ignored Deployments

Name	Status	Preview	Updated
examples-basic-web	⬜️ Ignored (Inspect)		Mar 29, 2023 at 4:54PM (UTC)
examples-cra-web	⬜️ Ignored (Inspect)		Mar 29, 2023 at 4:54PM (UTC)
examples-designsystem-docs	⬜️ Ignored (Inspect)		Mar 29, 2023 at 4:54PM (UTC)
examples-gatsby-web	⬜️ Ignored (Inspect)		Mar 29, 2023 at 4:54PM (UTC)
examples-kitchensink-blog	⬜️ Ignored (Inspect)		Mar 29, 2023 at 4:54PM (UTC)
examples-nonmonorepo	⬜️ Ignored (Inspect)		Mar 29, 2023 at 4:54PM (UTC)
examples-svelte-web	⬜️ Ignored (Inspect)		Mar 29, 2023 at 4:54PM (UTC)
examples-vite-web	⬜️ Ignored (Inspect)		Mar 29, 2023 at 4:54PM (UTC)
turbo-site	⬜️ Ignored (Inspect)	Visit Preview	Mar 29, 2023 at 4:54PM (UTC)

github-actions · 2023-03-24T04:36:17Z

🟢 CI successful 🟢

Thanks

crates/turbopack-node/src/render/mod.rs

github-actions · 2023-03-29T03:38:47Z

✅ This changes can build next-swc

jridgewell

Ok, this is now fully updated. I really dislike the amount of duplication that this requires for anything that streams. Because of the different response types (proxy always streams a response, but static can return a rewrite, full response, or stream response) I can't share code between the two.

Streaming itself is cumbersome. The incoming stream needs defined types, and the outgoing stream needs a new set of defined types with "prepared" values. This all happens in a pulling function, which is 2 steps removed from the actual place I want to use the stream in the main function. In the main function, I can only read the prepared values and return them. This might be simpler if we can macro over the boilerplate.

Oh, and the generator! {} block doesn't format, so that's ugly.

sokra

Look good in general. Reviewed from mobile. Looking forward to see this in action

crates/turbopack-node/src/render/render_proxy.rs

sokra · 2023-03-29T04:34:05Z

crates/turbopack-dev-server/src/source/mod.rs

@@ -246,29 +266,29 @@ pub struct ContentSourceData {
    pub cache_buster: u64,
 }

-type Chunk = Result<Bytes, BodyError>;
+pub type BodyChunk = Result<Bytes, BodyError>;


Suggested change

pub type BodyChunk = Result<Bytes, BodyError>;

pub type BodyChunk = Result<Bytes, SharedError>;

BodyError coverts everything to a string loosing all the structure. We want to avoid that.

This requires a slightly larger change, and it's complicated by ShareError not being serializable. That would prevent any body response from being cachable, which I think is bad for our end goal? Anyways, I'd like to defer this to a follow up PR for the time being.

That would prevent any body response from being cachable, which I think is bad for our end goal?

It would still be cacheable in memory.

While I like to have static rendering to be cacheable, next.js doesn't cache rendering at all in dev and always server-renders a fresh page.

sokra · 2023-03-29T04:39:45Z

crates/turbopack-node/src/render/render_proxy.rs

+        Ok(v) => Err(BodyError::new(format!("unexpected render item: {:#?}", v))),
+        Err(e) => Err(BodyError::new(format!(
+            "error streaming proxied contents: {}",
+            e


Use .context() instead of format!
We want to avoid converting to string

I agree, but I'm deferring to a follow up PR.

sokra · 2023-03-29T04:44:07Z

crates/turbopack-node/src/render/render_proxy.rs

+
+    // We create a new cell in this task, which will be updated from the
+    // [render_stream_internal] task.
+    let cell = turbo_tasks::macro_helpers::find_cell_by_type(*RENDERSTREAM_VALUE_TYPE_ID);


We probably should move these hacks into turbo-tasks-bytes

I actually think we should move Stream<T> into turbo-tasks proper, and design a macro interface as you suggested.

crates/turbopack-node/src/render/render_static.rs

sokra

Approving that, but I need some cleanup in follow-up PRs. Also make sure to document public methods

Paired with vercel/turborepo#4329, this implements streaming responses for App and API renders. This is accomplished by sending an initial `headers` message (carrying the status code and headers list), N `body-chunk` messages of bytes, and a final `body-end` message to signal completion. Once sent to Turbopack, these chunk messages will be streamed out of the node rendering process directly into the HTTP server's response. Closes WEB-27 --------- Co-authored-by: Tobias Koppers <tobias.koppers@googlemail.com> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

Follow up to #4329, this removes `BodyError` (which was just a simple `String` and not an `Error`) and switches to `SharedError`. I've implemented a basic `PartialEq` and `Serialization` for `SharedError` so that it doesn't infect everything that uses `Body`.

### Description Implements streaming rendering for static/proxy renders (I don't know why they're called static or proxy…) using an initial `Headers` message, N `BodyChunk` messages, and a final `BodyEnd` message. Updating proxy rendering was very easy. It already implemented separate messages for header and body, but it buffered the full body before sending the full contents. But static rendering was a little more difficult, it has multiple paths that are immediately ready with a full body that I didn't want to break apart. So I kept the `Response` message for a full response that transforms into a `ContentSourceContent::Static`, and implemented a new enum type for streaming responses into a `ContentSourceContent::HttpProxy`. These is some weirdness with different data types on the `headers` field of a `ContentSourceContent::Static`'s `StaticContent` and a `ContentSourceContent::HttpProxy`'s `ProxyResultVc`. One is a `HeaderListVc` and the other is a `Vec<(String, String)>` (and really should be a `HeadersList`), but I'm leaving this for a follow up. ### Testing Instructions Paired Next.js PR: #47476 Re: WEB-27

Follow up to vercel/turborepo#4329, this removes `BodyError` (which was just a simple `String` and not an `Error`) and switches to `SharedError`. I've implemented a basic `PartialEq` and `Serialization` for `SharedError` so that it doesn't infect everything that uses `Body`.

### Description Implements streaming rendering for static/proxy renders (I don't know why they're called static or proxy…) using an initial `Headers` message, N `BodyChunk` messages, and a final `BodyEnd` message. Updating proxy rendering was very easy. It already implemented separate messages for header and body, but it buffered the full body before sending the full contents. But static rendering was a little more difficult, it has multiple paths that are immediately ready with a full body that I didn't want to break apart. So I kept the `Response` message for a full response that transforms into a `ContentSourceContent::Static`, and implemented a new enum type for streaming responses into a `ContentSourceContent::HttpProxy`. These is some weirdness with different data types on the `headers` field of a `ContentSourceContent::Static`'s `StaticContent` and a `ContentSourceContent::HttpProxy`'s `ProxyResultVc`. One is a `HeaderListVc` and the other is a `Vec<(String, String)>` (and really should be a `HeadersList`), but I'm leaving this for a follow up. ### Testing Instructions Paired Next.js PR: #47476 Re: WEB-27

Follow up to vercel/turborepo#4329, this removes `BodyError` (which was just a simple `String` and not an `Error`) and switches to `SharedError`. I've implemented a basic `PartialEq` and `Serialization` for `SharedError` so that it doesn't infect everything that uses `Body`.

### Description Implements streaming rendering for static/proxy renders (I don't know why they're called static or proxy…) using an initial `Headers` message, N `BodyChunk` messages, and a final `BodyEnd` message. Updating proxy rendering was very easy. It already implemented separate messages for header and body, but it buffered the full body before sending the full contents. But static rendering was a little more difficult, it has multiple paths that are immediately ready with a full body that I didn't want to break apart. So I kept the `Response` message for a full response that transforms into a `ContentSourceContent::Static`, and implemented a new enum type for streaming responses into a `ContentSourceContent::HttpProxy`. These is some weirdness with different data types on the `headers` field of a `ContentSourceContent::Static`'s `StaticContent` and a `ContentSourceContent::HttpProxy`'s `ProxyResultVc`. One is a `HeaderListVc` and the other is a `Vec<(String, String)>` (and really should be a `HeadersList`), but I'm leaving this for a follow up. ### Testing Instructions Paired Next.js PR: #47476 Re: WEB-27

Follow up to vercel/turborepo#4329, this removes `BodyError` (which was just a simple `String` and not an `Error`) and switches to `SharedError`. I've implemented a basic `PartialEq` and `Serialization` for `SharedError` so that it doesn't infect everything that uses `Body`.

jridgewell requested a review from a team as a code owner March 24, 2023 04:18

turbo-orchestrator bot added the team: turbopack label Mar 24, 2023

jridgewell mentioned this pull request Mar 24, 2023

Turobpack: Implement Streaming App/API rendering vercel/next.js#47476

Merged

jridgewell force-pushed the jrl-rsc-streaming branch from 7a7338e to 5582363 Compare March 24, 2023 04:31

sokra requested changes Mar 27, 2023

View reviewed changes

crates/turbopack-node/src/render/mod.rs Outdated Show resolved Hide resolved

crates/turbopack-node/src/render/mod.rs Outdated Show resolved Hide resolved

jridgewell added 5 commits March 28, 2023 19:01

Update proxy handler

6f0d0ed

Implement streamed static rendering

75f44d9

Fix merge conflicts

ece41f2

Implement Stream Cell approach for proxy renders

2f36ed4

Implement Stream Cell approach for static renders

831a267

jridgewell force-pushed the jrl-rsc-streaming branch from 2fe5db9 to 831a267 Compare March 29, 2023 03:32

jridgewell commented Mar 29, 2023

View reviewed changes

jridgewell requested a review from sokra March 29, 2023 03:58

sokra requested changes Mar 29, 2023

View reviewed changes

jridgewell added 2 commits March 29, 2023 12:53

Fix strongly_consistent usage for pools

7fe1eca

Use try_next unwrapping

84d95f1

jridgewell requested a review from sokra March 29, 2023 21:50

sokra approved these changes Mar 29, 2023

View reviewed changes

jridgewell merged commit ffb7e70 into main Mar 29, 2023

jridgewell deleted the jrl-rsc-streaming branch March 29, 2023 22:24

jridgewell mentioned this pull request Mar 29, 2023

Use SharedError for body streaming #4392

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement Streaming Static/Proxy Rendering #4329

Implement Streaming Static/Proxy Rendering #4329

jridgewell commented Mar 24, 2023 •

edited

Loading

vercel bot commented Mar 24, 2023 •

edited

Loading

github-actions bot commented Mar 24, 2023 •

edited

Loading

github-actions bot commented Mar 29, 2023

jridgewell left a comment

sokra left a comment

sokra Mar 29, 2023

jridgewell Mar 29, 2023

sokra Mar 29, 2023

sokra Mar 29, 2023

jridgewell Mar 29, 2023

sokra Mar 29, 2023

jridgewell Mar 29, 2023

sokra left a comment

	pub type BodyChunk = Result<Bytes, BodyError>;
	pub type BodyChunk = Result<Bytes, SharedError>;

Implement Streaming Static/Proxy Rendering #4329

Implement Streaming Static/Proxy Rendering #4329

Conversation

jridgewell commented Mar 24, 2023 • edited Loading

Description

Testing Instructions

vercel bot commented Mar 24, 2023 • edited Loading

github-actions bot commented Mar 24, 2023 • edited Loading

🟢 CI successful 🟢

github-actions bot commented Mar 29, 2023

jridgewell left a comment

Choose a reason for hiding this comment

sokra left a comment

Choose a reason for hiding this comment

sokra Mar 29, 2023

Choose a reason for hiding this comment

jridgewell Mar 29, 2023

Choose a reason for hiding this comment

sokra Mar 29, 2023

Choose a reason for hiding this comment

sokra Mar 29, 2023

Choose a reason for hiding this comment

jridgewell Mar 29, 2023

Choose a reason for hiding this comment

sokra Mar 29, 2023

Choose a reason for hiding this comment

jridgewell Mar 29, 2023

Choose a reason for hiding this comment

sokra left a comment

Choose a reason for hiding this comment

jridgewell commented Mar 24, 2023 •

edited

Loading

vercel bot commented Mar 24, 2023 •

edited

Loading

github-actions bot commented Mar 24, 2023 •

edited

Loading