Skip to content

Conversation

@The-Noah
Copy link
Contributor

@The-Noah The-Noah commented Oct 9, 2022

Fixes #6739

Ever since #6565 calling fetch in a load function fails on Cloudflare (Pages+Workers). This solution checks for the unique cf object that Cloudflare sets to bypass properties Cloudflare does not provide, assuming reasonable defaults.

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

  • If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. All changesets should be patch until SvelteKit 1.0

@changeset-bot
Copy link

changeset-bot bot commented Oct 9, 2022

🦋 Changeset detected

Latest commit: adaffd4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@sveltejs/kit Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@Conduitry
Copy link
Member

We very much would like to avoid having any code specific to a particular hosting platform inside SvelteKit core. I don't have a suggestion off the top of my head for what existing adapter API would work for providing the equivalent functionality. I do think it would be better to make the API a bit more flexible though so the Cloudflare adapter can do it itself, rather than having Cloudflare-specific code life in the core framework and thus in all apps for any platform.

@The-Noah
Copy link
Contributor Author

The-Noah commented Oct 9, 2022

I definitely agree that keeping platform specific code outside of the core is good, and that the Cloudflare adapter taking care of it would be the best solution. However, this issue cripples deploying to Cloudflare, making it for all intents and purposes impossible on modern versions of SvelteKit. Would it be possible to have this patch until a better solution can be made?

@dominikg
Copy link
Member

It can be worked around on cloudflare workers when deployed via wrangler, see #6739 (comment) and #6739 (comment). Not sure how that could be applied to cloudflare pages.
One option could be to use pnpm patch to update your local sveltekit until this situation is solved by a change in cloudflare or sveltekit.

The proposed fix here not only adds platform specific code to core but also makes assumptions about the values provided, which won't work for everyone.

A more generic solution could take a clue from the handleFetch workaround, offering a way for adapters to provide runtime bits to kit. But it feels weird that cloudflare throws this error. Did you raise it with them too?

@The-Noah
Copy link
Contributor Author

I've tried both of those workarounds but neither work when deployed to Pages (with wrangler). I am currently using my patched version as a workaround, thank you for the suggestion.

I haven't raised this with Cloudflare, but I will do that now.

@zlepper
Copy link

zlepper commented Oct 12, 2022

This is sadly a "documented" limit in Cloudflare: https://community.cloudflare.com/t/how-to-get-request-mode-from-cloudflare-worker/421996 (I really hate they way they implemented it rather than just ignoring the property entirely), so I doubt they are going to solve anything on their side right now :(

Rich-Harris added a commit that referenced this pull request Oct 31, 2022
Rich-Harris added a commit that referenced this pull request Oct 31, 2022
* [fix] fetch erroring on Cloudflare

Fixes #6739

* alternative to #7192

Co-authored-by: The Noah <[email protected]>
@Rich-Harris
Copy link
Member

Closed in favour of #7453 (which includes this branch) — hopefully it's sufficient 🤞

@Rich-Harris
Copy link
Member

(and thank you!)

@allozaur
Copy link
Contributor

allozaur commented Nov 1, 2022

@Rich-Harris @dummdidumm i just tested this with newest (1.0.0-next.531) version of SvelteKit and the problem seems to persist on my end.

Reproduction repo:

https://github.com/allozaur/sveltekit-server-endpoint-test/tree/main/src/routes

Preview in Cloudflare Pages:

https://sveltekit-server-endpoint-test.pages.dev/pikachu

Screenshots

localhost

Zrzut ekranu 2022-11-1 o 00 16 06

Cloudflare Pages

Zrzut ekranu 2022-11-1 o 01 36 37

@Deebster
Copy link

Deebster commented Nov 1, 2022

@allozaur It's working for me using both wrangler publish and pages (src).

@allozaur
Copy link
Contributor

allozaur commented Nov 1, 2022

@Deebster your example doesn't use +server.js/+server.ts route, but a +page.js/+page.ts. In my case i am:

  1. Fetching data from a 3rd party API via src/routes/api/pokemon/[page]/+server.js route (https://sveltekit-server-endpoint-test.pages.dev/api/pokemon/pikachu)
import { error, json } from "@sveltejs/kit";

export async function GET(/** @type{ { params: any } } */ { params }) {
  try {
    const result = await fetch(`https://pokeapi.co/api/v2/pokemon/${params.name}`)
    const data = await result.json();

    return json(data);

  } catch (e) {
    throw error(404);
  }
}
  1. Loading the data via src/routes/[name]/+page.js
import { error } from "@sveltejs/kit";

export async function load(/** @type{ { fetch: any, params: any } } */ { fetch, params }) {
  const req = await fetch(`/api/pokemon/${params.name}`, { mode: "cors" });
  const res = await req.json();


  if (res) {
    return {
      pokemon: res,
    }
  }

  throw error(404);
}
  1. Displaying loaded data in src/routes/[name]/+page.svelte ((https://sveltekit-server-endpoint-test.pages.dev/pikachu)
<script>
	/**
	 * @type {any}
	 */
	export let data;

	$: console.log(1, data);
</script>

<h1>{data.pokemon.name}</h1>
<img src={data.pokemon.sprites.front_default} alt={data.pokemon.name} />

What's really strange is that when you access the data from the +server.js route via browser it works totally fine, but when +page.js wants to access it — then it does not work...

@Deebster
Copy link

Deebster commented Nov 1, 2022

Hmm, I am assuming the fix works because I've set export const csr = false; so it only runs server-side, even though it's not a real *server.[tj]s file.

I'm wondering if you could be hitting a separate problem: there's a limitation with Cloudflare whereby worker-to-worker requests require setting up a service binding*.

This assumes that SvelteKit does a full request instead of internally routing the call - I thought it wouldn't do a real request but I can't see that in the docs. Also, this doesn't explain why your previous fix worked, but I'll hit send anyway in case this is helpful.

* In my actual project, I have a separate API worker on a subdomain and needed to use a handleFetch() in hooks.server.ts to use the fetch on the binding (but only once I stopped using Pages so I could apply my previous hack for this issue!)

@allozaur
Copy link
Contributor

allozaur commented Nov 1, 2022

@Deebster @Rich-Harris @dummdidumm i managed to get this working

@dummdidumm
Copy link
Member

what was the problem / what was the fix?

@didierfranc
Copy link

I supposed it's been fixed here 9f57ff3

@Adrian-MID
Copy link

@Deebster @Rich-Harris @dummdidumm i managed to get this working

Is there any chance you could share the working code with me? I seem to be comprehensively stuck on this same issue. Looking at a working example would be very helpful.

@Vextil
Copy link
Contributor

Vextil commented Feb 3, 2023

@Adrian-MID Not sure if this helps but we were having issues when copying a request to replace the URL to make internal requests during SSR.

Initially we had this, which did not work for Cloudflare Pages because Request.mode is undefined and SvelteKit attempts to call it internally when making the fetch request:

export const handleFetch = (({ event, request, fetch }) => {
  if (request.url.startsWith(PUBLIC_URL)) {
    if (INTERNAL_URL) {
        request = new Request(
        request.url.replace(PUBLIC_URL, INTERNAL_URL),
        request
      );
    }
  }
  return fetch(request);
}) satisfies HandleFetch;

So we ended up changing it to this and it works fine because it avoids the Request.mode call:

export const handleFetch = (({ event, request, fetch }) => {
  if (request.url.startsWith(PUBLIC_URL)) {
    if (INTERNAL_URL) {
      return fetch(request.url.replace(PUBLIC_URL, INTERNAL_URL), {
        method: request.method,
        body: request.body,
        mode: 'cors',
        credentials: 'include',
        headers: request.headers,
      });
    }
  }
  return fetch(request);
}) satisfies HandleFetch;

It's a hack but it works, and the reason it does is because SvelteKit does the following when attempting to get the request mode:

fetch: async (info2, init3) => {
    ...
    mode = (info2 instanceof Request ? info2.mode : init3?.mode) ?? "cors";

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error when doing external fetch on Cloudflare Workers