kit icon indicating copy to clipboard operation
kit copied to clipboard

@sveltejs/adapter-cloudflare v2.2.0 always triggers hooks on build

Open danawoodman opened this issue 2 years ago • 22 comments
trafficstars

Describe the bug

Took a while to track down the cause of this, but I recently updated to adapter-cloudflare 2.2.0 and my builds started to break.

It appears a change from 2.1.0 to 2.2.0 now always triggers hooks.server.js to run, even if there are no configured prerenderable routes, which then triggers my auth middleware which redirects the user to another domain if they have no authentication cookie set (see hooks.server.js in the repro repo).

v2.1.0 works fine and I can build without the hook getting triggered by the adapter, but now in 2.2.0 it always triggers and thus breaks the build. AFAIK this means that there is no way to build for CloudFlare anymore if you have such an auth hook in place.

Reproduction

https://stackblitz.com/edit/sveltejs-kit-template-default-g2rzrj?file=src/hooks.server.js

Run npm run build to see the issue. Downgrade to 2.1.0 to see it go away

Logs

> Using @sveltejs/adapter-cloudflare
file:///Users/my-app/node_modules/@sveltejs/kit/src/core/postbuild/fallback.js:53
        throw new Error(`Could not create a fallback page — failed with status ${response.status}`);
              ^

