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

Error when doing external fetch on Cloudflare Workers #6739

Closed
xforce opened this issue Sep 11, 2022 · 9 comments
Closed

Error when doing external fetch on Cloudflare Workers #6739

xforce opened this issue Sep 11, 2022 · 9 comments
Labels
p1-important SvelteKit cannot be used by a large number of people, basic functionality is missing, etc. pkg:adapter-cloudflare-workers
Milestone

Comments

@xforce
Copy link

xforce commented Sep 11, 2022

Describe the bug

Since #6565 doing external fetch in a load function will fail with Failed to get the 'mode' property on 'Request' on Cloudflare Workers.

Reproduction

The simplest reproduction is simply having a load function like this

export async function load({ fetch }) {
  await fetch("https://google.com");
}

A full project here: https://github.com/xforce/sveltekit-cloudflare-error
This requires are cloudflare account with workers enabled.
run: npm run build && npx wrangler publish.

Logs

"outcome": "ok",
  "scriptName": "sveltekit-cloudflare-worker-error",
  "exceptions": [],
  "logs": [
    {
      "message": [
        "Error: Failed to get the 'mode' property on 'Request': the property is not implemented.\n    at fetch (worker.js:1748:24)\n    at async fetcher (worker.js:1735:22)\n    at async load (worker.js:597:3)\n    at async load_data (worker.js:2011:16)\n    at async worker.js:2823:18"
      ],
      "level": "error",
      "timestamp": 1662933225460
    }
  ],


### System Info

