Skip to content

Evaluate checkFlags from the cache when the replicator is not ready - #201

Closed
ryanechternacht wants to merge 1 commit into
mainfrom
ryan/replicator-checkflags-offline
Closed

ryanechternacht wants to merge 1 commit into
mainfrom
ryan/replicator-checkflags-offline

Conversation

@ryanechternacht

Copy link
Copy Markdown
Member

Problem

In replicator mode, datastreamClient.isConnected() returns the replicator health endpoint's ready field. When a customer's Schematic account is closed or Schematic is unreachable, the replicator stays up and keeps its Redis cache, but reports ready: false.

checkFlags with keys only took the DataStream path when isConnected() was true. With the replicator not ready it skipped the cache, called features.checkFlags on the API, and when that failed returned flag defaults for every key. checkFlag has no such gate and keeps evaluating from Redis, so the single and bulk checks gave different answers for the same flag.

What I found

  • The gate came in with add datastream support to checkflags behavior #110 (add datastream support to checkflags). No rationale is recorded for it.
  • The Go SDK, the reference implementation, does not gate either path. CheckFlags in client/schematic_client.go goes to the datastream whenever useDataStream() is true, and resolveEntities in datastream/client.go returns whatever the cache holds in replicator mode.
  • Nothing downstream blocks it. checkFlagsViaDataStream loops over datastreamClient.checkFlag, and the replicator branch of that method evaluates from the cache without a readiness check. The Node datastream client has no separate bulk path (Go's evaluateBulk has no Node equivalent), so there is no other gate to remove.

Change

checkFlags now takes the DataStream path whenever a DataStream client exists and keys were passed. The isConnected() condition is gone, in both modes.

For WebSocket mode this is the same behavior checkFlag already has and matches Go: a disconnected client evaluates entities it has cached, and throws for any flag or entity it would need to fetch. checkFlagsViaDataStream returns null on the first throw, so the whole set still goes to the API in that case.

Test

New tests/unit/replicator/check-flags-not-ready.test.ts (and a .fernignore entry for the directory). It uses a real DataStream client in replicator mode, the real WASM rules engine, and the in-memory fake Redis seeded the way the replicator writes it (snake_case JSON under versioned keys). fetch is stubbed so the health endpoint returns { ready: false, cache_version: "v-test" }.

  • checkFlags for a metric-gated flag and an always-on flag, both defaulting to false, returns true for each with the cached company's ID, and neither features.checkFlags nor features.checkFlag is called.
  • checkFlag and checkFlags agree for the same flag.

Both tests fail on main (every key comes back as its default) and pass with this change. yarn build and yarn test pass (101 suites; the Redis integration suite is skipped locally without TEST_REDIS_URL).

🤖 Generated with Claude Code

checkFlags only took the DataStream path when isConnected() was true. In
replicator mode isConnected() is the health endpoint's ready field, which
goes false when Schematic is unreachable or the account is closed even
though the replicator's Redis cache is intact. checkFlags then skipped the
cache, called the API, and returned flag defaults for every key when that
failed. checkFlag has no such gate, so the two disagreed.

Drop the isConnected() gate so checkFlags evaluates locally whenever a
DataStream client exists, matching checkFlag and the Go SDK. In WebSocket
mode a disconnected client still evaluates entities it has cached and
throws for anything it would need to fetch, which sends the whole set to
the API as before.

Add a replicator mode test with a real DataStream client, fake Redis
seeded in the replicator's key layout, the real WASM engine, and a health
endpoint reporting ready: false. checkFlags returns the evaluated values
and never calls the API.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@ryanechternacht

Copy link
Copy Markdown
Member Author

Closing in favor of fixing this in the Replicator. The SDK is right to gate flag checks on Replicator readiness; the Replicator should keep reporting ready in /health once its cache is fully loaded, including after it loses its connection to Schematic. A Replicator PR is coming for that. The separate track optimistic-update PR stays open.

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