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

🐛 BUG: Inconsistent behavior with pages functions runtime for cache deletion in wrangler pages dev #2575

Open
yw662 opened this issue Feb 23, 2023 · 8 comments
Labels
bug Something isn't working

Comments

@yw662
Copy link

yw662 commented Feb 23, 2023

Which Cloudflare product(s) does this pertain to?

Pages

What version of Wrangler are you using?

2.11.0

What operating system are you using?

macos 13.1

Describe the Bug

In pages functions, wrangler would allow to delete a certain kind of cache entries while the real runtime won't. The code is like

  static async delete(cache: ManagedCache, key: string) {
    try {
      await cache.cache.delete(key)
    } catch (e) {
      logger.log('[cache][workaround]', cache.name, key, String(e))
      await this.put(cache, key, new Response('', { headers: { DELETED: 'DELETED', 'Expires': new Date(0).toUTCString() } }))
    }
  }

Wrangler won't throw on the await cache.delete(key) line, while the actual runtime always throws Error: unable to delete cached response. (and fallback to the workaround, the workaround works though).

The cache entry was added like:

  static async put(cache: ManagedCache, key: string, resp: Response): Promise<void> {
    const headers = new Headers(resp.headers)
    if (!headers.has('Expires') && !headers.has('Cache-Control')) {
      headers.set('Expires', new Date(cache.expires + Date.now()).toUTCString())
    }
    const putted = new Response(resp.clone().body, { headers })
    await cache.cache.put(key, putted)
    return
  }

And inside .wrangler/state/cache, it looks like:

{
  "key": "<key>",
  "expiration": 1677275323,
  "metadata": {
    "status": 200,
    "headers": [
      [
        "access-control-allow-headers",
        "Content-Type, Authorization"
      ],
      [
        "access-control-allow-methods",
        "OPTIONS,POST,GET"
      ],
      [
        "access-control-allow-origin",
        "http://localhost"
      ],
      [
        "connection",
        "keep-alive"
      ],
      [
        "content-length",
        "<length>"
      ],
      [
        "content-type",
        "application/json"
      ],
      [
        "date",
        "Thu, 23 Feb 2023 21:48:43 GMT"
      ],
      [
        "expires",
        "Fri, 24 Feb 2023 21:48:43 GMT"
      ],
      [
        "via",
        "1.1 <YeahThisIsFromAWS>.cloudfront.net (CloudFront)"
      ],
      [
        "x-amz-apigw-id",
        "<hidden>"
      ],
      [
        "x-amz-cf-id",
        "<hidden>"
      ],
      [
        "x-amz-cf-pop",
        "<hidden>"
      ],
      [
        "x-amzn-requestid",
        "<hidden>"
      ],
      [
        "x-amzn-trace-id",
        "<hidden>"
      ],
      [
        "x-cache",
        "Miss from cloudfront"
      ]
    ]
  }
}

(the amazon fields are from upstream, I don't know whether they would be relevant so I would just put them there).

I am not sure whether this is a bug for the real runtime or for workers-sdk. There is a discussion for it at discord id 1074116255134535700 (workers-help > cannot delete cache entries) in cloudflare server.

@yw662 yw662 added the bug Something isn't working label Feb 23, 2023
@mrbbot
Copy link
Contributor

mrbbot commented Mar 2, 2023

Hey! 👋 Thanks for opening this issue. I've raised it internally. 👍

@jspspike jspspike assigned nevikashah and unassigned nevikashah Mar 7, 2023
@mrbbot
Copy link
Contributor

mrbbot commented Mar 21, 2023

Hey again, it looks like we're not going to be able to fix this for a while. 😞 Does this error occur every time or just occasionally? Apologies about this, I'd keep using your workaround for the time being.

@yw662
Copy link
Author

yw662 commented Mar 21, 2023

Idk. Should this be considered a bug of miniflare/wrangler or the runtime ?

@mrbbot
Copy link
Contributor

mrbbot commented Mar 23, 2023

This is a bug in the runtime, or more likely a component the runtime communicates with. 👍

@stenet
Copy link

stenet commented Apr 11, 2023

Hey, I get the same error in Cloudflare Pages (production). The error occurs every time cache.delete() is executed. match and put work without any problem.

Is there a workaround for this until the problem is fixed?

@yw662
Copy link
Author

yw662 commented Apr 11, 2023

Hey, I get the same error in Cloudflare Pages (production). The error occurs every time cache.delete() is executed. match and put work without any problem.

Is there a workaround for this until the problem is fixed?

Yes there is one. Since you can put, you can put sth with a header like DELETED: true (or whatever), and detect that thing when you match.

@stenet
Copy link

stenet commented Apr 11, 2023

Thanks! Looks like putting a Response with Cache-Control: max-age=0 works the same like a delete :)

@yw662
Copy link
Author

yw662 commented Apr 11, 2023

Thanks! Looks like putting a Response with Cache-Control: max-age=0 works the same like a delete :)

Oh yeah I have that in my workaround. So glad to know it can work on its own without other tricks. Thanks.

@penalosa penalosa transferred this issue from cloudflare/workers-sdk Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Other
Development

No branches or pull requests

4 participants