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

Response caching broken due to HTTP 304 Not Modified response missing Expires header #436

Closed
aguthrie opened this issue Sep 5, 2024 · 5 comments

Comments

@aguthrie
Copy link

aguthrie commented Sep 5, 2024

Describe the bug
I have a 60s TTL enabled in the ld relay and am using the php sdk which uses the Kevinrob/guzzle-cache-middleware library to handle caching.

I'm seeing an issue where the first response is initially cached for 60s, then once it expires the middleware repeatedly calls ld relay, and ld relay returns a HTTP 304 Not Modified response.

The issue appears to be that ld relay does not set the Expires header in the HTTP 304 response, so the existing response with the old expiry header remains cached, and is retrieved again and again and is expired, causing a new request to the relay to be made.

Note: the Kevinrob/guzzle-cache-middleware intentionally does not set the expiry on the cache itself (e.g. memcached) because it supports refetching stale cache entries. If it did, this wouldn't be an issue since the entry would expire from the cache itself

references

To reproduce

  • configure ld relay ttl to 60s
  • configure a cache on the php sdk guzzlefeaturerequester

Expected behavior
Responses are cached for 60s, then fetched again. When fetched again, a HTTP 304 response with an Expires header set to now() + 60s should be sent so the cache entry is valid for another 60s.

Logs
If applicable, add any log output related to your problem.

SDK version
launchdarkly/server-sdk 4.3.0

Language version, developer tools
PHP
kevinrob/guzzle-cache-middleware v4.1.2

OS/platform
Debian Linux

Additional context
None

@cwaldren-ld
Copy link
Contributor

Hi @aguthrie , thanks for the report. We'll look into this. Can you let me know what version of Relay you have deployed?

Filed internally as 255328.

@aguthrie
Copy link
Author

aguthrie commented Sep 5, 2024

we're running 8.2.0. I can try upgrading to the latest, but it looks like the relevant cache response code hasn't changed since 6.x

@cwaldren-ld
Copy link
Contributor

No need, just wanted to confirm. Thanks!

@keelerm84
Copy link
Member

keelerm84 commented Sep 6, 2024

We have released v8.9.4 which we believe should address this issue for you. Thank you again for opening this issue and letting us know.

@aguthrie
Copy link
Author

aguthrie commented Sep 6, 2024

just tested with v8.9.4 and confirmed the cache now works as expected. really appreciate the quick turnaround on this one!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants