nuxt-multi-cache icon indicating copy to clipboard operation
nuxt-multi-cache copied to clipboard

Route Cache Caches the underlying API calls as well!

Open sathishzakapps opened this issue 1 year ago • 22 comments

Hi @dulnan

I'm encountering an issue with version 3.3.0.

When using the Route Cache for the route ~/pages/product/[id].vue, it caches the underlying API calls as well. When rendering the page, I use two API calls: one to get content (/api/cms/product) and the actual product data (/api/productInfo?id=productId), using Redis as the cache base.

image

I can see three entries in the cache: one for the actual page and two for /api/cms/product and /api/productInfo.

I would like only the page to be cached. Is there any way to achieve this? It worked as expected in version 3.2.0.

Thanks in advance; any help would be greatly appreciated.

sathishzakapps avatar Jul 29 '24 13:07 sathishzakapps

That sounds very weird and unexpected. I will look into this now.

dulnan avatar Jul 29 '24 13:07 dulnan

Thank you @dulnan

To add to the above I have tried to avoid caching the api calls using the enabledForRequest but does not help.

image

sathishzakapps avatar Jul 29 '24 13:07 sathishzakapps

I am unable to reproduce this behaviour. I have created a new API route that does not call useRouteCache() at all, meaning it should not be cached. I then call this API using useFetch() on a page that calls useRouteCache() with isCacheable(). I only have one entry in the route cache, the page. Only when I explicitly call isCacheable() inside the API event handler do I see that its response has been cached.

If you say thatenabledForRequest does not work, then something super fishy must be going on: This method is called very early on in the request life cycle and if its returns false, then the cache is completely unavailable for any subsequent code... This makes no sense :thinking:

Do you have other places where you use useRouteCache()? e.g. inside a server middleware?

dulnan avatar Jul 29 '24 14:07 dulnan

Okay just as I sent it, I was able to reproduce it. And... it's VERY weird what I'm seeing. This is impossible.

dulnan avatar Jul 29 '24 14:07 dulnan

@dulnan I think the Nitro webhook calls the cache on every request that it is making including the page renders and API calls. After all, Page and all API calls are go through Nitro's life cycle.. (Not sure I am blabbering, though).

sathishzakapps avatar Jul 29 '24 14:07 sathishzakapps

Yeah, I found the problem, look:

Page component

<script lang="ts" setup>
import { useFetch, useRequestEvent } from '#imports'

const event = useRequestEvent()

if (event) {
  event.context.foobar = 'TEST'
}

const { data } = await useFetch('/api/uncacheableApi')
</script>

API route

import { defineEventHandler } from 'h3'

export default defineEventHandler<{ data: number }>((event) => {
  console.log('Test: ' + event.context.foobar)
  return {
    data: Date.now(),
  }
})

This logs Test: TEST to the console. Apparently the context is shared (?!) for the entire request, including any API calls that are made during server side rendering. One of the things I did change in the latest release was using this exact event.context to add the cache helper, instead of putting it on the event directly. This explains why enabledForRequest does not work: The first time it's called, your method returns true, because that path is not starting with /api. But at that point it is now set in event.context and thus available for any and all API routes. Crazy.

I will do a fix now and release it asap.

dulnan avatar Jul 29 '24 14:07 dulnan

Thank you so much @dulnan. Understood whats happening :)

sathishzakapps avatar Jul 29 '24 14:07 sathishzakapps

I have just released 3.3.1, would you like to give it a try?

Also thank you very much for bringing this issue to my attention! I would probably not have noticed this for some time and seeing as this bug could result in very unexpected runtime behaviour, I'm glad I was able to fix it now.

dulnan avatar Jul 29 '24 15:07 dulnan

@dulnan Thank you so much!!! that worked :) the quickest response I have ever got!

you are doing great help by writing this module, we understand how important the cache is, providing the Nuxt's performance.

However, I encountered the below error. while serving the cached response.

Serving cached route for path: /product/A-23958 {
  fullKey: '/product/A-23958'
}

[nuxt] [request error] [unhandled] [500] Cannot set headers after they are sent to the client
  at new NodeError (node:internal/errors:405:5)  
  at ServerResponse.setHeader (node:_http_outgoing:652:11)  
  at setResponseHeaders (./.output/server/chunks/runtime.mjs:2559:20)  
  at Object.handler (./.output/server/chunks/runtime.mjs:5423:7)  
  at ./.output/server/chunks/runtime.mjs:3063:31  
  at process.processTicksAndRejections (node:internal/process/task_queues:95:5)  
  at async Object.callAsync (./.output/server/chunks/runtime.mjs:5544:16)  
  at async Server.toNodeHandle (./.output/server/chunks/runtime.mjs:3329:7)

This issue should not related to the new release, as I can see the same issue in the previous version as well!

Kindly let me know if I can avoid this error?

