Explicitly disallow fetch() caching

[mattrobenolt]

For the sake of our customers, I don't feel it's worth us fighting that "this implementation is wrong" with our customers in the middle.

If this hack were specific to an implementation of fetch, I'd feel differently, but given this is in the spec, and we're just being overly explicit, rather than relying on a "default" behavior, I'm more ok with it.

I still firmly disagree with implementations that are choosing to cache POST, especially also with an Authorization header, but there's only so much I/we can control.

See context: vercel/next.js#46856

Fixes: cloudflare/workers-sdk#101 cloudflare/workers-sdk#99

[dgraham]

Thanks!

[ryespresso]

FYI, this breaks cloudflare workers. Any thoughts on how to fix this?

The 'cache' field on 'RequestInitializerDict' is not implemented.

[mattrobenolt]

sigh, we can just revert it. I'm not sure how we can make everything happy, and this seemed like a backwards compatible change since this field is in the fetch spec. I'm guessing the extra field can't just be ignored because of TypeScript being too strict?

As a TypeScript noob, can we just have an arbitrary dict here? Since in most things, this would be ignored as extraneous.

[mattrobenolt]

I don't think there's much we can do other than maybe open an issue over here? https://github.com/cloudflare/workers-sdk Beyond us reverting.

[iheanyi]

@ryeshrimp Yikes, sorry for breaking your application. To move forward, you have two options.

1. You can pin the previous version of 1.6.0 to your package.json since this is the last-known good version of the package.

2. I'm not sure if this will work within Cloudflare, but could you use a custom package like undici and use the fetch from that in your application? See the example below.

import { connect } from '@planetscale/database'
import { fetch } from 'undici'

const config = {
  fetch,
  host: '<host>',
  username: '<user>',
  password: '<password>'
}

const conn = connect(config)
const results = await conn.execute('select 1 from dual')
console.log(results)

[dgraham]

cloudflare/workers-sdk#192

[ryespresso]

Thanks for making the bug report @dgraham. And thanks for the options @iheanyi. Happily using 1.6 for now. We'll see how cloudflare responds. Will try option 2 in a bit

[sjc5]

FYI, this breaks cloudflare workers. Any thoughts on how to fix this?

The 'cache' field on 'RequestInitializerDict' is not implemented.

+1 on noticing the breaking, and +1 on pinning to 1.6.0 fixing it. Glad to see I'm not alone.

[chocolatkey]

If you want to fix cloudflare workers + 1.7.0, override the fetch option in the config:

fetch: (url: string, init: RequestInit<RequestInitCfProperties>) => {
    delete (init as any)["cache"]; // Remove cache header
    return fetch(url, init);
}

[rjvim]

@chocolatkey Pretty new to CF Workers, so not sure how to use the above solution.

The following is code is init'ing a new CF Worker project which uses fetch.

export default {
  async fetch(
    request: Request,
    env: Env,
    ctx: ExecutionContext
  ): Promise<Response> {
    return await router.handle(request);
  },
};

How can I override the fetch in this scenario? Any help please?

[chocolatkey]

@rjvim I guess I wasn't specific enough in my example, the fetch property is part of the config you can send to the planetscale connect function. For example:

import { connect } from '@planetscale/database'

const config = {
  host: 'xxx',
  username: 'xxx',
  password: 'xxx',
  fetch: (url: string, init: RequestInit<RequestInitCfProperties>) => {
    delete (init as any)["cache"]; // Remove cache header
    return fetch(url, init);
  }
}

const conn = connect(config)

P.S. do this outside the request handling function, it will get reused between requests to the worker

[maraisr]

This broke all Cloudflare deployments 😅

cc @dgraham

[mattrobenolt]

https://github.com/cloudflare/workers-sdk/issues/3005

[a-eid]

@iheanyi undici does not seem to work with cloudflare workers, reverting to 1.6.0 also produces the same error.

[rishavs]

@rjvim I guess I wasn't specific enough in my example, the fetch property is part of the config you can send to the planetscale connect function. For example:

import { connect } from '@planetscale/database'

const config = {
  host: 'xxx',
  username: 'xxx',
  password: 'xxx',
  fetch: (url: string, init: RequestInit<RequestInitCfProperties>) => {
    delete (init as any)["cache"]; // Remove cache header
    return fetch(url, init);
  }
}

const conn = connect(config)

P.S. do this outside the request handling function, it will get reused between requests to the worker

Thanks @chocolatkey . For those looking for a solution, I can confirm this works. Also, as its just a couple of lines of change, reverting this - once workers fix their bugs, will be easy.

[ian-speckart]

@rjvim I guess I wasn't specific enough in my example, the fetch property is part of the config you can send to the planetscale connect function. For example:

import { connect } from '@planetscale/database'

const config = {
  host: 'xxx',
  username: 'xxx',
  password: 'xxx',
  fetch: (url: string, init: RequestInit<RequestInitCfProperties>) => {
    delete (init as any)["cache"]; // Remove cache header
    return fetch(url, init);
  }
}

const conn = connect(config)

P.S. do this outside the request handling function, it will get reused between requests to the worker

I confirm that the issue persists and that the solution works.

"P.S. do this outside the request handling function, it will get reused between requests to the worker"

@rjvim Is that something we should be doing, though?
I'm asking because the username and password are inside the env object, which is accessible inside the request handling function.
The only way to create the connection outside is to hardcode the credentials in the worker, without using environment variables.

Also, Planetscale said that their serverless driver uses a connection factory that provides fresh connections (I'm using that with your solution): https://github.com/planetscale/database-js#connection-factory

[mateusedua]

Just like chocolatkey mentioned above, this works perfectly.

const config = {
    host: "",
    username: "",
    password: "",
    fetch: (url, init) => {
        delete (init)["cache"];
        return fetch(url, init);
    }
}

export default config