From ecaac1334e93671462571597b51ca71e07a5fd55 Mon Sep 17 00:00:00 2001 From: Tyler Roach Date: Mon, 29 Jun 2026 11:12:02 -0400 Subject: [PATCH] fix(flags): tag fallback_reason so OpenFeature can distinguish causes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SelectedVariant now carries two source fields: `variant_source` (local | remote | fallback) and `fallback_reason` (FLAG_NOT_FOUND | MISSING_CONTEXT_KEY | NO_ROLLOUT_MATCH | BACKEND_ERROR | NOT_READY, set only when source is fallback). Three behaviorally distinct outcomes — flag-not-found, no-rollout-match, and missing-context-key — previously all returned the bare fallback. The OpenFeature wrapper collapsed them to FLAG_NOT_FOUND, sending callers chasing the flag name when the real cause was usually a rule miss or absent context. The wrapper now dispatches on fallback_reason and maps each to the spec-correct OpenFeature response. Most notably, NO_ROLLOUT_MATCH becomes `reason: DEFAULT` with no error code instead of FLAG_NOT_FOUND. Constant names align with mixpanel-php for consistency across SDKs. Linear: SDK-79 Co-Authored-By: Claude Opus 4.7 --- lib/FeatureFlags/MixpanelLocalFlags.php | 2 +- lib/FeatureFlags/MixpanelRemoteFlags.php | 6 +- lib/FeatureFlags/MixpanelSelectedVariant.php | 60 ++++++++++++++----- test/FeatureFlags/MixpanelLocalFlagsTest.php | 9 ++- test/FeatureFlags/MixpanelRemoteFlagsTest.php | 6 ++ 5 files changed, 64 insertions(+), 19 deletions(-) diff --git a/lib/FeatureFlags/MixpanelLocalFlags.php b/lib/FeatureFlags/MixpanelLocalFlags.php index 57f54c9..bec5d4c 100644 --- a/lib/FeatureFlags/MixpanelLocalFlags.php +++ b/lib/FeatureFlags/MixpanelLocalFlags.php @@ -129,7 +129,7 @@ public function getVariant($flagKey, FeatureFlags_MixpanelSelectedVariant $fallb $this->_trackExposure($flagKey, $selected, $context, 'local', $latencyMs); } - return $selected; + return $selected->withSource(FeatureFlags_MixpanelSelectedVariant::SOURCE_LOCAL); } public function getAllVariants(array $context) { diff --git a/lib/FeatureFlags/MixpanelRemoteFlags.php b/lib/FeatureFlags/MixpanelRemoteFlags.php index 358ed70..795755d 100644 --- a/lib/FeatureFlags/MixpanelRemoteFlags.php +++ b/lib/FeatureFlags/MixpanelRemoteFlags.php @@ -39,7 +39,8 @@ public function getVariant($flagKey, FeatureFlags_MixpanelSelectedVariant $fallb return $fallback->withFallbackReason(FeatureFlags_MixpanelSelectedVariant::REASON_FLAG_NOT_FOUND); } - $selected = FeatureFlags_MixpanelSelectedVariant::fromArray($flags[$flagKey]); + $selected = FeatureFlags_MixpanelSelectedVariant::fromArray($flags[$flagKey]) + ->withSource(FeatureFlags_MixpanelSelectedVariant::SOURCE_REMOTE); if ($reportExposure) { // Pass start/end so the exposure event carries @@ -62,7 +63,8 @@ public function getAllVariants(array $context) { $out = array(); foreach ($flags as $key => $payload) { - $out[$key] = FeatureFlags_MixpanelSelectedVariant::fromArray($payload); + $out[$key] = FeatureFlags_MixpanelSelectedVariant::fromArray($payload) + ->withSource(FeatureFlags_MixpanelSelectedVariant::SOURCE_REMOTE); } return $out; } diff --git a/lib/FeatureFlags/MixpanelSelectedVariant.php b/lib/FeatureFlags/MixpanelSelectedVariant.php index 6b1719f..bd988a1 100644 --- a/lib/FeatureFlags/MixpanelSelectedVariant.php +++ b/lib/FeatureFlags/MixpanelSelectedVariant.php @@ -3,23 +3,30 @@ /** * A feature-flag variant after evaluation. * - * Matches the shape used by the Python, Ruby, Go, and Java SDKs so that - * downstream wrappers (a future OpenFeature provider) and analytics - * tooling can rely on the same field names across languages. + * Matches the shape used by the Python, Ruby, Go, Java, and Node SDKs so + * that downstream wrappers (the OpenFeature provider) and analytics tooling + * can rely on the same field names across languages. * * The `experimentId`, `isExperimentActive`, and `isQaTester` fields are * kept available so a future OpenFeature wrapper can forward them as - * `flag_metadata` — addressing finding "Design C" in the cross-SDK - * audit (other wrappers throw this metadata away). + * `flag_metadata` — addressing finding "Design C" in the cross-SDK audit + * (other wrappers throw this metadata away). * - * `fallbackReason` is `null` when evaluation succeeded; when the SDK - * returns the fallback you passed in, it's set to one of the REASON_* - * constants below so the caller (or a future OpenFeature wrapper) can - * distinguish flag-not-found from missing-context-key from no-rollout- - * match etc. — addressing audit finding #1. + * Two fields describe the result's provenance: + * - `variantSource` is always set: `local` (local rule evaluation), + * `remote` (server-side /flags response), or `fallback` (developer + * fallback returned because the SDK had no value to serve). + * - `fallbackReason` is `null` on success; when `variantSource === 'fallback'` + * it's set to one of the REASON_* constants below so the OpenFeature + * wrapper can map each reason to the spec-correct error code instead of + * collapsing every fallback to FLAG_NOT_FOUND (audit finding #1). */ class FeatureFlags_MixpanelSelectedVariant { + const SOURCE_LOCAL = 'local'; + const SOURCE_REMOTE = 'remote'; + const SOURCE_FALLBACK = 'fallback'; + const REASON_FLAG_NOT_FOUND = 'FLAG_NOT_FOUND'; const REASON_MISSING_CONTEXT_KEY = 'MISSING_CONTEXT_KEY'; const REASON_NO_ROLLOUT_MATCH = 'NO_ROLLOUT_MATCH'; @@ -41,7 +48,10 @@ class FeatureFlags_MixpanelSelectedVariant { /** @var bool|null */ public $isQaTester; - /** @var string|null null on success; one of the REASON_* constants when the fallback was returned */ + /** @var string|null one of SOURCE_*; set by the providers on every returned variant */ + public $variantSource; + + /** @var string|null null on success; one of the REASON_* constants when variantSource === SOURCE_FALLBACK */ public $fallbackReason; public function __construct( @@ -50,7 +60,8 @@ public function __construct( $experimentId = null, $isExperimentActive = null, $isQaTester = null, - $fallbackReason = null + $fallbackReason = null, + $variantSource = null ) { $this->variantKey = $variantKey; $this->variantValue = $variantValue; @@ -58,6 +69,7 @@ public function __construct( $this->isExperimentActive = $isExperimentActive; $this->isQaTester = $isQaTester; $this->fallbackReason = $fallbackReason; + $this->variantSource = $variantSource; } /** @@ -78,15 +90,32 @@ public static function fromArray(array $data) { } /** - * Return a copy of this variant with the supplied fallbackReason - * set. Used by the providers to tag the caller's fallback without - * mutating their object. + * Return a copy of this variant with the given source. Clears + * fallbackReason — use {@link withFallbackReason} when returning a + * fallback. + * + * @param string $source one of the SOURCE_* constants + * @return FeatureFlags_MixpanelSelectedVariant + */ + public function withSource($source) { + $clone = clone $this; + $clone->variantSource = $source; + $clone->fallbackReason = null; + return $clone; + } + + /** + * Return a copy of this variant tagged as a fallback with the given + * reason. Sets `variantSource` to SOURCE_FALLBACK and `fallbackReason` + * to the supplied REASON_* constant. Used by the providers to tag the + * caller's fallback without mutating their object. * * @param string $reason one of the REASON_* constants * @return FeatureFlags_MixpanelSelectedVariant */ public function withFallbackReason($reason) { $clone = clone $this; + $clone->variantSource = self::SOURCE_FALLBACK; $clone->fallbackReason = $reason; return $clone; } @@ -101,6 +130,7 @@ public function toArray() { 'experiment_id' => $this->experimentId, 'is_experiment_active' => $this->isExperimentActive, 'is_qa_tester' => $this->isQaTester, + 'variant_source' => $this->variantSource, 'fallback_reason' => $this->fallbackReason, ); } diff --git a/test/FeatureFlags/MixpanelLocalFlagsTest.php b/test/FeatureFlags/MixpanelLocalFlagsTest.php index ba22a2c..1d25b73 100644 --- a/test/FeatureFlags/MixpanelLocalFlagsTest.php +++ b/test/FeatureFlags/MixpanelLocalFlagsTest.php @@ -86,12 +86,17 @@ public function testReturnsFallbackAndSetsReasonWhenFlagMissing() { $fallback = new FeatureFlags_MixpanelSelectedVariant(null, 'fallback'); $result = $this->_provider->getVariant('unknown', $fallback, array('distinct_id' => 'u1')); $this->assertEquals('fallback', $result->variantValue); + $this->assertEquals( + FeatureFlags_MixpanelSelectedVariant::SOURCE_FALLBACK, + $result->variantSource + ); $this->assertEquals( FeatureFlags_MixpanelSelectedVariant::REASON_FLAG_NOT_FOUND, $result->fallbackReason ); // The caller's fallback object must not be mutated — we return a clone. $this->assertNull($fallback->fallbackReason); + $this->assertNull($fallback->variantSource); } public function testGetVariantBeforeLoadReturnsNotReady() { @@ -132,7 +137,9 @@ public function testReturnsVariantOnSuccessfulEval() { $this->assertSame(true, $result->variantValue); $this->assertEquals('exp-my-flag', $result->experimentId); $this->assertTrue($result->isExperimentActive); - // null fallbackReason means evaluation succeeded — no fallback used. + // variantSource=local marks a real local-eval match. null fallbackReason + // means evaluation succeeded — no fallback used. + $this->assertEquals(FeatureFlags_MixpanelSelectedVariant::SOURCE_LOCAL, $result->variantSource); $this->assertNull($result->fallbackReason); } diff --git a/test/FeatureFlags/MixpanelRemoteFlagsTest.php b/test/FeatureFlags/MixpanelRemoteFlagsTest.php index 9fc4ad7..346cd16 100644 --- a/test/FeatureFlags/MixpanelRemoteFlagsTest.php +++ b/test/FeatureFlags/MixpanelRemoteFlagsTest.php @@ -56,6 +56,8 @@ public function testGetVariantSendsContextAndFlagKey() { $this->assertEquals('on', $variant->variantKey); $this->assertSame(true, $variant->variantValue); + $this->assertEquals(FeatureFlags_MixpanelSelectedVariant::SOURCE_REMOTE, $variant->variantSource); + $this->assertNull($variant->fallbackReason); $this->assertEquals('/flags', $this->_provider->lastRequest['path']); $this->assertEquals('my-flag', $this->_provider->lastRequest['query']['flag_key']); $this->assertEquals(json_encode($context), $this->_provider->lastRequest['query']['context']); @@ -93,6 +95,10 @@ public function testFlagMissingInResponseSetsFlagNotFoundReason() { $fallback = new FeatureFlags_MixpanelSelectedVariant(null, 'fb'); $variant = $this->_provider->getVariant('my-flag', $fallback, array('distinct_id' => 'u1')); $this->assertEquals('fb', $variant->variantValue); + $this->assertEquals( + FeatureFlags_MixpanelSelectedVariant::SOURCE_FALLBACK, + $variant->variantSource + ); $this->assertEquals( FeatureFlags_MixpanelSelectedVariant::REASON_FLAG_NOT_FOUND, $variant->fallbackReason