[Flight] Don't hang forever when prerendering a rejected promise

[unstubbable]

Rendering a rejected promise as the root model is already supported in renderToReadableStream. In this case, a stream is returned early, and reading the stream will start flowing, so that enqueued chunks are flushed.

When prerendering, flowing is only started when onAllReady is called. This was previously only done in performWork. However, if the root model is a rejected promise, erroredTask also needs to check if there are any abortable tasks left, and if there are none, it must call onAllReady. This ensures that the prerender doesn't hang, and a prelude stream is returned.

In addition, this fixes a bug where we returned the prelude early when a readable stream or async iterable were still in progress. The fix for this issue is to ensure that abortListeners is also empty before calling onAllReady.

[react-sizebot]

Comparing: 7213509...66ff9e7

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-stable/react-dom/cjs/react-dom.production.js	=	6.68 kB	6.68 kB	=	1.83 kB	1.83 kB
oss-stable/react-dom/cjs/react-dom-client.production.js	=	516.14 kB	516.14 kB	=	91.87 kB	91.87 kB
oss-experimental/react-dom/cjs/react-dom.production.js	=	6.69 kB	6.69 kB	=	1.83 kB	1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js	=	621.45 kB	621.45 kB	=	110.00 kB	110.00 kB
facebook-www/ReactDOM-prod.classic.js	=	654.82 kB	654.82 kB	=	115.57 kB	115.57 kB
facebook-www/ReactDOM-prod.modern.js	=	645.10 kB	645.10 kB	=	114.04 kB	114.04 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-stable-semver/react-server/cjs/react-server-flight.production.js	+0.22%	57.82 kB	57.95 kB	+0.10%	11.64 kB	11.65 kB
oss-stable/react-server/cjs/react-server-flight.production.js	+0.22%	57.82 kB	57.95 kB	+0.10%	11.64 kB	11.65 kB
oss-experimental/react-server/cjs/react-server-flight.production.js	+0.20%	62.78 kB	62.91 kB	+0.08%	12.51 kB	12.52 kB

Generated by 🚫 dangerJS against 66ff9e7

[sebmarkbage]

I don't love the approach of just checking this everywhere deep inside various places. Because it adds checks in hot paths that get called over and over again. It should ideally only be checked inside a path where we've already checked something else to indicate that we're at the end. Or at least only if we're already decrementing or deleting something at the same time instead of passively all the time.

However, there's also a timing issue that I've tried to avoid calling callbacks in things like flush (but ofc things like fatal errors still need that) since that can happen deep inside like a node streaming stack. Where as performWork is more top level of the stack and safer e.g. if it throws or reasons about state.

[sebmarkbage]

I think the conclusion is just that I'd like to be able to see the mutation to the set that we read somewhat close to where we call the callback (but not moving the callback too early because then that can lead to inconsistencies when it observes a partial state or it throws).

[gnoff]

If we move the allready call back into performWork we need to be wary of a bug that was present in the original.

It's possible to be in a state where there are no abortableTasks but there is an abortListener. If the async streamable thing with the listener emits a value it can schedule a new task.

The thing the hasAbortableTasks thing was trying to prevent was a ping-after-abort causing a second call to onAllReady. But we can probably just read the request status after retrying tasks. If it's not OPENING or OPEN then we can I think just omit the onAllReady call

[gnoff]

We could handle tasks in performWork and listeners in the callbacks. The only time we'd clear a task is if we're in a performWork loop so we can always be certain we'll handle those there.