feat(analytics): add Source discriminated union with Fallback.Reason (SDK-79)#89
Open
tylerjroach wants to merge 4 commits into
Open
feat(analytics): add Source discriminated union with Fallback.Reason (SDK-79)#89tylerjroach wants to merge 4 commits into
tylerjroach wants to merge 4 commits into
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>
rahul-mixpanel
left a comment
There was a problem hiding this comment.
Four recommendations left as inline comments — one high, one medium, two low priority.
The OpenFeature wrapper short-circuits to PROVIDER_NOT_READY at the top of resolve via the areFlagsReady check, so no producer ever constructs a Source.Fallback with Reason.NOT_READY — the case was dead, same pattern Swift PR #745 / Android PR #981 / Python PR #180 / Ruby PR #153 / Node PR #277 cleaned up. Trivial to add back the moment a real call site needs it. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Source.Fallback now overrides equals/hashCode so two Fallback(REASON) instances compare structurally — otherwise SelectedVariant.equals() returns false for any two fallbacks with the same reason. - Source subclasses get toString() (Local / Remote / Fallback(REASON)) so SelectedVariant.toString() doesn't print Source\$Fallback@1a2b3c. - Restore @param Javadoc on both 5-arg and 6-arg SelectedVariant constructors. - Clarify isSuccess()'s source-based check vs the old variantKey-based check via a brief Javadoc note. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
Author
|
Thanks for the review — pushed
Mirrored on mixpanel-android (PR #981, commit |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Sourcediscriminated union (com.mixpanel.mixpanelapi.featureflags.model.Source):Source.Local|Source.Remote|Source.Fallback(Reason). Abstract class + nested static finals — Java 8 source level so nosealedkeyword.Source.Fallback.Reasonenum:FLAG_NOT_FOUND|MISSING_CONTEXT_KEY|NO_ROLLOUT_MATCH|BACKEND_ERROR|NOT_READY. Constants align with mixpanel-php.SelectedVariant.getSource()replaces the priorvariantSourcestring field.isFallback()now checkssource instanceof Source.Fallback.LocalFlagsProviderandRemoteFlagsProvidertag every returned variant: matches withSource.local()/Source.remote(), fallbacks withSource.fallback(reason)carrying the specific reason.Why
Three behaviorally distinct outcomes — flag-not-found, no-rollout-match, and missing-context-key — previously all returned the bare fallback. A future OpenFeature wrapper (in
openfeature-provider/) can now dispatch on the reason and map each to the spec-correct error code instead of collapsing them all toFLAG_NOT_FOUND.Discriminated union rather than two flat fields because the language is type-safe enough to model it that way; invalid states like "successful evaluation with a fallback reason" are unrepresentable.
Cross-SDK fix tracked at Linear SDK-79. Constant set matches
mixpanel-phpso callers across SDKs see the same vocabulary.Follow-up PR
The
openfeature-providermodule needs a matching update to dispatch onSource.Fallback.Reason— that PR depends on this one shipping in a newmixpanel-javarelease first (the provider consumes the published Maven coordinate). It'll come right after release.Test plan
mvn test -Dgpg.skip=true)LocalFlagsProviderTest(53) andRemoteFlagsProviderTest(8) continue passing — theisFallback()contract is preservedSelectedVariant.equals()updated to includesource🤖 Generated with Claude Code