withastro / astro

The web framework for content-driven websites. ⭐️ Star to support our work!
https://astro.build
Other
46.62k stars 2.47k forks source link

Server Islands unusable behind Cloudflare #11638

Closed DeJayDev closed 2 months ago

DeJayDev commented 3 months ago

Astro Info

Astro                    v4.13.1
Node                     v20.16.0
System                   Linux (x64)
Package Manager          npm
Output                   hybrid
Adapter                  @astrojs/cloudflare
Integrations             none

Describe the Bug

When running Astro Server Islands behind Cloudflare Tunnels, Astro while loops itself and halts rendering thus crashing the tab. This does not happen when running on Pages in production or using Wrangler over localhost.

Please run the linked example (astro start, you need --host) then using cloudflared run cloudflared tunnel --url http://localhost:8788.

I've reproduced this with cloudflared version 2024.6.1 and 2024.8.2.

Astro inserts a script to "swap" the placeholder with the server rendered component. The client gets stuck swapping and runs in a loop until the browser halts execution.

// Swap!
while(script.previousSibling?.nodeType !== 8 &&
  script.previousSibling?.data !== 'server-island-start') {
  script.previousSibling?.remove();
}
script.previousSibling?.remove();

image

What's the expected result?

This shouldn't happen :)

Link to Minimal Reproducible Example

https://github.com/DeJayDev/astro-server-islands-cf-bug

Participation

tedevra commented 3 months ago

heyho, i have a similar (if not the same) issue with my cloudflare adapter and a dynamic catch all page route: https://github.com/withastro/astro/issues/11597

DeJayDev commented 3 months ago

Correction, this does happen in production builds, just took me another go to figure out what I was missing here.

Currently, this makes using Server Islands in hybrid projects impossible when hosted on Cloudflare. At least for me ☹️

Here's a live example: Access through pages.dev to view intended behavior: https://astro-server-islands-cf-bug.pages.dev/ Access through a Custom Domain to view unintended behavior: https://astro-11638.dejaydev.com/

matthewp commented 2 months ago

@alexanderniebuhr think we need special configuration in the Cloudflare adapter for the /_server-islands route maybe? That's the only thing I can think that might cause this.

alexanderniebuhr commented 2 months ago

I don't think there is a custom configuration needed. https://astro-tips.dev/external-resources/ is also hosted on Cloudflare, hybrid output and it uses a custom domain. Do we have the specifics of this error? Does it happen for a special case / set-up?

alexanderniebuhr commented 2 months ago

I tried to deploy the provided reproduction and I think I found some relevant information, it only fails if the custom domain includes a subdomain:

Works: https://islands-test.pages.dev/ Works: https://islands-test.nbhr.io/ Breaks: https://astrovault.dev/

I'm still trying to understand if that is an Cloudflare behavior, an Adapter issue, or an core issue, but maybe that helps others with debugging.. Does the same happen for other adapters if used with a subdomain?

alexanderniebuhr commented 2 months ago

I can't observe any issue in the deployment logs though, all domains look the same. The request even returns 200 in the network console, it just seems to not replace the island 🤔

alexanderniebuhr@MacBookPro ~/D/t/astro-server-islands-cf-bug (main)> wrangler pages deployment tail

 ⛅️ wrangler 3.70.0
-------------------

Select a project:
❯ islands-test
  astro-env-deploy
  craftlions
  starlight-ssr
  studio
  env-var
  astropix-docs
  cf-headers-issue
  astro-install-test
  terra-astralis
  orbinapes
  smoggy-spiral
  blog
No deployment specified. Using latest deployment for production environment.
Connected to deployment 60b0f68c-2a7c-4d9d-802a-540c18f7ea7d, waiting for logs...

POST https://islands-test.pages.dev/_server-islands/Location - Ok @ 8/10/2024, 7:03:14 AM
POST https://islands-test.nbhr.io/_server-islands/Location - Ok @ 8/10/2024, 7:03:23 AM
POST https://astrovault.dev/_server-islands/Location - Ok @ 8/10/2024, 7:03:32 AM
DeJayDev commented 2 months ago

I think it's about how you're accessing the page through Cloudflare network. pages.dev is direct, you cannot configure the proxy settings or modify anything its them on both sides. Using a custom domain introduces the Cloudflare Proxy and the resolution of the CNAME. The subdomain thing is a great callout, as Cloudflare does CNAME flattening at the root. Maybe the flattening process makes it similar to accessing the page as though it was pages.dev proper? You can see that here:

Access through pages.dev to view intended behavior: https://astro-server-islands-cf-bug.pages.dev/ Access through a Custom Domain to view unintended behavior: https://astro-11638.dejaydev.com/

The section of code the debugger places you in when you have your browser jump to the last call is in the section to remove the script that grabs the island again. Is this like Rocket Loader or something?

ItsRauf commented 2 months ago

This is an issue with Cloudflare's "Auto Minify". This feature performs minification on the HTML before being sent to the browser which includes removing HTML comments.

Removing HTML comments gets rid of <!--server-island-start--> which Astro's server islands feature looks for here: https://github.com/withastro/astro/blob/8118120e91ba9497f98f6a95511b9d401ffd7299/packages/astro/src/runtime/server/render/server-islands.ts#L84-L87 Node type 8 is specified as an HTML comment and Cloudflare removing all of those causes this code to hang the client as seen in this issue

Fixing this issue is as simple as disabling the Cloudflare feature which according to Cloudflare should be deprecated but is in fact still functioning. Cloudflare Dashboard screenshot of Auto Minify deprecation

gtrabanco commented 2 months ago

It's 14 August, the option has already been deactivated but it keeps with same issue. I hope PR #11692 will solve it

Related info:

Astro                    v4.13.3
Node                     v20.16.0
System                   macOS (arm64)
Package Manager          bun
Output                   hybrid
Adapter                  @astrojs/vercel/serverless
Integrations             @astrojs/sitemap
                         @astrojs/react
tedevra commented 2 months ago

@gtrabanco

Even though the "Minify" config is not available in cloudflare web dashboard anymore, it still exists!

You can check the current config via the cloudflare API: GET https://api.cloudflare.com/client/v4/zones/##REPLACE-WITH-ZONE-ID##/settings/minify

and with a PATCH to that URL including the following body you can disable it: { "value":{ "css": "off", "html": "off", "js": "off" } }

matthewp commented 2 months ago

@alexanderniebuhr Is there anything that can be done on the adapter side to disable the minification?

alexanderniebuhr commented 2 months ago

No, in fact this isn't even a configuration option for Cloudflare Pages. It is a configuration option for the DNS Zone. I'll forward it to my Cloudflare contact.

matthewp commented 2 months ago

@alexanderniebuhr Thank you. Stripping comments is extremely unsafe, Astro has built-in minification on by default and it doesn't do this. Many frameworks use comments as markers in this way so I would imagine this behavior breaks others as well.

alexanderniebuhr commented 2 months ago

Just got confirmation from Cloudflare on this. They will disable the "Auto Minify" feature globally, until that happens the only workaround is using the API for configuration. There is no ETA for the global change, at least a couple weeks is what I hear.

gtrabanco commented 2 months ago

@gtrabanco

Even though the "Minify" config is not available in cloudflare web dashboard anymore, it still exists!

You can check the current config via the cloudflare API: GET https://api.cloudflare.com/client/v4/zones/##REPLACE-WITH-ZONE-ID##/settings/minify

and with a PATCH to that URL including the following body you can disable it: { "value":{ "css": "off", "html": "off", "js": "off" } }

Thanks a lot! I'll try =)

Edit: In case is useful to any other one:

disable_minify_cf() {
  local -r zone=$1

  curl -X PATCH "https://api.cloudflare.com/client/v4/zones/${zone}/settings/minify" -H "Authorization: Bearer ${CLOUDFLARE_API_KEY}" -H "Content-Type:application/json" --data '{"value":{"css":"off","html":"off","js":"off" }}'
}