React 18: "The stream is not in a state that permits close" in renderToReadableStream #22772

Activity

1. jplhomer

   added

   React 18Bug reports, questions, and general feedback about React 18Bug reports, questions, and general feedback about React 18

   Type: Discussion

   on Nov 15, 2021
2. gaearon commented on Nov 18, 2021

   

   gaearon

   on Nov 18, 2021

   Collaborator

   More actions

   Have you looked at what chain of assumptions leads to this? It seems like it would be good to understand what different state machines are involved and why something's happening in CF that doesn't happen in the browser.
3. jplhomer

   changed the title [-]React 18: "The stream is not in a state that permits close" in `pipeToReadableStream`[/-] [+]React 18: "The stream is not in a state that permits close" in `renderToReadableStream`[/+] on Dec 1, 2021
4. jplhomer commented on Dec 1, 2021

   

   jplhomer

   on Dec 1, 2021

   ContributorAuthor

   More actions

   Have you looked at what chain of assumptions leads to this?

   @gaearon after lots of digging, here's what we've discovered:

   The difference between Cloudflare/Oxygen Workers runtimes and the browser comes down to how the stream is consumed after being passed to Response.

   In the browser and related fizz tests, this is how the stream (via response.body) is consumed:

   await response.blob();

   This presumably results in a single pull.

   In the worker runtime, response.body is read through an async iterator:

   for await (const chunk of response.body) {
     // write chunk from server
   }

   Importantly, this results in pull getting called multiple times as more data is enqueued and as the reader sees fit. This is an assumption on my part, so I could be wrong here.

   When combined with Suspense boundaries, pingTask callbacks trigger flushCompletedQueues. At the same time, repeated pull requests from the stream reader also call flushCompletedQueues.

   This results in the following error on the server:

   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)
   

   You can also reproduce this directly in the browser by attempting to read the stream with response.body.getReader(). Here is a new fixture with a reproduction of the issue, including a couple test cases that show the different errors you'll see based on the number of Suspense boundaries you have:

   https://github.com/jplhomer/react/blob/jl-fizz-ssr-streaming-browser/fixtures/fizz-ssr-browser-streaming/index.html

   If this seems like a legitimate bug to you, a couple ideas I had:

   • Should there be some sort of check like if (isAlreadyFlushingQueues) return in flushCompletedQueues?
   • Should we splice completed boundaries earlier to account for concurrency so they are removed from the array and not re-flushed?

   Here are some other resources and scratch notes: jplhomer/vite-streaming-ssr-demo#1 (comment)

   👍React with 👍5Reacted by dan, Fran Dios, Bret Little, tosuke and Andrea Ascari
5. jplhomer

   added a commit that references this issue on Dec 9, 2021

   fix: prevent concurrent flushing issues by tracking flushing status.

   ...

   4c5d67c
6. salazarm

   assigned

   sebmarkbage

   on Feb 22, 2022
7. sebmarkbage commented on Feb 23, 2022

   

   sebmarkbage

   on Feb 23, 2022

   Last edited by sebmarkbage

   Contributor

   More actions

   We'll fix this reentrancy bug somehow. However, right now it's a bit lower priority because CloudFlare technically don't support ReadableStreams and so you have to go through the polyfill and any subtle differences might be due to the polyfill. To really know if we fixed this issue we should apply this to native CloudFlare ReadableStreams... which I'm told is coming. So that's how we know if it actually worked. Until then, the support for CloudFlare workers is... partial.

   On a second thought this should probably fix it. I think I finally understand how it's supposed to work now. #23342

   👍React with 👍2Reacted by Tyler Brostrom and Fran Dios
8. sebmarkbage

   added 2 commits that reference this issue on Feb 23, 2022

   Re-add reentrancy avoidance

   ...

   This issue will close once commit dbd4eac is merged into the 'master' branch.

   dbd4eac

   Re-add reentrancy avoidance

   ...

   This issue will close once commit 8ecc536 is merged into the 'master' branch.

   8ecc536
9. sebmarkbage

   closed this as completedin #23342on Feb 23, 2022
10. sebmarkbage

    added a commit that references this issue on Feb 23, 2022

    Re-add reentrancy avoidance (#23342)

    ...

    This issue will close once commit 2c693b2 is merged into the 'main' branch.

    2c693b2
11. sebmarkbage commented on Feb 23, 2022

    

    sebmarkbage

    on Feb 23, 2022

    Contributor

    More actions

    @jplhomer Please take another look and see if this fixes all the issues you've noticed.

    👍React with 👍2Reacted by Fran Dios and Josh Larson
12. jplhomer commented on Feb 23, 2022

    

    jplhomer

    on Feb 23, 2022

    ContributorAuthor

    More actions

    @sebmarkbage works like a charm! Thanks for taking the time to look at it and get it fixed 🎉
13. zhengjitf

    added a commit that references this issue on Apr 15, 2022

    Re-add reentrancy avoidance (react#23342)

    ...

    This issue will close once commit 16a5c9d is merged into the 'main' branch.

    16a5c9d
14. mrizwanashiq

    added a commit that references this issue on Jun 25, 2026

    Re-add reentrancy avoidance (react#23342)

    ...

    This issue will close once commit 4104c78 is merged into the 'main' branch.

    4104c78