Error: Could not create a fallback page — failed with status 302
    at generate_fallback (file:///Users/my-app/node_modules/@sveltejs/kit/src/core/postbuild/fallback.js:53:8)
    at async process.<anonymous> (file:///Users/my-app/node_modules/@sveltejs/kit/src/utils/fork.js:25:17)
error during build:
Error: Failed with code 1
    at ChildProcess.<anonymous> (file:///Users/my-app/node_modules/@sveltejs/kit/src/utils/fork.js:68:13)
    at ChildProcess.emit (node:events:513:28)
    at ChildProcess.emit (node:domain:489:12)
    at Process.ChildProcess._handle.onexit (node:internal/child_process:293:12)

System Info

System:
    OS: macOS 13.2.1
    CPU: (8) arm64 Apple M2
    Memory: 740.67 MB / 24.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 16.19.0 - ~/Library/Caches/fnm_multishells/80853_1678404247439/bin/node
    npm: 8.19.3 - ~/Library/Caches/fnm_multishells/80853_1678404247439/bin/npm
  Browsers:
    Brave Browser: 106.1.44.112
    Chrome: 111.0.5563.64
    Firefox: 103.0
    Safari: 16.3
  npmPackages:
    @sveltejs/adapter-auto: 2.0.0 => 2.0.0 
    @sveltejs/adapter-cloudflare: 2.1.0 => 2.1.0 
    @sveltejs/kit: 1.11.0 => 1.11.0 
    svelte: 3.55.1 => 3.55.1 
    vite: 4.1.4 => 4.1.4

Severity

breaks production builds, no work around except downgrading

Additional Information

No response

danawoodman avatar Mar 10 '23 00:03 danawoodman

I'm guessing this is the result of #9294.

I'm not sure what it would make sense to do about this. Can you check building from $app/environment and not apply any auth middleware in that case?

Conduitry avatar Mar 10 '23 01:03 Conduitry

Same issue here. I am now checking with the building value, but this is a weird work around...

stefandevo avatar Mar 10 '23 12:03 stefandevo

@Conduitry yeah, I had a feeling it was from #9294 as well based on it being the main change from 2.1.0 to 2.2.0.

I could probably do that, but as @stefandevo says, it is a weird workaround/hack. I would assume that Kit would not run my hooks if I have no pre-rendering configured anywhere. For now I'll stay on 2.1.0 but if we decide to update I'll do the suggestion you offer, thanks 👍

danawoodman avatar Mar 10 '23 19:03 danawoodman

I have also encountered this issue, with auth middleware as well.

paulgibbs avatar Mar 17 '23 13:03 paulgibbs

Same issue here.

bobby avatar Mar 29 '23 20:03 bobby

Can you check building from $app/environment and not apply any auth middleware in that case?

This seems like the best solution for now, unless an option is added to disable generation of a 404.html from the cloudflare adapter (which rich pushed back on before when I tried to add it so i'm not sure if that would happen. Although then neither of us could think of a case where it would make sense to be disabled, I wanted it "just in case" something like this happened)


I would say the solution on the svelte side could be:

  • Add an option to disable 404.html generation

and/or

  • Document the generation of 404.html on the docs

ajgeiss0702 avatar Mar 31 '23 19:03 ajgeiss0702

Instead of always generating a fallback page, I suggest that the cloudflare adapter copies over a static 404.html to the /_app/ or / root directory. The <head> of this file should include <meta http-equiv="Refresh" content="0; url='/'" /> to redirect to the root page only if visited through a browser.

The benefits of this approach are:

  • avoids running hooks during build.
  • returns 404 when requesting missing assets in /_app/* or routes excluded by a wildcard.

    the original issue was that it would return /index.html and a 200 status instead of 404.

  • redirects users to / when navigating to these non-existent excluded routes.

    excluded routes in _routes.json bypass the Cloudflare worker and returns the page html file or nearest index.html / 404.html.

Although this solution still doesn't really sit right with me, I feel that it is a good compromise.

teemingc avatar Apr 01 '23 16:04 teemingc

Instead of always generating a fallback page, I suggest that the cloudflare adapter copies over a static 404.html to the /_app/ or / root directory. The <head> of this file should include <meta http-equiv="Refresh" content="0; url='/'" /> to redirect to the root page only if visited through a browser.

The benefits of this approach are:

* avoids running hooks during build.

* returns 404 when requesting missing assets in `/_app/*` or routes excluded by a wildcard.
  > the original issue was that it would return `/index.html` and a 200 status instead of 404.

* redirects users to `/` when navigating to these non-existent excluded routes.
  > excluded routes in `_routes.json` bypass the Cloudflare worker and returns the page html file or nearest `index.html` / `404.html`.

Although this solution still doesn't really sit right with me, I feel that it is a good compromise.

Just redirecting to the root doesn't seem like a good solution at all to me. Instead of getting a 404 page, users would just get redirected to the root page, which almost defeats the purpose of a 404 page at all.

I feel the standard that works for most sites, and matches the functionality of other adapters is better than having a different behavior that doesn't make a ton of sense for most sites. It should either be an option, or just something that should be worked around (as the workaround is pretty simple. It should probably still be documented though)

ajgeiss0702 avatar Apr 01 '23 23:04 ajgeiss0702

I feel the standard that works for most sites, and matches the functionality of other adapters is better than having a different behavior that doesn't make a ton of sense for most sites.

Perhaps the fallback page should also be opt-in and print a warning if it's not configured? Rather than having it automatically generated and confusing users why their build idea failing.

Or perhaps we should document and warn users about their hooks whenever the fallback page generator fails (this would benefit the static adapter as well).

In the end, it's difficult to get a behaviour consistent with other adapters until cloudflare removes the 100 rule limit of the _routes.json file.

teemingc avatar Apr 02 '23 04:04 teemingc

I've had to revisit this issue a couple different times since this hasn't been fixed and I keep forgetting why my sveltekit apps won't build in Pages but will locally 😞 The answer isn't explicitly here in code yet, so sharing what worked for me based on the above suggestions:

src/hooks.server.ts

// other imports here
import { building } from '$app/environment'

export async function handle({ event, resolve }) {
  if (building) {
    const response = await resolve(event)
    return response // bailing here allows the 404 page to build
  }

// rest of the complex server code down here that was causing 404 page not to build

dbradleyfl avatar Apr 25 '23 03:04 dbradleyfl

Is there a temporary work around for this for Cloudflare Pages? I've tried downgrading to @sveltejs/[email protected] and using @dbradleyfl method if (building) await resolve(event), but I get the same "Error: Could not create a fallback page — failed with status 303" when deploying to Cloudflare Pages.

figuerom16 avatar May 13 '23 18:05 figuerom16

Is there a temporary work around for this for Cloudflare Pages? I've tried downgrading to @sveltejs/[email protected] and using @dbradleyfl method if (building) await resolve(event), but I get the same "Error: Could not create a fallback page — failed with status 303" when deploying to Cloudflare Pages.

It sounds like your site is still processing the hooks during build. Make sure that if building stops execution if it is building

ajgeiss0702 avatar May 13 '23 18:05 ajgeiss0702

Yup sounds like you're not returning the result of await resolve(event). The return is the important part @figuerom16

dbradleyfl avatar May 13 '23 18:05 dbradleyfl

@ajgeiss0702 @dbradleyfl Doh! You're correct. I forgot the return. Works with the newest @sveltejs/[email protected]. Thank you much! if (building) return await resolve(event)

figuerom16 avatar May 13 '23 18:05 figuerom16

I've had to revisit this issue a couple different times since this hasn't been fixed and I keep forgetting why my sveltekit apps won't build in Pages but will locally 😞 The answer isn't explicitly here in code yet, so sharing what worked for me based on the above suggestions:

src/hooks.server.ts

// other imports here
import { building } from '$app/environment'

export async function handle({ event, resolve }) {
  if (building) {
    const response = await resolve(event)
    return response // bailing here allows the 404 page to build
  }

// rest of the complex server code down here that was causing 404 page not to build

You're the GOAT, for anyone having this specific problem - this is the issue. Cloudflare Pages build failing due to auth redirect on base path.

Figured i'd write it so this ranks on Google! Saved me time by putting me on to building brother - thank you!

RowanAldean avatar Sep 11 '23 22:09 RowanAldean

  if (building) {
    const response = await resolve(event)
    return response // bailing here allows the 404 page to build
  }

How do you add this if you're using sequence along with SvelteKitAuth? Doesn't seem to fix my error by adding resolve(event) in the handle that handles my routes in my sequence.

Update https://github.com/sveltejs/kit/issues/7899 Found a solution to conditionally render sequence depending if we're building or not.

socketopp avatar Jan 03 '24 14:01 socketopp

  if (building) {
    const response = await resolve(event)
    return response // bailing here allows the 404 page to build
  }

How do you add this if you're using sequence along with SvelteKitAuth? Doesn't seem to fix my error by adding resolve(event) in the handle that handles my routes in my sequence.

Update #7899 Found a solution to conditionally render sequence depending if we're building or not.

Do you have your source code for this?

givenergy-dexter avatar Jan 08 '24 11:01 givenergy-dexter

  if (building) {
    const response = await resolve(event)
    return response // bailing here allows the 404 page to build
  }

How do you add this if you're using sequence along with SvelteKitAuth? Doesn't seem to fix my error by adding resolve(event) in the handle that handles my routes in my sequence. Update #7899 Found a solution to conditionally render sequence depending if we're building or not.

Do you have your source code for this?

export const handle = building
    ? sequence()
    : sequence(...);

socketopp avatar Jan 08 '24 11:01 socketopp

  if (building) {
    const response = await resolve(event)
    return response // bailing here allows the 404 page to build
  }

How do you add this if you're using sequence along with SvelteKitAuth? Doesn't seem to fix my error by adding resolve(event) in the handle that handles my routes in my sequence. Update #7899 Found a solution to conditionally render sequence depending if we're building or not.

Do you have your source code for this?

export const handle = building
    ? sequence()
    : sequence(...);

Yeah, nice one, I ended up doing:

import { sequence } from "@sveltejs/kit/hooks";

import { building } from '$app/environment'

function optional (fn) {
  if (building) {
    return ({ event, resolve }) => resolve(event)
  }

  return fn
}

export const handle = optional(sequence())

givenergy-dexter avatar Jan 08 '24 11:01 givenergy-dexter

  if (building) {
    const response = await resolve(event)
    return response // bailing here allows the 404 page to build
  }

How do you add this if you're using sequence along with SvelteKitAuth? Doesn't seem to fix my error by adding resolve(event) in the handle that handles my routes in my sequence. Update #7899 Found a solution to conditionally render sequence depending if we're building or not.

Do you have your source code for this?

export const handle = building
    ? sequence()
    : sequence(...);

Yeah, nice one, I ended up doing:

import { sequence } from "@sveltejs/kit/hooks";

import { building } from '$app/environment'

function optional (fn) {
  if (building) {
    return ({ event, resolve }) => resolve(event)
  }

  return fn
}

export const handle = optional(sequence())

Do you see any benefits from doing one to another?

socketopp avatar Jan 09 '24 09:01 socketopp

Do you see any benefits from doing one to another?

No, just a bit more readable I suppose.

givenergy-dexter avatar Jan 10 '24 13:01 givenergy-dexter

Can someone sanity check https://github.com/sveltejs/kit/pull/11596 and tell me if it seems like a good fix for this?

Rich-Harris avatar Jan 10 '24 22:01 Rich-Harris