dulnan / nuxt-multi-cache

Advanced caching of components, routes and data for Nuxt 3. Dynamically define CDN cache control headers. Provides cache management API for purging items by key or using cache tags.
https://nuxt-multi-cache.dulnan.net
MIT License
209 stars 18 forks source link

Route Cache Caches the underlying API calls as well! #64

Open sathishzakapps opened 1 month ago

sathishzakapps commented 1 month ago

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.

dulnan commented 1 month ago

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

sathishzakapps commented 1 month ago

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
dulnan commented 1 month ago

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 commented 1 month ago

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

sathishzakapps commented 1 month ago

@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).

dulnan commented 1 month ago

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.

sathishzakapps commented 1 month ago

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

dulnan commented 1 month ago

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.

sathishzakapps commented 1 month ago

@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 commented 1 month ago

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.

sathishzakapps commented 1 month ago

Surely,

image

and nitro config

image
dulnan commented 1 month ago

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

sathishzakapps commented 1 month ago

@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 commented 1 month ago

@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
dulnan commented 1 month ago

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.

sathishzakapps commented 1 month ago

Thanks @dulnan

dulnan commented 1 month ago

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.

sathishzakapps commented 1 month ago

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..

dulnan commented 1 month ago

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!

sathishzakapps commented 1 month ago

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

dulnan commented 4 weeks ago

@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.

sathishzakapps commented 3 weeks ago

@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!!!