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

Miniflare sets CF-Cache-Status to null on misses #4379

Open
aeneasr opened this issue Dec 28, 2022 · 3 comments
Open

Miniflare sets CF-Cache-Status to null on misses #4379

aeneasr opened this issue Dec 28, 2022 · 3 comments
Labels
miniflare Relating to Miniflare

Comments

@aeneasr
Copy link

aeneasr commented Dec 28, 2022

Contrary to cloudflare's behavior, miniflare does not set CF-Cache-Status when the cache was a miss.

Cloudflare however returns the header with MISS:

CF-Cache-Status: miss
@mrbbot
Copy link
Contributor

mrbbot commented Jan 9, 2023

Hey! 👋 Could I clarify, is this when using the fetch API? Or the caches API? Would you be able to provide the code snippet where you're seeing this? Miniflare does not support cache with fetch, so this is potentially why you're seeing this.

@mrbbot mrbbot added the question A question about a feature rather than a bug label Jan 9, 2023
@aeneasr
Copy link
Author

aeneasr commented Jan 10, 2023

Hey, sorry for being so sparse on detail in this issue. The code is essentially:

export async function handleRequest(request: Request, ctx: ExecutionContext) {
  // ... cache key computation ...

  // Try to fetch the item from the cache
  const cache = caches.default
  let response

  response = await cache.match(cacheKey)

  if (!response) {
    response = await fetchAndPotentiallyCache({
      request,
      cacheKey,
      ctx,
    })
  }

  response = new Response(response.body, response)

  // Done
  return response
}

export async function fetchAndPotentiallyCache({
  request,
  cacheKey,
  ctx,
}: {
  request: Request
  cacheKey: string
  ctx: ExecutionContext
}): Promise<Response> {
  let response = await fetch(request, {
    cf: {
      cacheEverything: true
    },
  })

  response = new Response(response.body, response)
  response.headers.set(
    "Cache-Control",
    `s-maxage=60`,
  )

  // This is a small workaround for miniflare
  // TODO remove once https://github.com/cloudflare/miniflare/issues/82 is fixed
  const cloned = response.clone()
  ctx.waitUntil(
    caches.default
      .delete(cacheKey)
      .then(() => caches.default.put(cacheKey, cloned)),
  )

  return response
}

When we get a cache hit in cache.match, the header is set to CF-Cache-Status: hit. If not, the header is unset.

@mrbbot
Copy link
Contributor

mrbbot commented Nov 7, 2023

Hey! 👋 Apologies for the delayed follow up. I'm going to transfer this to workers-sdk as that's the new home for Miniflare 3, and that will make it easier for us to track this. 👍

@mrbbot mrbbot removed the question A question about a feature rather than a bug label Nov 7, 2023
@mrbbot mrbbot transferred this issue from cloudflare/miniflare Nov 7, 2023
@mrbbot mrbbot added the miniflare Relating to Miniflare label Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
miniflare Relating to Miniflare
Projects
Status: Backlog
Development

No branches or pull requests

2 participants