feat(feature-flags) local evaluation psr 16 caching#161
Conversation
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
lib/Client.php:1271-1275
**Cache failures not fully swallowed** — only `CacheInvalidArgumentException` (invalid key) is caught, but real-world backends (Redis, Memcached, file) throw plain `\RuntimeException` or `\Exception` on connection failures, serialization errors, etc. The PR's own comment says "A cache failure must never break evaluation", but an uncaught general exception will propagate through `loadFlags()` and crash the caller. The same narrow catch exists in `clearFeatureFlagCache()` (line ~1291) and in the `get()` guard in `loadFlags()` (line ~1167) — all three should use `\Throwable`.
```suggestion
} catch (\Throwable $e) {
if ($this->debug) {
error_log("[PostHog][Client] Failed to cache feature flag definitions: " . $e->getMessage());
}
}
```
Reviews (1): Last reviewed commit: "feat(feature-flags) local evaluation psr..." | Re-trigger Greptile |
|
Thanks for the PR @jaapieaapie1! We'll have a look at this shortly. |
dustinbyrne
left a comment
There was a problem hiding this comment.
Thanks again for taking the time to put this together! I've taken some time to look at the implementation and the referenced issues.
For server-side SDKs, we've been moving towards a specific implementation that allows for various custom caching strategies like the one you've implemented here. Take a look at https://posthog.com/docs/feature-flags/local-evaluation/distributed-environments
We don't yet support this for PHP, but in short, the interface would look something like:
interface FeatureFlagDefinitionCacheProvider
{
/**
* @return array{
* flags: array,
* group_type_mapping: array,
* cohorts: array
* }|null|ProviderAwaitable
*/
public function getFlagDefinitions();
/** @return bool|ProviderAwaitable */
public function shouldFetchFlagDefinitions();
/** @return void|ProviderAwaitable */
public function onFlagDefinitionsReceived(array $data);
/** @return void|ProviderAwaitable */
public function shutdown();
}We have these interfaces available in Kotlin, Ruby, Node, and Python if you'd like to take a look.
I'm not opposed to shipping specific implementations of the above in our SDKs, like a PSR-16 compliant cache. Though, I do think it's directionally worthwhile to wait for the above abstraction before we build this into the public API space.
💡 Motivation and Context
For high volume applications you want local evaluation and caching for the feature flags, this implements a way for users to provide PSR16 compatible caching layer for PostHog to use for it's feature flag definitions.
💚 How did you test it?
Automatic tests and tested in my own Laravel application.
📝 Checklist
If releasing new changes
pnpm changesetto generate a changeset file