-
-
Notifications
You must be signed in to change notification settings - Fork 99
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
Cached responses 200 -> 304 #2328
Comments
Thanks for you report! I personally don't think it's a good idea to add this to Terminus directly. I think I'm open to change my mind though if other people face the same issue and had to do the same workaround? |
Yeah it's one to understand where the responsibility of the cache is owned at this point. My argument for adding it to Terminus (as a condition to turn on and off Use case being that if you're doing checks in Kubernetes or other deployment methods/services that allow for health checking you want to know real time that the service is up and not some old out dated previous cached result that maybe during that time you ran out of memory, database crashed etc. |
I had this issue too and had to use the work around. |
When using the `@HealthCheck` decorator it will now per default set the following header: `Cache-Control: no-cache, no-store, must-revalidate` To disable this behavior set `@HealthCheck({ noCache: false })` resolves #2328
After further considerations I have implemented this feature #2335 |
When using the `@HealthCheck` decorator it will now per default set the following header: `Cache-Control: no-cache, no-store, must-revalidate` To disable this behavior set `@HealthCheck({ noCache: false })` resolves #2328
When using the `@HealthCheck` decorator it will now per default set the following header: `Cache-Control: no-cache, no-store, must-revalidate` To disable this behavior set `@HealthCheck({ noCache: false })` resolves #2328
When using the `@HealthCheck` decorator it will now per default set the following header: `Cache-Control: no-cache, no-store, must-revalidate` To disable this behavior set `@HealthCheck({ noCache: false })` resolves #2328
Released with v10.1.0 🎉 |
Thanks @BrunnerLivio ! |
Is there an existing issue for this?
Current behavior
When using a health check route
@HealthCheck()
within a controller the response is cached.It originally returns the 200 status code then as now the result is cached it's returning 304.
This will cause issues for kubernetes and other services trying to read 200 status code, then maybe retrying to get a cached result for of 304.
Minimum reproduction code
Steps to reproduce
No response
Expected behavior
Set
Cache-Control
tono-store
Package version
latest
NestJS version
latest
Node.js version
18.1
In which operating systems have you tested?
Other
A work around for people is to apply
@Header('Cache-Control', 'no-store')
under@HealthCheck()
The text was updated successfully, but these errors were encountered: