jplhomer / vite-streaming-ssr-demo

19 stars 7 forks source link

`renderToReadableStream` breaks in Workers runtime #1

Closed jplhomer closed 2 years ago

jplhomer commented 2 years ago

When bundling a worker file and running it with miniflare, the following error is observed in the server console:

TypeError: The stream is not in a state that permits close
    at ReadableStreamDefaultController.close (/Users/joshlarson/src/github.com/jplhomer/vite-streaming-ssr-demo/node_modules/web-streams-polyfill/src/lib/readable-stream/default-controller.ts:72:13)
    at Mc (/Users/joshlarson/src/github.com/jplhomer/vite-streaming-ssr-demo/dist/worker/worker.js:3636:145)
    at Bc (/Users/joshlarson/src/github.com/jplhomer/vite-streaming-ssr-demo/dist/worker/worker.js:3457:33)
    at ping (/Users/joshlarson/src/github.com/jplhomer/vite-streaming-ssr-demo/dist/worker/worker.js:3128:24)
GET / 200 OK (2091.13ms)

In the browser, it appears as though it works, but you'll also notice a browser console error:

Uncaught TypeError: Cannot read properties of null (reading 'parentNode')
    at $RS (127.0.0.1/:11)
    at 127.0.0.1/:11

This is because the stream pushing out an invalid chunk:

...
<div hidden id="<div hidden id="S:1">
...

To repro:

yarn && yarn build && yarn workers
jplhomer commented 2 years ago

Tracking here: https://github.com/facebook/react/issues/22772

jplhomer commented 2 years ago

I just updated to miniflare@next which uses Node's native Web Streams API under the hood for ReadableStream. No more polyfill!

And boom, we've just isolated our issue:

TypeError: Invalid state: Controller is already closed
    at new NodeError (node:internal/errors:371:5)
    at ReadableStreamDefaultController.close (node:internal/webstreams/readablestream:956:13)
    at Mc (/Users/joshlarson/src/github.com/jplhomer/vite-streaming-ssr-demo/dist/worker/worker.js:3634:145)
    at Bc (/Users/joshlarson/src/github.com/jplhomer/vite-streaming-ssr-demo/dist/worker/worker.js:3455:33)
    at ping (/Users/joshlarson/src/github.com/jplhomer/vite-streaming-ssr-demo/dist/worker/worker.js:3126:24) {
  code: 'ERR_INVALID_STATE'
}

This appears to be a bug in the way React is writing to the stream when in a SW/Workers runtime.

jplhomer commented 2 years ago

By modifying the Suspense contents a bit in App.jsx, we can trigger a new (but possibly related) error:

          <Suspense fallback={<Spinner />}>
            <Comments />
          </Suspense>
+         <Suspense fallback={<Spinner />}>
+           <Comments />
+         </Suspense>

If you run yarn build-dev && yarn workers, you'll see the following error:

Error: Aborted, errored or already flushed boundaries should not be flushed again. This is a bug in React.
    at flushSubtree (/Users/joshlarson/src/github.com/jplhomer/vite-streaming-ssr-demo/dist/worker/worker.js:12538:15)
    at flushSegment (/Users/joshlarson/src/github.com/jplhomer/vite-streaming-ssr-demo/dist/worker/worker.js:12545:14)
    at flushSegmentContainer (/Users/joshlarson/src/github.com/jplhomer/vite-streaming-ssr-demo/dist/worker/worker.js:12583:5)
    at flushPartiallyCompletedSegment (/Users/joshlarson/src/github.com/jplhomer/vite-streaming-ssr-demo/dist/worker/worker.js:12622:7)
    at flushCompletedBoundary (/Users/joshlarson/src/github.com/jplhomer/vite-streaming-ssr-demo/dist/worker/worker.js:12591:7)
    at flushCompletedQueues (/Users/joshlarson/src/github.com/jplhomer/vite-streaming-ssr-demo/dist/worker/worker.js:12659:14)
    at performWork (/Users/joshlarson/src/github.com/jplhomer/vite-streaming-ssr-demo/dist/worker/worker.js:12497:9)
    at /Users/joshlarson/src/github.com/jplhomer/vite-streaming-ssr-demo/dist/worker/worker.js:11752:16
    at scheduleWork (/Users/joshlarson/src/github.com/jplhomer/vite-streaming-ssr-demo/dist/worker/worker.js:8042:5)
    at pingTask (/Users/joshlarson/src/github.com/jplhomer/vite-streaming-ssr-demo/dist/worker/worker.js:11751:7)

