ITADN

`cf()` is missing four things `handle()` does: cookies/sessions, the CDN `no-store` default, the assets fallback, the prerender phase

#17600Openiseraph-dev 创建于 25 天前
- P4: importanttriage: needs triagepkg: cloudflare
I
iseraph-devcommented
### Astro Info ``` Astro v7.1.6 Vite v8.2.0 Node v24.19.0 System Linux (x64) Package Manager pnpm Output server Adapter @astrojs/cloudflare (v14.1.7) Integrations none ``` ### Describe the Bug **This issue is blocked by #17591.** Custom worker entrypoints 500 on every request today (`FetchState(request) called on a request without an attached app`), so none of what follows is reachable on a released build, and the repro's `src/worker.ts` carries that issue's two-line workaround just to boot. Triage this one after it. The two fixes don't overlap, though: the candidate fix on #17591 adds `createFetchState()` and leaves `cf()` untouched, which I checked against the pkg.pr.new build over there. I moved a site from the default `main` to the custom worker entrypoint the docs describe (`astro/fetch` + `cf()`; the Hono variant behaves the same). Routing kept working. Four other things stopped, none of them loudly. The adapter has two request entrypoints: - `handle()`, behind `@astrojs/cloudflare/entrypoints/server`, for the default `main` - `cf()`, from `@astrojs/cloudflare/fetch` and `@astrojs/cloudflare/hono`, for a custom one Swapping one for the other should only change who calls Astro. Here's what actually changed. One project, `main` the only variable, production build served by `astro preview` on workerd (repro at the bottom): | | default `main` | custom `main` (`cf()`) | |---|---|---| | `set-cookie` on a page that sets one | `repro=1; Path=/` and `astro-session=...` | not sent | | `/session`, two hits with a cookie jar | `count: 1` then `count: 2` | `count: 1` both times | | `cloudflare-cdn-cache-control`, cache provider on, no `routeRules` | `no-store` | not sent | | `/stray.txt` with `assets.run_worker_first: true` | 200, serves the file | 404 page | | `astro build` with one `prerender = true` page | prerenders `/static/index.html` | fails, exit 1 | Nothing warns about any of it and it all type-checks. I'm filing the four together because they have one cause. Since `cf()` shipped in 13.6.0, `utils/handler.ts` has had four functional changes: #16335 (CDN cache providers), #17049 (prerender errors), #17481 (immutable headers), #16194 (build-time image optimization). `src/fetch.ts` has had one change in its life (#16968, lazy init) and `src/hono.ts` one (#17594, types only). Three of the gaps below are steps `handle()` picked up and `cf()` never did. The fourth is a step `cf()` did copy, whose precondition stopped holding in the new shape. Code is quoted from `main` at `caace2ec6e`. 14.1.7 ships an earlier version of the same steps. #### 1. Cookies are dropped, and sessions with them `handle()` collects them off the response itself: ```ts // Collect cookies before any rebuild: `setCookieHeaders` looks the // response up by identity, so it must see the original object. const setCookieHeaders = app.setCookieHeaders ? [...app.setCookieHeaders(response)] : []; ``` `cf()` has nothing for this, and it doesn't set `renderOptions.addCookieHeader` either, which is Astro's own way of doing the same thing (with it set, `prepareResponse()` appends the cookies). It already writes two of the three render options on that object: ```ts state.renderOptions.waitUntil = ctx.waitUntil.bind(ctx); state.renderOptions.prerenderedErrorPageFetch = createErrorPageFetch(env); ``` The session cookie is an ordinary cookie, so it goes down with the rest and every request starts a new session. `cf()` injects the SESSION KV binding itself (`injectSessionBinding`), so the storage half works and only the cookie half is missing, which is what made this hard to spot. Setting `state.renderOptions.addCookieHeader = true` before `astro(state)` in the repro brings back both cookies and the 1 → 2 count. #### 2. No `no-store` default for the CDN cache `handle()` ends with: ```ts // When the Cloudflare cache provider is configured, default uncached // responses to `no-store` so opting in to route caching never // accidentally caches a route that didn't set any cache intent. // Cloudflare's Worker cache otherwise caches all GET responses for // up to 2 hours by default. const needsNoStoreDefault = cacheProviderEnabled && !response.headers.has('Cloudflare-CDN-Cache-Control'); ``` That comment (from #17481) describes a custom entrypoint just as well as the default one. A route that set no cache intent can sit in the Worker cache for up to two hours. `cf()` doesn't read `cacheProviderEnabled`. It could: `utils/cf.ts`, which `fetch.ts` imports for `injectSessionBinding`, imports `sessionKVBindingName` from `virtual:astro-cloudflare:config`, so the module is already in the graph. The dates fit an oversight rather than a decision. `cf()` shipped in 13.6.0 (2026-05-28), written while advanced routing was still experimental; the cache provider arrived in 14.0.0 (2026-06-22, #16335). #### 3. The assets fallback can't fire Both entrypoints carry the same fallback under the same comment. `handle()`: ```ts if (!routeData) { // NOTE this ASSETS binding path is needed for users who are using `run_worker_first` routing const asset = await fallbackToAssets(request.url, env); if (asset) return asset as CfResponse; } ``` `src/fetch.ts`: ```ts if (!state.routeData) { const asset = await fallbackToAssets(state.request.url, env); if (asset) return asset; } ``` The guards aren't equivalent. `handle()` does the matching itself, `app.match(request)` in production and `app.devMatch()` in dev, and gets `undefined` when nothing matches. `state.routeData` is never `undefined`: `BasePipeline`'s constructor calls `ensure404Route()`, and `FetchState.#resolveRouteData()` falls back to that route. ```ts // Fall back to a 404 route so middleware can still run. if (!this.routeData) { const custom404 = getCustom404Route(pipeline.manifestData); if (custom404 && !custom404.prerender) { this.routeData = custom404; } } ``` `DEFAULT_404_ROUTE` is `prerender: false`, so this holds even in a project with no `404.astro` of its own. `fallbackToAssets()` never runs on this path, and a file sitting in the assets directory without an Astro route behind it is answered by the 404 route. I also measured this directly, before building the repro. With the ASSETS binding wrapped in a Proxy, `cf()` made zero `ASSETS.fetch` calls on `/`, on an unknown path and on a real route, and `state.routeData` was already `/`, `/404` and `/session` before `cf()` ran. #### 4. Nothing answers the build's prerender endpoints `handle()` answers four internal endpoints during the prerender build and installs error propagation for it (trimmed here, casts and the image-config import removed): ```ts const app = createApp(); if (isPrerender) { installPrerenderErrorPropagation(app); } export async function handle(request, env, context) { if (isPrerender) { if (isStaticPathsRequest(request)) return handleStaticPathsRequest(app); if (isPrerenderRequest(request)) return handlePrerenderRequest(app, request); if (isStaticImagesRequest(request)) return handleStaticImagesRequest(); if (isImageTransformRequest(request)) return handleImageTransformRequest(request, { ... }); } ... ``` `cf()` has none of it, and the prerender worker is the user's worker. The adapter builds it from the entry worker's config (`src/index.ts`): ```ts experimental: { prerenderWorker: { config(_, { entryWorkerConfig }) { const { queues, ...restWorkerConfig } = entryWorkerConfig; return { ...restWorkerConfig, name: 'prerender', ... }; }, }, }, ``` `FilteredEntryWorkerConfig` is `Omit<ResolvedAssetsOnlyConfig, 'topLevelName' | 'name'>`, so `main` comes along. Only the Vite environment differs, and that is what `isPrerender` keys off (`vite-plugin-config.ts`: `export const isPrerender = ${this.environment?.name === 'prerender'}`). The flag ends up `true` in a bundle where nothing reads it, and `POST /__astro_static_paths` gets routed to the 404 page: ``` prerendering static routes Failed to get static paths from the Cloudflare prerender server (404: Not Found). <!doctype html> ... <pre>Path: /__astro_static_paths</pre> ... ``` The build exits 1, so a project with even one prerendered route can't build on a custom entrypoint at all. It stays invisible until you have one: `generatePages()` returns early when there is nothing to prerender, which is why the pure-SSR site where I found the other three never hit it. `installPrerenderErrorPropagation` not running is the same gap from the build side. #17049 fixed prerender errors being swallowed, and that fix only exists on the `handle()` path. ### What's the expected result? A custom entrypoint should get the same Cloudflare setup as the default one. Same cookies and sessions, same cache default, same asset behaviour, and a build that can prerender. ### Suggested fix Each of these is small on its own, but four separate patches leave the same trap for the fifth step someone adds to `handle()`. What I'd rather see is both paths running one shared sequence, the way `utils/cf.ts` already shares the individual helpers, with `handle()` calling into it. Taken one at a time: 1. `state.renderOptions.addCookieHeader = true` in `cf()`. One line, on an object it already writes to. 2. Import `cacheProviderEnabled` from `virtual:astro-cloudflare:config` and apply the same default to the response. 3. The guard needs something that says whether the router matched. `app.match()` keeps that and `FetchState.#resolveRouteData()` discards it, so either surface it on `FetchState`, or drop the branch and document that `run_worker_first` needs a fallback in user code. 4. Either answer the four prerender endpoints on the shared path, or point the prerender worker at `@astrojs/cloudflare/entrypoints/server`. `index.ts` already customizes that worker's config, and the prerender worker doesn't need the user's routing, only those endpoints and `app.render`. Whatever 1 and 2 turn into should keep the immutable-header handling from #17481, since a response served out of the Workers Cache API throws when you mutate its headers. The two exports aren't symmetric here. The Hono `cf()` already awaits `next()`, so it has somewhere to put a post-response step. The `astro/fetch` one runs before rendering and returns an asset or `undefined`, so it needs a second export for the response side. Happy to send the PR if you tell me which shape you want. ### Workarounds For 1 and 2 in the `astro/fetch` shape, which is what the repro runs: ```ts const asset = await cf(state, env, ctx); if (asset) return asset; state.renderOptions.addCookieHeader = true; const response = await astro(state); if (!response.headers.has('Cloudflare-CDN-Cache-Control')) { response.headers.set('Cloudflare-CDN-Cache-Control', 'no-store'); } return response; ``` The same thing as Hono middleware registered after `cf()` (`getFetchState` is public API of `astro/hono` since Astro 7.0, #16998): ```ts import { getFetchState } from 'astro/hono'; app.use(async (c, next) => { getFetchState(c).renderOptions.addCookieHeader = true; await next(); if (!c.res.headers.has('Cloudflare-CDN-Cache-Control')) { c.res.headers.set('Cloudflare-CDN-Cache-Control', 'no-store'); } }); ``` Static assets reach neither version, because `cf()` returns before calling `next()`, so they keep their own caching. Both mutate headers without a guard, so if the app can serve responses out of the Workers Cache API, copy the rebuild `handle()` does now. For 4, `prerenderEnvironment: 'node'` avoids the whole thing. The build that fails on workerd succeeds with `cloudflare({ prerenderEnvironment: 'node' })` and a custom `main`. For 3 I don't have one, short of reimplementing the fallback above `astro()`. ### Steps to Reproduce Repro: <https://github.com/iseraph-dev/repro-astro-cf-parity> ``` pnpm install npx wrangler types npx astro build npx astro preview curl -s -D - -o /dev/null http://localhost:4321/ # no set-cookie, no cloudflare-cdn-cache-control curl -s http://localhost:4321/session -c jar.txt # count: 1 curl -s http://localhost:4321/session -b jar.txt # count: 1 again ``` Point `main` at `@astrojs/cloudflare/entrypoints/server`, rebuild, and both headers show up; the second `/session` hit returns `count: 2`. For 3, add `"run_worker_first": true` to the `assets` block, then put a file the manifest doesn't know about into the assets directory after the build: ``` npx astro build && echo "stray file body" > dist/client/stray.txt && npx astro preview curl -s http://localhost:4321/stray.txt ``` Default entrypoint returns `stray file body`. Custom entrypoint returns the 404 page. For 4, add a prerendered page and rebuild: ``` printf -- '---\nexport const prerender = true;\n---\n<html><body><p>prerendered</p></body></html>\n' > src/pages/static.astro npx astro build ``` Default entrypoint prerenders `/static/index.html`. Custom entrypoint fails as quoted above. ### Things I checked that are fine `injectSessionBinding`, `matchStaticAsset`, `locals` / `clientAddress` / `waitUntil` / `prerenderedErrorPageFetch`, dev route matching (`FetchState` resolves it), and the lazy `setGetEnv` / `createApp` in `fetch.ts`, which is deliberate (#16968).
3 条评论