sathishzakapps avatar Jul 29 '24 15:07 sathishzakapps

Sure, could you show me which other modules you have installed or nitro/h3 related libraries you use? I know for example that the h3-compression library is not compatible with this module due to the way the response is compressed.

dulnan avatar Jul 29 '24 15:07 dulnan

Surely, image

and nitro config

image

sathishzakapps avatar Jul 29 '24 15:07 sathishzakapps

Could you try (if easily possible) to disable nuxt-security to see if it would resolve the issue?

dulnan avatar Jul 30 '24 04:07 dulnan

@dulnan tried removing them but no luck. not sure whats happening, shall check if any of the code that we wrote causing this issue and come back here. If I need further help..

sathishzakapps avatar Jul 30 '24 06:07 sathishzakapps

@dulnan Thank you so much!!! that worked :) the quickest response I have ever got!

you are doing great help by writing this module, we understand how important the cache is, providing the Nuxt's performance.

However, I encountered the below error. while serving the cached response.

Serving cached route for path: /product/A-23958 {
  fullKey: '/product/A-23958'
}

[nuxt] [request error] [unhandled] [500] Cannot set headers after they are sent to the client
  at new NodeError (node:internal/errors:405:5)  
  at ServerResponse.setHeader (node:_http_outgoing:652:11)  
  at setResponseHeaders (./.output/server/chunks/runtime.mjs:2559:20)  
  at Object.handler (./.output/server/chunks/runtime.mjs:5423:7)  
  at ./.output/server/chunks/runtime.mjs:3063:31  
  at process.processTicksAndRejections (node:internal/process/task_queues:95:5)  
  at async Object.callAsync (./.output/server/chunks/runtime.mjs:5544:16)  
  at async Server.toNodeHandle (./.output/server/chunks/runtime.mjs:3329:7)

This issue should not related to the new release, as I can see the same issue in the previous version as well!

Kindly let me know if I can avoid this error?

@dulnan found the issue, getting this error since I used the routeRules to set the no-cache header for all the routes.

So once after the response sent to the client from our cache the routeRules is trying to set no-cache header.

PS: We have not faced this issue in the version 3.2.0 when we use the same routeRules config.

image

sathishzakapps avatar Jul 30 '24 12:07 sathishzakapps

Oh I see. I did not think of this scenario. Generally I would advise against using route cache + route rules together, but in this case it would be a valid use case to add headers. I will check to see if it's possible.

dulnan avatar Jul 31 '24 03:07 dulnan

Thanks @dulnan

sathishzakapps avatar Jul 31 '24 07:07 sathishzakapps

I looked into it and once again it's such an annoying problem with the way h3/nitro are setup. For some context: As an author of a module like nuxt-multi-cache, it's very hard to implement route caching. There are only so many ways one can hook into the life cycle of the request. The problem now is that the part where I cache the response happens at the very end of the request. This is why the module is able to also cache the headers applied by your route rule initially and also stores them in the cache. However, in the second request, when the module wants to serve something from cache, the only "sane" way to achieve that is inside the onRequest hook. There I load it from cache and immediately return the response. However: Nitro will still go through the entire rest of the request, calling hooks, applying route rules, etc. As of now I have not found a way to prevent that reliably.

That was also the case pre 3.3.0, however there it worked because "serving from cache" was implemented as an "event handler", because nitro did not provide a hook at that time. But in order to implement "stale if error" and "stale while revalidate", I could no longer implement it as an event handler, so I had to switch to the hook.

Anyway, as a temporary workaround, I suggest to not use routeRules to apply headers but instead apply them in a server middleware using setResponseHeader. In the meantime I will continue to explore options to make route rules work with route cache.

dulnan avatar Aug 01 '24 06:08 dulnan

Thank you for the detailed explanation @dulnan. For now I shall write the no-cache to the routes that I am not caching at all and I'll see if I need them in the routes I am caching and add them as necessary...

I need to understand more on h3/nitro implementation.

Thank you once again for the prompt replies :) helped me a lot..

sathishzakapps avatar Aug 01 '24 08:08 sathishzakapps

I may have found a workaround which I've already implemented in #66, but I still need to do some more testing. You may not need to apply the workaround for long, if it turns out to work properly!

dulnan avatar Aug 01 '24 08:08 dulnan

Great to hear! Hope all will be good (y) in the new PR and works well!!

sathishzakapps avatar Aug 01 '24 08:08 sathishzakapps

@sathishzakapps I just merged the change that should hopefully fix this issue. I will do some more testing (together with other fixes pushed today) and if all works, create a new release today.

dulnan avatar Aug 25 '24 07:08 dulnan

@dulnan Thank you for the update, I shall get the latest and test them and let you know in case of any issues? Once again thank you!!!

sathishzakapps avatar Aug 26 '24 12:08 sathishzakapps