This indicates that React is attempting to flush the same segment multiple times.

Why is this? Let's find out...

If we walk down the stacktrace, all the way to flushCompletedQueues, we can throw an Exception, catch it, and print out the stacktrace to find out the caller:

function flushCompletedQueues(request, destination) {
  try {
    throw new Error('test');
  } catch (e) {
    console.log(`flushCompletedQueues called from stack: ${e.stack.split('\n').slice(1).join('\n')}`);
  }

  // ...
}

This leads to the following output when we run the script again:

flushCompletedQueues called from stack:     at flushCompletedQueues (/Users/joshlarson/src/github.com/jplhomer/vite-streaming-ssr-demo/dist/worker/worker.js:12628:13)
    at performWork (/Users/joshlarson/src/github.com/jplhomer/vite-streaming-ssr-demo/dist/worker/worker.js:12497:9)
    at /Users/joshlarson/src/github.com/jplhomer/vite-streaming-ssr-demo/dist/worker/worker.js:11752:16
    at scheduleWork (/Users/joshlarson/src/github.com/jplhomer/vite-streaming-ssr-demo/dist/worker/worker.js:8042:5)
    at pingTask (/Users/joshlarson/src/github.com/jplhomer/vite-streaming-ssr-demo/dist/worker/worker.js:11751:7)
    at ping (/Users/joshlarson/src/github.com/jplhomer/vite-streaming-ssr-demo/dist/worker/worker.js:11778:16)

flushCompletedQueues called from stack:     at flushCompletedQueues (/Users/joshlarson/src/github.com/jplhomer/vite-streaming-ssr-demo/dist/worker/worker.js:12628:13)
    at startFlowing (/Users/joshlarson/src/github.com/jplhomer/vite-streaming-ssr-demo/dist/worker/worker.js:12718:7)
    at Object.pull (/Users/joshlarson/src/github.com/jplhomer/vite-streaming-ssr-demo/dist/worker/worker.js:12753:11)
    at ensureIsPromise (node:internal/webstreams/util:172:19)
    at readableStreamDefaultControllerCallPullIfNeeded (node:internal/webstreams/readablestream:1852:5)
    at readableStreamDefaultControllerEnqueue (node:internal/webstreams/readablestream:1794:3)
    at ReadableStreamDefaultController.enqueue (node:internal/webstreams/readablestream:966:5)
    at writeChunk (/Users/joshlarson/src/github.com/jplhomer/vite-streaming-ssr-demo/dist/worker/worker.js:8047:17)
    at writeStartSegment (/Users/joshlarson/src/github.com/jplhomer/vite-streaming-ssr-demo/dist/worker/worker.js:10209:9)
    at flushSegmentContainer (/Users/joshlarson/src/github.com/jplhomer/vite-streaming-ssr-demo/dist/worker/worker.js:12582:5)

[then the error seen above]

This makes me wonder: is the ReadableStreamController calling pull at some cadence, maybe due to the async iterator in the server, which conflicts with pingTask?

Should there be some sort of check like if (isAlreadyFlushingQueues) return?

Or maybe splice completed boundaries/segments early so they are removed from their arrays?

jplhomer commented 2 years ago

This is fixed upstream! 🎉 https://github.com/facebook/react/pull/23342