Skip to content

fix(analytics): distinguish fallback reasons + forward backend error message (SDK-79, SDK-83)#277

Open
tylerjroach wants to merge 7 commits into
masterfrom
fix/sdk-79-variant-source-fallback-reason
Open

fix(analytics): distinguish fallback reasons + forward backend error message (SDK-79, SDK-83)#277
tylerjroach wants to merge 7 commits into
masterfrom
fix/sdk-79-variant-source-fallback-reason

Conversation

@tylerjroach

@tylerjroach tylerjroach commented Jun 29, 2026

Copy link
Copy Markdown

Summary

Bundles two related fixes. Both touch the same fallback_reason plumbing 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.

SelectedVariant now carries two source fields: variant_source (local / remote / fallback) and fallback_reason (set only when source is fallback). The wrapper 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.

SDK-83 — forward the backend's error message (remote eval)

When the remote /flags endpoint 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 to FLAG_NOT_FOUND — indistinguishable from a genuinely missing flag. Go propagates these as GENERAL with the full backend message; the goal here is to match that.

To carry the message, FallbackReason is upgraded from a bare string constant to a small value object with kind (the PHP-aligned discriminator) and optional message (set on BACKEND_ERROR and MISSING_CONTEXT_KEY). The remote provider's catch branches tag the fallback with FallbackReason.backend_error(message); the wrapper forwards message into OpenFeature's error_message / errorMessage.

Fix pattern

FallbackReason kinds (PHP-aligned)

Kind When it fires Carries message?
FLAG_NOT_FOUND Flag key not in the ruleset / absent from /flags response no
MISSING_CONTEXT_KEY Required context attribute (distinct_id / targeting key) absent the missing attribute name
NO_ROLLOUT_MATCH Flag exists, user isn't in any rollout no
BACKEND_ERROR Remote /flags error response the backend's response body (SDK-83)
NOT_READY Provider not initialized no

OpenFeature mapping

Kind OpenFeature response
FLAG_NOT_FOUND error_code: FLAG_NOT_FOUND, reason: DEFAULT
MISSING_CONTEXT_KEY error_code: TARGETING_KEY_MISSING, reason: ERROR, error_message: <attribute name>
NO_ROLLOUT_MATCH reason: DEFAULT, no error
BACKEND_ERROR error_code: GENERAL, reason: ERROR, error_message: <backend body>
NOT_READY error_code: PROVIDER_NOT_READY, reason: ERROR

Test plan

  • Full base SDK + OpenFeature wrapper test suites pass
  • New BACKEND_ERROR producer-side test verifies the backend response body propagates through to fallback_reason.message
  • Wrapper test verifies error_message / errorMessage forwards the backend message for BACKEND_ERROR and the missing-attribute name for MISSING_CONTEXT_KEY

🤖 Generated with Claude Code

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 tylerjroach requested review from a team and rahul-mixpanel June 29, 2026 15:10
@linear-code

linear-code Bot commented Jun 29, 2026

Copy link
Copy Markdown

SDK-79

SDK-83

@tylerjroach tylerjroach changed the title fix(flags): tag fallback_reason so OpenFeature can distinguish causes (SDK-79) feat(analytics): tag fallback_reason so OpenFeature can distinguish causes (SDK-79) Jun 29, 2026
tylerjroach and others added 2 commits June 29, 2026 11:58
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>
@tylerjroach tylerjroach changed the title feat(analytics): tag fallback_reason so OpenFeature can distinguish causes (SDK-79) fix(analytics): distinguish fallback reasons + forward backend error message (SDK-79, SDK-83) Jun 29, 2026
tylerjroach and others added 3 commits June 30, 2026 10:59
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>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_source and structured fallback_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.kind and map to spec-appropriate OpenFeature reason / 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.

Comment thread packages/mixpanel/lib/flags/local_flags.js Outdated
Comment thread packages/mixpanel/lib/flags/remote_flags.js
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>
@tylerjroach

Copy link
Copy Markdown
Author

Pushed b8b474d addressing both @copilot inline comments.

local_flags.js:160 — added the missing closing quote so the log message reads correctly: Cannot find flag definition for key: 'my-flag'.

remote_flags.js:107err.message was unsafe for non-Error rejections (null, undefined, plain strings). Extracted a safe message once (err instanceof Error ? err.message : String(err ?? 'unknown error')) and reused it for both the log line and the FallbackReason.backendError payload. New test locks in the null-rejection path — the existing HTTP error test is unchanged.

No behavioral change to the happy path or to real HTTP errors — Ketan's approval should still stand.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants