fix(analytics): distinguish fallback reasons + forward backend error message (SDK-79, SDK-83)#153
Conversation
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 <noreply@anthropic.com>
tylerjroach
changed the title
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #153 +/- ##
==========================================
- Coverage 97.23% 96.97% -0.26%
==========================================
Files 15 15
Lines 723 761 +38
==========================================
+ Hits 703 738 +35
- Misses 20 23 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…end message (SDK-79, SDK-83) SDK-79 made fallback_reason a PHP-aligned string constant on every returned fallback variant. That covers the local-eval cases (flag not found, no rollout match, missing context key) but couldn't carry detail — when the remote /flags endpoint returned an error response, the SDK still had nowhere to attach the backend's message. Upgrade FallbackReason from a module of string constants to a small value object with `kind` (the discriminator) and optional `message`. Frozen singletons for the no-detail reasons (flag_not_found, no_rollout_match, not_ready); factory methods for the ones that carry detail (missing_context_key(key), backend_error(message)). SDK-83: RemoteFlagsProvider's rescue MixpanelError now tags the fallback with FallbackReason.backend_error(e.message) so the wrapper can forward the backend's response into ResolutionDetails#error_message. Without this the caller sees a bare GENERAL error and has to dig through logs to find out the backend rejected the request (e.g. "distinct_id must be provided in evalContext as a string"). Matches Go's existing behavior — Python and Node will follow in their PRs. Wrapper dispatches on reason.kind, forwards reason.message into error_message for backend_error and missing_context_key. Linear: SDK-79, SDK-83 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
tylerjroach
changed the title
The wrapper short-circuits to PROVIDER_NOT_READY at the top of resolve when flags_ready? is false, so no producer ever constructs a FallbackReason with kind :not_ready — the case was dead, same pattern Swift PR #745 / Android PR #981 / Python PR #180 cleaned up. Remove :not_ready from the KINDS array, drop the not_ready factory and NOT_READY singleton, and drop the :not_ready arm from the wrapper's case dispatch. The 'provider not ready' spec covers the short-circuit path unchanged. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Comments were explaining the absence of a case to a hypothetical cross-SDK reader. The absence is self-explanatory. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Confidence Score: 4/5The OpenFeature wrapper can crash with mixpanel-ruby versions allowed by its current dependency range.
openfeature-provider/lib/mixpanel/openfeature/provider.rb, openfeature-provider/mixpanel-ruby-openfeature.gemspec, and lib/mixpanel-ruby/flags/types.rb
|
| Filename | Overview |
|---|---|
| lib/mixpanel-ruby/flags/local_flags_provider.rb | Tags local matches and fallback outcomes, and filters aggregate results by source. |
| lib/mixpanel-ruby/flags/remote_flags_provider.rb | Tags remote matches and preserves request errors as detailed fallback reasons. |
| lib/mixpanel-ruby/flags/types.rb | Adds variant-source and fallback-reason types, but leaves the nested reason object unconverted in to_h. |
| openfeature-provider/lib/mixpanel/openfeature/provider.rb | Maps fallback reasons correctly for the updated SDK, but assumes the new API exists across all accepted dependency versions. |
Reviews (1): Last reviewed commit: "Merge branch 'master' into fix/sdk-79-va..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| # so we can map each to the spec-correct OpenFeature response instead | ||
| # of collapsing every fallback to FLAG_NOT_FOUND. SDK-83: BACKEND_ERROR | ||
| # also carries the backend message, forwarded as error_message. | ||
| case result.fallback_reason&.kind |
There was a problem hiding this comment.
The wrapper accepts mixpanel-ruby ~> 3.1, including releases where SelectedVariant has no fallback_reason. With one of those versions, every evaluation raises NoMethodError here after the existing rescue block, so callers receive an exception instead of resolution details. Raise the wrapper's minimum dependency to the release that introduces this API or handle legacy results explicitly.
Context Used: Our developers like concise reviews. Avoid verbose... (source)
| is_qa_tester: @is_qa_tester | ||
| is_qa_tester: @is_qa_tester, | ||
| variant_source: @variant_source, | ||
| fallback_reason: @fallback_reason |
There was a problem hiding this comment.
When a fallback result is converted with SelectedVariant#to_h, this field remains a FallbackReason object rather than the kind and message hash exposed by its own to_h. Callers serializing the result therefore receive an object representation instead of the new fallback metadata.
| fallback_reason: @fallback_reason | |
| fallback_reason: @fallback_reason&.to_h |
Context Used: Our developers like concise reviews. Avoid verbose... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
2 participants
Summary
Bundles two related fixes. Both touch the same
fallback_reasonplumbing so they merge as one PR.SDK-79 — distinguish fallback reasons (local eval)
Three local-evaluation outcomes — flag-not-found, no-rollout-match, and missing-context-key — previously all returned the bare developer 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.SelectedVariantnow carries two source fields:variant_source(local/remote/fallback) andfallback_reason(set only when source isfallback). The wrapper dispatches onfallback_reasonand maps each to the spec-correct OpenFeature response — most notably,NO_ROLLOUT_MATCHbecomesreason: DEFAULTwith no error code instead ofFLAG_NOT_FOUND.SDK-83 — forward the backend's error message (remote eval)
When the remote
/flagsendpoint returned an error response (e.g. HTTP 400 with "distinct_id must be provided in evalContext as a string"), the catch block returned the fallback variant without attaching the cause. The wrapper saw a fallback and translated toFLAG_NOT_FOUND— indistinguishable from a genuinely missing flag. Go propagates these asGENERALwith the full backend message; the goal here is to match that.To carry the message,
FallbackReasonis upgraded from a bare string constant to a small value object withkind(the PHP-aligned discriminator) and optionalmessage(set onBACKEND_ERRORandMISSING_CONTEXT_KEY). The remote provider's catch branches tag the fallback withFallbackReason.backend_error(message); the wrapper forwardsmessageinto OpenFeature'serror_message/errorMessage.Fix pattern
FallbackReasonkinds (PHP-aligned)FLAG_NOT_FOUND/flagsresponseMISSING_CONTEXT_KEYNO_ROLLOUT_MATCHBACKEND_ERROR/flagserror responseNOT_READYOpenFeature mapping
FLAG_NOT_FOUNDerror_code: FLAG_NOT_FOUND,reason: DEFAULTMISSING_CONTEXT_KEYerror_code: TARGETING_KEY_MISSING,reason: ERROR,error_message: <attribute name>NO_ROLLOUT_MATCHreason: DEFAULT, no errorBACKEND_ERRORerror_code: GENERAL,reason: ERROR,error_message: <backend body>NOT_READYerror_code: PROVIDER_NOT_READY,reason: ERRORTest plan
BACKEND_ERRORproducer-side test verifies the backend response body propagates through tofallback_reason.messageerror_message/errorMessageforwards the backend message forBACKEND_ERRORand the missing-attribute name forMISSING_CONTEXT_KEY🤖 Generated with Claude Code