Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

renderToReadableStream breaks in Workers runtime #1

Closed
jplhomer opened this issue Nov 15, 2021 · 4 comments
Closed

renderToReadableStream breaks in Workers runtime #1

jplhomer opened this issue Nov 15, 2021 · 4 comments

Comments

@jplhomer
Copy link
Owner

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
Copy link
Owner Author

Tracking here: facebook/react#22772

@jplhomer
Copy link
Owner Author

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
Copy link
Owner Author

jplhomer commented Nov 30, 2021

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 jplhomer changed the title pipeToReadableStream breaks in Workers runtime renderToReadableStream breaks in Workers runtime Dec 1, 2021
@jplhomer
Copy link
Owner Author

This is fixed upstream! 🎉 facebook/react#23342

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant