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

Using CDN like CloudFront, CloudFlare on App Directory breaks everything #49140

Closed
wants to merge 2 commits into from
Closed

Using CDN like CloudFront, CloudFlare on App Directory breaks everything #49140

wants to merge 2 commits into from

Conversation

HamAndRock
Copy link

Hello this is my first PR, sorry about the mess :)

What?

With app directory being used for production app, we want to add CDN to cache HTML responses (yes we can leverage static rendering etc. + we are self hosting on AWS).

Hovewer we still ideally want to cache the HTML on a CDN, hovewer we ran into and issue where the same url for the page can return 2 payload depending on the content-type, with CloudFront and CloudFlare you can't cache on level of mime types. So we can't cache the RSC payload with x-component type and the HTML output with mime type HTML separately. This results into several errors when the RSC payload is replaced with HTML that was cached from CDN and router doesn't find HTML to match the spec of x-component and starts doing infinite loop of page refreshes.

Some screenshots of the cached payload and the errors statis that RSC is not matching what it should be and causing a page reloads infinitely.

image

This got cached on CDN first, returning just the RSC payload instead of HTML, after a while it started returning normal HTML but causing those RSC fetch fail errors.

image

Reproduce?

To reproduce create any app directory project with server components and add a CDN in front of it.

Why?

We want to leverage selfhosted next app director to use CDNs like CloudFront and CloudFlare.

How?

Add rscQuery query param to differ between RSC payload and HTML payload for CDN caching purposes. This is totally not the best solution but allowed use to have set of 2 different urls that we can cache on URL basis. I am not sure if adding this rscQuery prop doesn't break anything internally but we tested this on project that will be shipped soon and we ran into 0 issues, but again this is a "patch" level fix, the problem is more complicated probably.

Add `rscQuery` query param to differ between RSC payload and HTML payload for CDN caching purposes
@HamAndRock HamAndRock changed the title Update fetch-server-response.ts Using CDN like CloudFront, CloudFlare on App Directory breaks everything May 3, 2023
@Fredkiss3
Copy link
Contributor

Hello, there is an issue regarding this, one of the next core team answered that normally that next adds a Vary header by default to fix this, cloudfare & cloudfront don't respect them (which is weird, since it is a standard).

So i proposed a solution for cloudfare : #48569 (comment) which is basically what you are trying to do here, adding query params for each value in the Vary header. I think you could do the same with cloudfront (i don't really know) ?

@khuezy
Copy link
Contributor

khuezy commented May 4, 2023

CloudFlare/Front not honoring Vary is weird... but to work around this issue, you can add the "rsc", "react-router-state-tree" cache control policy. That will differentiate between the /page and the x-component type.

@juanferreras
Copy link

cc @timneutkens would it be possible to get your opinion (or any other core member's) on this issue?

Next.js is doing things correctly by setting the Vary header, but if most CDNs users are leveraging (when not serving from Vercel) will not be compatible with this approach, it'd still block adoption (and/or harm perceived stability) for the app directory. Seeing users cite issues on Cloudflare (which might be proxying Vercel) and AWS Cloudfront, DigitalOcean and Firebase for self-hosted options.

Is there any downside towards a hybrid approach with query strings too? Many thanks!

(relevant tweet: https://twitter.com/matveydotgg/status/1655563317056774146)

@timneutkens
Copy link
Member

timneutkens commented May 16, 2023

We're going to solve this issue where cloudflare/firebase etc don't respect Vary soon, even when Next.js sets the right HTTP headers as per the spec. Some providers seem to only allow differential serving of requests on their enterprise plan. The idea right now is to change the RSC header to be one header instead of 3 and add a querystring hash of the router state tree.

The important context here is that we started with searchParams initially, but it turned out to not be the right fit as the url would get too long with the router state that the server needs, causing navigations to break. Additionally it would require additional parsing of the url to make e.g. middleware work correctly.

The wayback machine issue is different though, from what I'm seeing it seems their crawler is not correctly applying Vary, causing any fetch() and such to be cached. We've reached out to them via the available email address, haven't heard back on that yet.

@hirvesh
Copy link

hirvesh commented Jun 2, 2023

@timneutkens - any help needed here to get this over the line? 🤔

timneutkens pushed a commit that referenced this pull request Jun 12, 2023
Adding a `_rsc` query for RSC payload requests so that they can be
differentiated on resources level for CDN cache for the ones that didn't
fully respect to VARY header.

Also stripped them for node/edge servers so that they won't show up in
the url

x-ref:
#49140 (comment)

Closes #49140
Closes NEXT-1268
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants