[BUG] Render cache is reset when handling multiple requests at the same time

[frandiox]

Describe the bug
We currently use <RenderCacheProvider> in SSR to provide a cache where fetch promises are placed for the different rendering cycles of a request. This works with 1 request at a time but looks like it doesn't when handling multiple requests at the same time.

The cache provider has an initial value that should be replaced by each request's cache. However, it looks like the context gets reset to the initial value before the rendering finishes.

To Reproduce

1. Checkout main branch.
2. Add a proxy around the default cache context in foundation/RenderCacheProvider/RenderCacheContext.tsx such as this:

import {createContext} from 'react';
import type {RenderCacheProviderProps} from './types';

export const RenderCacheContext = createContext<RenderCacheProviderProps>({
-  cache: {},
+  cache: new Proxy({} as RenderCacheProviderProps['cache'], {
+    get(target, key: string) {
+      console.log('-- default cache get');
+      return target[key];
+    },
+    set(target, key: string, value) {
+      console.log('-- default cache set');
+      target[key] = value;
+      return true;
+    },
+  }),
});

3. Run the dev project normally and load the home page. Notice that the proxy above is not logging anything because the default cache gets replaced by <RenderCacheProvider>, which provides a new cache object for the current request.
4. Trigger 2 requests simultaneously. This can be done with 2 tabs opened or running the following code in the browser:

function doRequest() { return fetch('http://localhost:3000', {headers: {accept:'text/html'}}).then(r => r.text()) }
await Promise.all([doRequest(), doRequest()])

5. The terminal will start showing logs from the default cache Proxy. It looks like the <RenderCacheProvider> provides a new cache object per request at first, but at some point the Context gets reset to the initial value (the proxy above).

Expected behaviour
The proxy should not log anything because it must be replaced by the per-request-cache from the provider.

Additional context
Optionally, remove hydration from dev/src/entry-client.jsx (comment out export default renderHydrogen(...) to make sure RSC is not triggered. The bug is with the SSR part.

I found this issue while testing the SSR + RSC approach in #390 and noticed that the SSR part was buggy when handling multiple requests.

Could this be a bug in React experimental, @wizardlyhel ?

[wizardlyhel]

I remember now!

Here is the output of a page that has an iframe request to another template.

What's happening is that -- sometimes -- network cancels on request and tries again (not sure where the retry logic is coming from but seems to be browser driven) Browser seems to take the last successful response .

When the server gets these requests, it will try to process them. But when the connection closes, the async operation has no where to go because the associating callback functions has already been discarded. React's default behaviour is to use the default cache set in the RenderCacheContext.

This was the reason why I had a default cache to capture these failed requests' async operations. These cache values will never get used because they will never be bound to a RenderCacheContext of a request. (This is a memory leak but will only be limited to the number of unique failed requests)

[frandiox]

@wizardlyhel So this is not a real issue? I'm not sure if I understand completely the network related stuff.
When we do two requests with the code from OP, those are still 2 valid and separate requests that don't have any network-related problem, right? Or, since they are triggered at the same time and look exactly the same, the browser is just "canceling" one of them and just getting the result from the other?

Edit: I've tried with 2 requests from different browsers (Chrome and Safari) at the same time and I still see logs for the default cache 🙈

[jplhomer]

Per https://reactjs.org/docs/context.html#reactcreatecontext

The defaultValue argument is only used when a component does not have a matching Provider above it in the tree. This default value can be helpful for testing components in isolation without wrapping them.

Makes me think that some subsection of the app tree is being re-rendered outside of the scope of the provider with the value. Might be related to R18 streaming, or concurrent requests. I'm curious what other global JS state (shared between requests in Vite dev server) might be affecting this?

Could this be a bug in React experimental

Can we reproduce it in e.g. https://github.com/jplhomer/vite-streaming-ssr-demo ?

[frandiox]

@jplhomer Yes, if I didn't mess it up. Adding a log below this line (console.log("This should never be null:", ctx);) shows this:

It should not be null since there is always a provider, right?

[wizardlyhel]

Okay back to square one ...

I rechecked the code I had and found some errors with it .. not related to this particular issue but .. it might be something we should be aware of.

When streaming html into the DOM, it will request resources even if it is hidden .. for example, the iframe example I had:

1. An iframe is streamed in as a hidden html block
2. An inline script code removes the hidden html block and appends it to another html block - this is where we see the cancelled document network request and a new request for the same document

This can happen as many times as streaming needs to organize the app output.

FIX: Make sure the iframe component is a client component and the src attribute must be set with an useEffect or on componentWillMount

This behaviour will happen to any DOM resource:

• <img> - (positive effect) this turns out benefits loading as images will start the network request earlier and browser will cache the request
• <script src> - (unknown effect) - has the same network loading benefit but loses the control over when the script will execute
• inline <script> - (Needs to verify) inline script will run when streamed but and will not rerun when it is appended. May break bounded events when moving html elements
• <link href="css" /> - (Needs to verify) On stream, new css styles may cause reflow.

[wizardlyhel]

Verified it is accessing the default cache even if it is different url. They just have to be requests happening relatively at the same time.

[wizardlyhel]

I'm running out of ideas to debug this

[wizardlyhel]

I am wondering if this is a bug from retryTask from React DOM Server

Dangling scheduled work?

[frandiox]

@wizardlyhel It indeed looked like a bug in React to me. I saw React is internally pushing and popping contexts, and it looked like it was popping too much at some point. I did not find the root cause, though, so this is just an assumption. The fact that this issue is reproducible in a simpler repo also supports this...