fix(analytics): distinguish fallback reasons + forward backend error message (SDK-79, SDK-83)#277
fix(analytics): distinguish fallback reasons + forward backend error message (SDK-79, SDK-83)#277tylerjroach wants to merge 7 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>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…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 frozen string map to a small frozen value object exposing `kind` (the discriminator) and `message` (set on reasons that carry useful detail). Factory methods construct each kind; frozen singletons for the no-detail reasons. SDK-83: RemoteFeatureFlagsProvider's catch branches now tag the fallback with FallbackReason.backendError(err.message) so the OpenFeature wrapper can forward the backend's response into ResolutionDetails.errorMessage. 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"). Wrapper dispatches on fallback_reason.kind, forwards reason.message into errorMessage for BACKEND_ERROR and MISSING_CONTEXT_KEY. Linear: SDK-79, SDK-83 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The wrapper short-circuits to PROVIDER_NOT_READY at the top of _resolveFlag via the _initialized check, 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 / Ruby PR #153 cleaned up. Remove NOT_READY from FallbackReasonKind (both runtime + types.d.ts), drop the notReady() factory and _NOT_READY singleton, and drop the NOT_READY arm from the wrapper switch. PROVIDER_NOT_READY tests above (via _initialized=false) cover the short-circuit 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>
There was a problem hiding this comment.
Pull request overview
This PR improves feature-flag fallback signaling across the base Mixpanel SDK and the OpenFeature server provider by (1) distinguishing why a fallback occurred and (2) propagating backend error messages through to OpenFeature error details.
Changes:
- Introduces
variant_sourceand structuredfallback_reason(kind + optional message) for returned variants. - Updates local/remote flags providers to tag successful variants and fallbacks, including forwarding backend error messages on remote HTTP failures.
- Updates the OpenFeature wrapper to dispatch on
fallback_reason.kindand map to spec-appropriate OpenFeaturereason/errorCode/errorMessage, with new/expanded tests.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/openfeature-server-provider/test/MixpanelProvider.test.ts | Adds wrapper-level tests for fallback_reason.kind dispatch and message forwarding. |
| packages/openfeature-server-provider/src/MixpanelProvider.ts | Maps new fallback kinds to OpenFeature responses and forwards fallback messages into errorMessage. |
| packages/mixpanel/test/flags/remote_flags.js | Updates remote provider tests for variant_source tagging and BACKEND_ERROR propagation. |
| packages/mixpanel/test/flags/local_flags.js | Adds local provider tests verifying distinct fallback kinds and sources. |
| packages/mixpanel/lib/flags/variant_source.js | Adds runtime implementation for VariantSource, FallbackReason, and tagging helpers. |
| packages/mixpanel/lib/flags/types.d.ts | Extends SelectedVariant typing and adds declarations for fallback/source plumbing. |
| packages/mixpanel/lib/flags/remote_flags.js | Tags remote variants as remote, and tags fallbacks (including backend error message). |
| packages/mixpanel/lib/flags/local_flags.js | Tags local variants as local, and tags distinct fallback paths via fallback_reason. |
| packages/mixpanel/lib/flags/index.js | Re-exports the new tagging helpers/constants from the flags entrypoint. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Two low-priority Copilot comments on the SDK-79/SDK-83 PR: - local_flags.js:160 — closing quote was missing from the warn message, so the log read "Cannot find flag definition for key: 'my-flag" with an unbalanced quote. Added the trailing '. - remote_flags.js:107 — the catch block read `err.message` directly, which throws if the rejected value is null/undefined or a plain string. Extract a safe string once (Error → message, else String(err ?? 'unknown error')) and reuse it for both the log line and FallbackReason.backendError. New test covers the non-Error rejection path. No behavioral change on the happy path or on real HTTP errors — the existing BACKEND_ERROR test still passes unchanged. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Pushed
No behavioral change to the happy path or to real HTTP errors — Ketan's approval should still stand. |
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