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

Cloudflare Workers (Beta Assets). Broken Caching _headers #417

Open
1 task done
predaytor opened this issue Oct 6, 2024 · 4 comments
Open
1 task done

Cloudflare Workers (Beta Assets). Broken Caching _headers #417

predaytor opened this issue Oct 6, 2024 · 4 comments
Labels
- P2: has workaround Bug, but has workaround (priority) upstream wontfix This will not be worked on

Comments

@predaytor
Copy link

predaytor commented Oct 6, 2024

Astro Info

Astro                    v4.15.11
Node                     v22.8.0
System                   macOS (arm64)
Package Manager          npm
Output                   server
Adapter                  @astrojs/cloudflare
Integrations             astro-robots-txts

If this issue only occurs in one browser, which browser is a problem?

No response

Describe the Bug

Using the experimental deployment of Cloudflare Workers for Astro, caching does not seem to work for paths specified in _headers.

Response headers for static assets: cache-control: public, max-age=0, must-revalidate.

https://my-astro-app.thepredaytor.workers.dev/

public/_routes.json:

{
  "version": 1,
  "include": [
    "/*"
  ],
  "exclude": [
    "/_astro/*",
    "/.assetsignore",
    "/favicon.svg"
  ]
}

public/_headers:

/_astro/*
  Cache-Control: public, max-age=31557600, immutable

/favicon.ico
  Cache-Control: public, max-age=3600, immutable

/favicon.svg
  Cache-Control: public, max-age=3600, immutable

wrangler.toml:

#:schema node_modules/wrangler/config-schema.json
name = "my-astro-app"
compatibility_date = "2024-10-04"
compatibility_flags = ["nodejs_compat_v2"]
main = "./dist/_worker.js"
assets = { directory = "./dist", binding = "ASSETS" }

# Workers Logs
# Docs: https://developers.cloudflare.com/workers/observability/logs/workers-logs/
# Configuration: https://developers.cloudflare.com/workers/observability/logs/workers-logs/#enable-workers-logs
[observability]
enabled = true

[vars]
MY_VARIABLE = "test"

What's the expected result?

The response headers must represent those defined in the _headers file.

Link to Minimal Reproducible Example

https://stackblitz.com/github/predaytor/astro-cloudflare

Participation

  • I am willing to submit a pull request for this issue.
@github-actions github-actions bot added the needs triage Issue needs to be triaged label Oct 6, 2024
@florian-lefebvre florian-lefebvre transferred this issue from withastro/astro Oct 7, 2024
@alexanderniebuhr
Copy link
Member

@predaytor I need to confirm with the Cloudflare Team first, before I can give you an answer, but I'm not sure if Worker Assets support the _headers file in the same level as Pages do.

@petebacondarwin
Copy link

Yes, currently Workers + Assets does not read any of _headers, _redirects nor _routes.json.
The basic configuration of Workers + Assets appears to avoid the need for _routes.json in almost every case so far.

We are deciding what to do about the other two. Both can be handled in different ways with Workers.

For example it should be possible to setup Cloudflare Snippets in front of a Workers + Assets worker that can do things like redirects and modifying headers based on requests.

@alexanderniebuhr alexanderniebuhr added wontfix This will not be worked on upstream - P2: has workaround Bug, but has workaround (priority) and removed needs triage Issue needs to be triaged labels Oct 10, 2024
@predaytor
Copy link
Author

predaytor commented Dec 25, 2024

Still trying to find a solution for caching on Workers (Assets Beta). Either we can use Cache Rules, or spin up an additional proxy Service Worker in front of the main worker.

I also modify the response for /_astro/* files by adding a special cache-control, assuming that it does not affect CDN caching, according to docs:

Modifying cache-control, CDN-Cache-Control, or Cloudflare-CDN-Cache-Control headers will not change the way Cloudflare caches an object. Instead, you should create a Cache Rule

Applying Cache Rules doesn't work for unknown reason, the CF-Cache-Status headers are absent. (trace log attached)

{
  "trace": [
    {
      "step_name": "http_request_cache_settings",
      "type": "phase",
      "matched": true,
      "public_name": "Cache Rules",
      "trace": [
        {
          "step_name": "a45633542c1d4a3a8e19b7df0cf8cacd",
          "type": "ruleset",
          "matched": true,
          "name": "default",
          "kind": "zone",
          "trace": [
            {
              "step_name": "df03fb9a63784d08aba0560339f9593b",
              "type": "rule",
              "matched": true,
              "action_parameter": {
                "cache": true,
                "edge_ttl": { "mode": "respect_origin" },
                "browser_ttl": { "mode": "respect_origin" },
                "respect_strong_etags": true,
                "cache_key": {
                  "ignore_query_strings_order": true,
                  "cache_deception_armor": true
                },
                "origin_error_page_passthru": true
              },
              "expression": "(http.request.uri.path wildcard \"/_astro/*\")",
              "description": "Cache Everything [Template]",
              "action": "set_cache_settings"
            }
          ]
        }
      ],
      "zoneName": "predaytor.com"
    },
  ],
}
Знімок екрана 2024-12-25 о 19 43 42

@predaytor
Copy link
Author

The only working solution is to deploy a proxy (via service bindings) on top of the main worker and manually modify the response of the assets that need to be cached:

export default {
	async fetch(request, env, ctx) {
		try {
			return await handleRequest(request, env, ctx);
		} catch (ex) {
			console.error(ex);
			return new Response('Internal server error', { status: 500 });
		}
	},
} satisfies ExportedHandler<Env>;

async function handleRequest(request: Request, env: Env, ctx: ExecutionContext): Promise<Response> {
	let response = await env.app.fetch(request.clone());
	response = cacheStaticAssets(request, response);
	return response;
}

function cacheStaticAssets(request: Request, response: Response): Response {
	const cacheableExtensions = new Set([
		"7z", "avi", "avif", "apk", "bin", "bmp", "bz2", "class", "css", "csv",
		"doc", "docx", "dmg", "ejs", "eot", "eps", "exe", "flac", "gif", "gz",
		"ico", "iso", "jar", "jpg", "jpeg", "js", "mid", "midi", "mkv", "mp3",
		"mp4", "ogg", "otf", "pdf", "pict", "pls", "png", "ppt", "pptx", "ps",
		"rar", "svg", "svgz", "swf", "tar", "tif", "tiff", "ttf", "webm", "webp",
		"woff", "woff2", "xls", "xlsx", "zip", "zst"
	]);

	const url = new URL(request.url);
	const extension = url.pathname.split('.').pop();

	const newResponse = new Response(response.body, response);

	if (extension && cacheableExtensions.has(extension)) {
		newResponse.headers.set(
			'Cache-Control',
			'public, max-age=31536000, immutable',
		);
	}

	return newResponse;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P2: has workaround Bug, but has workaround (priority) upstream wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

3 participants