```shell
System:
    OS: Linux 5.19 Arch Linux
    CPU: (32) x64 AMD Ryzen 9 5950X 16-Core Processor
    Memory: 31.19 GB / 62.72 GB
    Container: Yes
    Shell: 3.5.1 - /bin/fish
  Binaries:
    Node: 18.9.0 - /bin/node
    Yarn: 1.22.19 - /bin/yarn
    npm: 8.19.1 - /bin/npm
  Browsers:
    Brave Browser: 105.1.43.89
    Chromium: 105.0.5195.102
    Firefox: 104.0.2
  npmPackages:
    @sveltejs/adapter-auto: next => 1.0.0-next.72
    @sveltejs/adapter-cloudflare-workers: ^1.0.0-next.54 => 1.0.0-next.54
    @sveltejs/kit: next => 1.0.0-next.480
    svelte: ^3.44.0 => 3.50.1
    vite: ^3.1.0 => 3.1.0

Severity

blocking an upgrade

Additional Information

This appears to come from this line

if (request.mode === 'no-cors') {

@dominikg
Copy link
Member

It looks like cloudflare does not support everything on RequestInit, here's what they have https://developers.cloudflare.com/workers//runtime-apis/request#requestinit mode and credentials are not on it

@dominikg
Copy link
Member

dominikg commented Sep 12, 2022

Just accessing credentials (or mode i guess) throws an error, so even req.credentials !== 'omit' isn't working. It's not just undefined, you cannot use it at all in cloudflare at runtime. Unfortunately it doesn't throw an error while using it in miniflare.

https://github.com/sveltejs/kit/pull/6565/files#diff-3f12c17057d74a7d75f121baea53823d2ebfefa033e5cb834c05f6edca1a847dR81

@dominikg
Copy link
Member

only workaround i found so far is reverting to 1.0.0-next.469 which is the release before #6565 was introduced (next.470).

@denizkenan
Copy link

denizkenan commented Sep 13, 2022

Adding an handleFetch in src/hooks.server.js fixes this bug quite awfully. But hey, it works! ¯\_(ツ)_/¯

....
export async function handleFetch({ request, fetch }) {
	// FIXES: https://github.com/sveltejs/kit/issues/6739
	const rekuest = {
		get(target: Request, prop: string) {
			if (['credentials', 'mode'].includes(prop)) {
				return '¯¯\\_(ツ)_//¯¯';
			}
			return target[prop];
		}
	};
	return fetch(new Proxy(request, rekuest));
}

PS: wrangler dev --local (powered by miniflare) was unable to detect this bug due its slightly different implementation of the Runtime spec. A fix has been committed to miniflare after reporting this behaviour (kudos to @mrbbot). Hopefully next miniflare release can detect custom behaviour of cloudflare's Request/Response objects.

@xforce
Copy link
Author

xforce commented Sep 13, 2022

@denizkenan This workaround doesn't seem to work on Cloudflare Workers, the runtime fetch doesn't appear to treat the proxy as a Request object and does a toString(). Which then leads to fetch failing with Fetch API cannot load: [object Object]

@benmccann benmccann added this to the 1.0 milestone Sep 14, 2022
@benmccann benmccann added adapters - general Support for functionality general to all adapters p1-important SvelteKit cannot be used by a large number of people, basic functionality is missing, etc. pkg:adapter-cloudflare-workers and removed adapters - general Support for functionality general to all adapters labels Sep 14, 2022
@tv42
Copy link

tv42 commented Sep 28, 2022

This is particularly hideous if you want to access a +server.js helper.
With SSR, you have two choices:

  • use Svelte's internal fetch: triggers this bug
  • use global fetch: CF Workers can't reach their own URLs (I guess to avoid infinite recursion), the request 404s.

I'm not aware of any workaround, except downgrading / patching SvelteKit :-(

EDIT: This sucks extra for me personally, because I already have code depending on handleFetch being called for internal URLs. I can't downgrade easily.

tv42 added a commit to tripoutside/sveltekit-cloudflare-workaround-issue6739 that referenced this issue Sep 28, 2022
Cloudflare Workers `Request` object is non-standard and has properties
`mode` and `credentials` that throw exceptions if accessed.
Its constructor also refuses to accept explicit values for those.

The spec for `Request` pretty clearly states defaults are mode="cors"
and credentials="same-origin":
https://fetch.spec.whatwg.org/#request-class

Use the knowledge that Cloudflare Workers `Request` has a property
`cf`, and avoid accessing the boobytrapped properties when running in
Cloudflare.

See sveltejs#6739
@tv42
Copy link

tv42 commented Sep 28, 2022

I made a quick workaround for this. Here's the tl;dr:

- request.credentials !== 'omit'
+ ('cf' in request ? 'same-origin' : request.credentials) !== 'omit'

See tripoutside@2dcb3e3

npm i 'https://gitpkg.now.sh/tripoutside/sveltekit-cloudflare-workaround-issue6739/packages/kit?2dcb3e3cc0c2178850e617ff5595d8042d91a408'

(Jump through hoops due to npm/npm#2974 and github tarballs always starting from root.)

@Deebster
Copy link

Deebster commented Oct 3, 2022

My workaround was similar to @denizkenan - add src/hooks.server.js:

import type { HandleFetch } from "@sveltejs/kit";

type RequestMode = "cors" | "navigate" | "no-cors" | "same-origin";
type RequestCredentials = "include" | "omit" | "same-origin";

class RequestFix extends Request {
    get mode(): RequestMode {
        return "cors";
    }

    get credentials(): RequestCredentials {
        return "same-origin";
    }
}

export const handleFetch: HandleFetch = async ({ request, fetch }) => {
    // FIXME: Hack for https://github.com/sveltejs/kit/issues/6739
    const req = new RequestFix(request);
    return fetch(req);
}

This works when publishing via wrangler, but not when using Pages' GitHub integration to have Cloudflare perform the build. This is using a different adapter, which I suspect explains the difference.

@The-Noah
Copy link
Contributor

The-Noah commented Oct 7, 2022

My workaround was similar to @denizkenan - add src/hooks.server.js:

I've tried this and it does indeed work when using wrangler (miniflare) to run it locally. However it still errors when deployed to Cloudflare Pages, even when using wrangler.

The-Noah added a commit to The-Noah/sveltekit that referenced this issue Oct 9, 2022
Fallstop added a commit to Fallstop/HCNoticeReader that referenced this issue Jan 10, 2023
Seems to be a re-emergence of this bug: sveltejs/kit#6739
TODO: Remove this when it's been re-patched
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p1-important SvelteKit cannot be used by a large number of people, basic functionality is missing, etc. pkg:adapter-cloudflare-workers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants