Skip to content

feat(feature-flags) local evaluation psr 16 caching#161

Open
jaapieaapie1 wants to merge 2 commits into
PostHog:mainfrom
jaapieaapie1:feat/psr16-caching-local-eval
Open

feat(feature-flags) local evaluation psr 16 caching#161
jaapieaapie1 wants to merge 2 commits into
PostHog:mainfrom
jaapieaapie1:feat/psr16-caching-local-eval

Conversation

@jaapieaapie1

Copy link
Copy Markdown

💡 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

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs if needed.
  • No breaking change or entry added to the changelog.

If releasing new changes

  • Ran pnpm changeset to generate a changeset file

@jaapieaapie1 jaapieaapie1 requested a review from a team as a code owner June 10, 2026 18:21
@greptile-apps

greptile-apps Bot commented Jun 10, 2026

Copy link
Copy Markdown
Prompt To Fix All With AI
Fix 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

Comment thread lib/Client.php Outdated
@dustinbyrne

Copy link
Copy Markdown
Contributor

Thanks for the PR @jaapieaapie1! We'll have a look at this shortly.

@dustinbyrne dustinbyrne left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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

Successfully merging this pull request may close these issues.

2 participants