Repository navigation
docs(flags): define failed remote flag handling - #211
Conversation
Cross-SDK audit of per-flag
|
| SDK | Fresh main revision / evidence | Verdict | Current findings and remaining work |
|---|---|---|---|
| .NET | 6f08ed0 | Fix | Failed values/payloads are filtered, but direct getters reconstruct a disabled structured result and report flag_missing, including after inconclusive local evaluation. Global errors therefore lack evaluation_failed. Bulk fallback also discards successful local values and payloads. |
| Android | d753954 | Fix | Client filtering remains gated by the global error bit. False/absent bits leak failed values/payloads and can defeat caller defaults. When a default is used, tracking receives that substituted value, not the missing-response sentinel. Server paths accept failed records and lack evaluation_failed; deprecated fallback can discard successful local pairs. Retain correct complete-refresh replacement. |
| Flutter | f11de77 | Fix and dependency updates | Windows/Linux Dart still filters only with global errors true. Fresh failed reads then become flag_missing,errors_while_computing_flags, not the required failure classification. Native dependency ranges inherit affected engines. Web behavior depends on the host's unpinned JS. Filter complete responses without introducing blanket old-cache retention. No new caller-default parameter is requested. |
| iOS | 808e861 | Fix | Normalization ignores failed. Getters, result lists and capture properties expose placeholders. Errored merges can overwrite a successful cached value with failed false; failed variants may also contribute payloads. Preserve failure diagnostics and use the existing missing-event representation instead of the placeholder. |
| Go | cbf49d4 | Changed: telemetry follow-up needed | #434, released in v1.42.3, still fixes no-result handling, bulk omission and local-value preservation. However, the poller result carries flagFailed but not the response-wide error bit. A public-getter/event repro yields only evaluation_failed when both errors must be reported. The no-poller path already copies both fields. No new combined getter/default API is required. |
| Elixir | e095a19 | Fix | Conversion accepts failed records as decisions, including their associated payloads. Filter affected direct/bulk/enrichment paths and retain failure identity for evaluation_failed after direct or local-inconclusive fallback. Local-wins merging already protects successful local records/pairs. |
| JavaScript | 6b84590 | Fix | Node: direct rich results and diagnostics ignore failure; combined fallback overwrites successful local values. A false failed record may leave the old payload paired with the wrong value; failed variants can leak payloads too. Browser/RN/core: global-bit-gated filtering leaks failed results/defaults; browser subset merges can overwrite prior success. Filtered fresh failures are misreported as flag_missing. |
| PHP | 8235861 | Telemetry fix only | Reconfirmed correct failed value/payload filtering, null structured results, and successful local-value preservation on exposed paths. Failed direct reads still become flag_missing, including local-inconclusive fallback; global errors consequently lack evaluation_failed. Event response is already null. No additional paired API is required. |
| Python | c60e7a6 | Fix | Rich/bulk conversion still ignores private _failed for older detailed and typed-v4 records. Combined fallback replaces successful local values and payloads. Decode already suppresses failed remote payloads; that does not prevent local-pair loss. Direct events need evaluation_failed and the missing-response None representation instead of placeholder false. |
| Ruby | 9f72706 | Fix | Normalization correctly omits failed values/payloads, but direct callers reconstruct false/structured results and flag_missing. Combined bulk/capture fallback preserves successful local values and payloads only when the global bit is true; false/absent drops them. This server same-call preservation must be independent of that bit. |
| Rust | aa81041 | Implement failed-aware decoding | FlagDetail still drops failed. Normalization emits the failed false placeholder and any attached payload. Decode the marker, omit failed contributions, and use existing direct-read None outcomes. Do not add a structured getter or flag-called tracking where the SDK has none. |
| Unity | 854e0c4 | Fix | C# parsing ignores failed, so failed false defeats the existing true default. Cache update also replaces state on errored refreshes rather than merging successful values. Fix missing-result/event diagnostics and retention at supported refresh boundaries. No new bulk API is required. |
| Pi | da7e307 | Not applicable | Reconfirmed analytics-only SDK usage. No evaluated SDK flag surface. MCP flag-management tools are separate. |
| OpenClaw | 0a4058d | Not applicable | Reconfirmed private analytics-only client and hooks, without SDK flag evaluation or enrichment. |
| OpenCode | baa32a6 | Not applicable | Reconfirmed analytics hooks rather than SDK flag APIs. No flag implementation needed for this issue. |
| KMP | e92e165 | Dependency updates | Pins remain Android 3.75.1/core 6.48.0, iOS 3.64.1, and JS 1.435.8. Inspected pinned engines still have filtering/default/diagnostic gaps; the Apple bridge's default cannot help while failed false remains a value. Consume fixed engine versions/backports and verify each platform. |
| Godot | b1db27e | Fix | Parsing ignores failed, defeating the existing true default. Refresh always replaces cache, including errored responses that must merge successful values. Emitted failed-read events lack both required diagnostics. No new bulk/local-evaluation API is required. |
| Unreal | 97d187e | Blocked / verify | Still requires an external POSTHOG_RS_PATH embedding checkout rather than a sufficient pinned implementation. C++ trusts exported bool/string values, so failed filtering, defaults and enrichment cannot be established end to end from main alone. Its diagnostic path lacks per-flag failure identity. Pin and audit the exporter boundary. |
| Roblox | cef6b1e | Fix | Filtering still requires global errors true, and flat mirrors can reintroduce failed values/payloads. Leaked false defeats the existing true default; emitted events lack the required error classifications. Complete/nonerrored clearing is already correct: independently filter incoming failed keys without retaining old cache solely because a record failed. |
Specs and shared coverage
| Repository | Inspected main | Current status / follow-up |
|---|---|---|
| sdk-specs | 4d19cd8 | This PR is still open. The reviewed target is 762f86a, covering four canonical capabilities and the reconciled failed acceptance fixture. Do not confuse proposed-spec coverage with main or executed SDK conformance. |
| SDK test harness | f7afba4 | Main is unchanged. Existing v4 scalar/payload failed-record fixtures provide partial coverage. Add rich-result, detailed-v2, caller-default, combined-pair preservation, refresh-boundary and dual-error/event-sentinel cases. Scalar false alone cannot distinguish correct failure handling from a placeholder leak. |
Scope and validation
failedexists in detailed v2 and v4 responses. Recognizing it does not require migrating every SDK to v4. Flat legacy maps retain their existing interpretation.- Check both
errorsWhileComputingFlags=falseand an absent field. When both error causes apply, emitted direct-read diagnostics must retain bothevaluation_failedanderrors_while_computing_flags. Event response is the platform's missing sentinel, not the caller default. - Preserve documented scalar/boolean no-result conventions. Honor caller defaults where the existing API supports them, with healthy false as a control. Do not invent default parameters, combined getters, enrichment, or tracking APIs.
- Same-call, same-context server-local pair preservation is not a blanket stale-client-cache policy. Snapshot-specific inclusion/unresolved tracking remains outside this PR's contract.
- Go reproduction: existing focused tests pass. Temporarily strengthened the existing public-getter/event test to require the global error alongside
evaluation_failed; it fails with"evaluation_failed" does not contain "errors_while_computing_flags". Restored it and reran successfully. Separate false/absent-bit value-handling probes pass. No SDK edits were retained. - Ruby focused existing tests and helper probes, PHP filtering probes, and JS/Python extracted-source checks revalidate the affected paths. These are not full SDK wire-delivery runs. Mobile/game SDKs and .NET/Elixir/Rust remain source-audited without runtime execution.
- Flutter's consumer-resolved Android version and host-provided browser version remain deployment-dependent. KMP's pinned engines were checked at their dependency versions, not substituted with latest main.
- The exact spec head passes strict validation for all 69 specs. Its Gherkin fixture parses into 31 cases; the current-head discovery result remains 4 harness-ready and 27 with existing missing bindings. No new integration tags or claim of end-to-end conformance.
- Re-probed the unchanged harness's
readers.v1_failedfixture: legacy scalar false and no payload. This allowed scalar sentinel does not prove rich-result/bulk/telemetry conformance. - All SDK audit worktrees are clean. No SDK fixes, commits, or pull requests were made as part of this reinspection.
dustinbyrne
left a comment
There was a problem hiding this comment.
The failed-record contract is consistent with the backend and preserves healthy/local results and tracking controls. One optional acceptance-example consistency suggestion is inline.
AI-assisted review.
haacked
left a comment
There was a problem hiding this comment.
Looks good! A few non-blocking suggestions and questions inline.
|
|
||
| If no successful local or otherwise supported fallback supplies a value for the same evaluation context, `get-feature-flag-result` SHALL return the platform's no-result outcome. The SDK MAY return an idiomatic error alongside that outcome where its public API supports one. Scalar and boolean getters that share this evaluation path SHALL retain their documented no-result/default mappings, including a legacy false sentinel; such a sentinel MUST NOT be treated as a successfully evaluated disabled flag. | ||
|
|
||
| An absent `failed` marker or explicit `failed: false` SHALL NOT by itself invalidate a record. In particular, a healthy boolean false SHALL remain a valid disabled result even when another flag caused response-wide evaluation errors. Legacy response formats without per-flag failure markers SHALL retain their existing value interpretation. |
There was a problem hiding this comment.
suggestion: This changes what Node, Python, iOS, Unity, and Godot return for a failed flag today, but the spec doesn't say whether SDKs can ship that in a minor release. Could you say that here, as is-feature-enabled does for its breaking change?
There was a problem hiding this comment.
If this matches how the backend already works, I'd treat it as a patch-level SDK bug fix rather than wait for a major release. We're correcting the interpretation of failed: true, not introducing new backend behavior. Each SDK should keep its existing no-result/default conventions, without changing public API signatures.
…n the legacy getter feature (#216) #211 says a record with `failed: true` has no evaluated value, and a scalar getter may keep its legacy false sentinel. The harness YAML feature_flags_v4 suite already accepts false, JSON null or no value for such a record. The generated Gherkin feature still required exactly false, so an SDK that returns no value for a failed record passed the YAML suite and failed this one. Regenerate both features with the harness generator. In the getter feature, the four reader cases with `failed: true` (dependency_error, error, error_with_value, v1_failed) now use "the public flag getter should return false or no value", and the feature description says so. Every other scenario is unchanged: other reader errors still expect exactly false. The payload feature changes only in its header, which names contract 10.0.0. A failed record already has no payload. The README describes the failed-record rule.
Summary
Define how SDKs handle a detailed
/flagsrecord markedfailed: true. It means the server could not evaluate the flag, not that the flag is disabled. The field exists in both the v2 and v4 response formats.This clarifies the behavior discussed in PostHog/posthog-go#434. The cross-SDK audit is in this comment.
Changes
evaluation_failedon tracked direct getter reads, notflag_missing. Preserve the missing response sentinel, tracking controls, dedupe, and the expliciterrors_while_computing_flagsdiagnostic.The changes are synced into four canonical specs and retained in two archived OpenSpec changes. No SDK code, snapshot missing/unresolved policy, integration opt-in tags or harness bindings change.
Deferred: The release-policy review question remains open at the author's request. This update adds no release-compatibility rule.
Validation
openspec validate --specs --strict --no-interactive: all 69 specs pass.f7afba4378941ae40bf84d8f10d30f2cfa4df212reports 4 harness-ready cases and 27 with existing missing bindings. No new unbound step phrases were introduced. This is not an SDK runtime conformance result.git diff --checkpasses. OpenSpec tasks are complete and no active change directory remains.autoreview --mode branch --base origin/mainpassed for762f86awith no actionable findings. The parent test-authoring audit checked the fixture's retained assertions and unchanged neighboring cases.