Streaming promises broken on Safari (due to Safari not rendering incomplete response)

oliver-ni commented 1 year ago

Describe the bug

Streaming promises, i.e., returning a nested promise in a server-side data loader, doesn't work properly in Safari.

I believe the way SvelteKit attempts to accomplish this is by first sending the initial HTML document, keeping the connection alive, and then finally ending the response with another script tag that patches the previously sent HTML.

However, this would rely on the browser starting to render the document before the response is fully received, which is not something that should be taken for granted — Safari, for example, does not do this, and waits for the response to be fully received before rendering the page. That means, if the promise takes 10 seconds to resolve, the page will not begin to load (and not show the loading state as intended) for the entire 10 seconds, before finally loading with the data fully rendered.

Comparison between the two browsers:

https://github.com/sveltejs/kit/assets/20295134/55365b01-0c61-4c59-a165-1c9e54f8f95c

Reproduction

Repository: https://github.com/Rich-Harris/sveltekit-on-the-edge

Relevant line Relevant lines

Reproduction steps:

Visit https://sveltekit-on-the-edge.vercel.app/edge/streaming
On Safari, the webpage loads for one second, rendering nothing, then finally loads with City and IP directly
On Chrome, the webpage loads immediately with the loading state, then one second later, shows City and IP

Logs

No response

System Info

System:
  OS: macOS 13.0
  CPU: (8) arm64 Apple M2
  Memory: 47.52 MB / 16.00 GB
  Shell: 5.8.1 - /bin/zsh
Binaries:
  Node: 18.15.0 - /run/current-system/sw/bin/node
  npm: 9.5.0 - /run/current-system/sw/bin/npm
  pnpm: 8.3.1 - /run/current-system/sw/bin/pnpm
Browsers:
  Chrome: 114.0.5735.198
  Safari: 16.1
npmPackages:
  @sveltejs/adapter-auto: ^2.0.0 => 2.1.0 
  @sveltejs/kit: ^1.20.4 => 1.21.0 
  svelte: ^4.0.0 => 4.0.1 
  vite: ^4.3.6 => 4.3.9

Severity

serious, but I can work around it

Additional Information

No response

elliott-with-the-longest-name-on-github commented 1 year ago

This might be related:

https://github.com/sveltejs/kit/issues/9154#issuecomment-1461169811

oliver-ni commented 1 year ago

I don't know if this approach is reliable. Perhaps it's better to use SSE or ws for this feature.

dummdidumm commented 1 year ago

There's nothing actionable on SvelteKit's side - if Safari doesn't support streaming promises (hopefully only with chunks below a certain threshold as indicated by #9154) then there's nothing we can do about that. How would a solution look like that solves this? I don't see one.

oliver-ni commented 1 year ago

Upon further investigation, I noticed that adding some more bytes to the initial response caused Safari to start rendering the first chunk of data before waiting on the second. However, if the initial chunk is not large enough, Safari doesn't render in chunks at all but waits for the entire response to be completed.

Actually, when I made the issue, I had a slightly wrong impression of how SvelteKit was accomplishing this. (I noticed that the responses weren't sending Transfer-Encoding: chunked, but I later learned that HTTP/2 had its own way of doing streaming, and SvelteKit did used the aforementioned header for HTTP/1.1.)

Still, although I am not intimately familiar with HTTP standards, based on the research I've done, I don't believe that this type of "progressive rendering" should be relied on for this feature. In the end, it's up to the browser to decide when to start rendering chunked content. The primary purpose of Transfer-Encoding: chunked and HTTP/2 streaming seems to be enabling the sending of dynamically generated content that the server doesn't know the length of beforehand (in which case it would not be able to send a Content-Length header).

That is, it's not part of any standard that browsers must begin to render chunks as they come in. Rather, this is an optimization technique that certain browsers have implemented in order to make pages seem to load more quickly to the end user, even if they have not been completely received. We should not take this implementation-specific optimization behavior for granted — it's not predictable nor guaranteed and may change on a whim.

Especially since it is not advertised anywhere in the SvelteKit docs that this feature may be unreliable (which it has proven to be, through this issue as well as #9154).

Maybe we can explore rebuilding this "streaming promises" feature through something like Server-Sent Events? SSE is a standardized and well-supported feature that is specifically intended for purposes such as these.

oliver-ni commented 1 year ago

I also noticed that in Safari, it's not simply the byte size of the chunk that determines whether Safari will start rendering. For example, none of the following got Safari to start "progressive rendering" for me:

Adding a large amount of text inside an HTML comment
Adding a large amount of text inside a container with display: none;
Adding a large amount of text inside a container with visibility: hidden;
Adding a large amount of CSS in a style tag
Adding a large amount of JS in a script tag

Seems like it only starts when there is some threshold amount of "real content", by some metric, to display.

More hints as to why we shouldn't try to rely on this behavior...

elliott-with-the-longest-name-on-github commented 1 year ago

SSE most likely is not an option, as it's not supported by many of the platforms Kit sites are deployed to.

elliott-with-the-longest-name-on-github commented 1 year ago

The only way to "fix" this that I can think of is:

Rather than sending one response to the browser that finishes streaming whenever all outstanding promises are resolved, send a response to the browser that contains unresolved promises for all of the deferred data, along with code to initiate a client-side fetch of the deferred data to the server. This would result in the first request completing, then a second request being fired to finish getting the deferred data. Since JS is already required for deferred data, this isn't introducing any dependencies that weren't there before. The downside here being that we now have two requests, and the deferred request can't start until after hydration, even if the deferred data is done resolving on the server -- which kind of defeats the purpose. At that point, you may as well just use an onMount fetch.

FWIW, this problem also affects Next.js's streaming-Suspense-boundary-server-components thing in v13. (I noticed it while building out an internal site at Vercel.) I'm not sure there's any realistic thing we can do to fix this. ☹️

dummdidumm commented 1 year ago

Maybe we just need to lean back and wait, now that streaming is becoming more popular, browser will surely feel pressured to do something about this.

pilcrowonpaper commented 1 year ago

From my testing Safari only renders HTML chunks larger than around 512 bytes of rendered content for text/html responses (this is not the case for application/json). It's either based on rendered data size or a certain "pixel count" threshold.

See the bug report here: https://bugs.webkit.org/show_bug.cgi?id=252413

It's possible that it'll only affects smaller websites (like demos) but I'm not sure.

MauScheff commented 10 months ago

In any case, I think it's best for everyone to make a note in the docs that this feature may not work on some browsers. That way, developers don't have to find out he hard way. Eg. I was developing and testing in chrome, until one day I realized things work differently in Safari and I need to rethink my approach.

timootten commented 2 months ago

To solve this issue, as mentioned in this Remix GitHub issue, you can add the following code to the layout or any pages that use streaming:

const sp = '\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b\u200b';

<div style="width: 0px; height: 0px;">{@html sp}</div>

Antonio-Bennett commented 2 months ago

@timootten what does this even do?

timootten commented 2 months ago

@Antonio-Bennett Safari requires a specific amount of bytes to render the first chunk of data, and that should be enough to ensure it works on Safari as well. I only tested this on my mobile Safari app. While this might not be the most optimal solution, it works fine for now.

Antonio-Bennett commented 2 months ago

@timootten ahhh okay makes sense thanks for the explanation

sveltejs / kit