-
Notifications
You must be signed in to change notification settings - Fork 67
fix(analytics): distinguish fallback reasons + forward backend error message (SDK-79, SDK-83) #153
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
dbe6f04
0b137d4
bb46240
d3565ae
49afe9f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,35 +1,114 @@ | ||
| module Mixpanel | ||
| module Flags | ||
| # Where a SelectedVariant came from. Set by the providers on every returned | ||
| # variant — coarse-grained (local / remote / fallback). For the specific | ||
| # reason behind a fallback, see {FallbackReason}. | ||
| module VariantSource | ||
| LOCAL = 'local'.freeze | ||
| REMOTE = 'remote'.freeze | ||
| FALLBACK = 'fallback'.freeze | ||
| end | ||
|
|
||
| # Why the SDK returned the developer fallback. Only meaningful when | ||
| # SelectedVariant#variant_source == VariantSource::FALLBACK. | ||
| # | ||
| # `kind` is the discriminator (matches the PHP constant set). `message` | ||
| # is set on the reasons that carry useful detail (BACKEND_ERROR with the | ||
| # backend's response, MISSING_CONTEXT_KEY with the missing attribute); | ||
| # nil otherwise. The OpenFeature wrapper dispatches on kind and forwards | ||
| # message into ResolutionDetails#error_message. | ||
| class FallbackReason | ||
| KINDS = %i[flag_not_found missing_context_key no_rollout_match backend_error].freeze | ||
|
|
||
| attr_reader :kind, :message | ||
|
|
||
| def initialize(kind, message: nil) | ||
| raise ArgumentError, "Unknown FallbackReason kind: #{kind.inspect}" unless KINDS.include?(kind) | ||
|
|
||
| @kind = kind | ||
| @message = message | ||
| freeze | ||
| end | ||
|
|
||
| def ==(other) | ||
| other.is_a?(FallbackReason) && other.kind == @kind && other.message == @message | ||
| end | ||
| alias_method :eql?, :== | ||
|
|
||
| def hash | ||
| [self.class, @kind, @message].hash | ||
| end | ||
|
|
||
| def to_h | ||
| { kind: @kind, message: @message }.compact | ||
| end | ||
|
|
||
| # Factory methods. Reasons without meaningful detail return a frozen | ||
| # singleton; reasons with detail allocate per call. | ||
| def self.flag_not_found; FLAG_NOT_FOUND; end | ||
| def self.no_rollout_match; NO_ROLLOUT_MATCH; end | ||
| def self.missing_context_key(key = nil); new(:missing_context_key, message: key); end | ||
| def self.backend_error(message); new(:backend_error, message: message); end | ||
|
|
||
| FLAG_NOT_FOUND = new(:flag_not_found) | ||
| NO_ROLLOUT_MATCH = new(:no_rollout_match) | ||
| end | ||
|
|
||
| # Selected variant returned from flag evaluation | ||
| class SelectedVariant | ||
| attr_accessor :variant_key, :variant_value, :experiment_id, | ||
| :is_experiment_active, :is_qa_tester | ||
| :is_experiment_active, :is_qa_tester, | ||
| :variant_source, :fallback_reason | ||
|
|
||
| # @param variant_key [String, nil] The variant key | ||
| # @param variant_value [Object] The variant value (any type) | ||
| # @param experiment_id [String, nil] Associated experiment ID | ||
| # @param is_experiment_active [Boolean, nil] Whether experiment is active | ||
| # @param is_qa_tester [Boolean, nil] Whether user is a QA tester | ||
| def initialize(variant_key: nil, variant_value: nil, experiment_id: nil, | ||
| is_experiment_active: nil, is_qa_tester: nil) | ||
| is_experiment_active: nil, is_qa_tester: nil, | ||
| variant_source: nil, fallback_reason: nil) | ||
| @variant_key = variant_key | ||
| @variant_value = variant_value | ||
| @experiment_id = experiment_id | ||
| @is_experiment_active = is_experiment_active | ||
| @is_qa_tester = is_qa_tester | ||
| @variant_source = variant_source | ||
| @fallback_reason = fallback_reason | ||
| end | ||
|
|
||
| # Return a copy of this variant tagged with the given source. Clears | ||
| # fallback_reason — use {#as_fallback} when returning a fallback. | ||
| def with_source(source) | ||
| copy_with(variant_source: source, fallback_reason: nil) | ||
| end | ||
|
|
||
| # Return a copy tagged as a fallback with the given reason. | ||
| def as_fallback(reason) | ||
| copy_with(variant_source: VariantSource::FALLBACK, fallback_reason: reason) | ||
| end | ||
|
|
||
| # Convert to hash representation | ||
| # @return [Hash] | ||
| def to_h | ||
| { | ||
| variant_key: @variant_key, | ||
| variant_value: @variant_value, | ||
| experiment_id: @experiment_id, | ||
| is_experiment_active: @is_experiment_active, | ||
| is_qa_tester: @is_qa_tester | ||
| is_qa_tester: @is_qa_tester, | ||
| variant_source: @variant_source, | ||
| fallback_reason: @fallback_reason | ||
| }.compact | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def copy_with(variant_source: @variant_source, fallback_reason: @fallback_reason) | ||
| SelectedVariant.new( | ||
| variant_key: @variant_key, | ||
| variant_value: @variant_value, | ||
| experiment_id: @experiment_id, | ||
| is_experiment_active: @is_experiment_active, | ||
| is_qa_tester: @is_qa_tester, | ||
| variant_source: variant_source, | ||
| fallback_reason: fallback_reason | ||
| ) | ||
| end | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -69,16 +69,41 @@ def resolve(flag_key, default_value, expected_type, evaluation_context) | |
|
|
||
| begin | ||
| result = @flags_provider.get_variant(flag_key, fallback, context, report_exposure: true) | ||
| rescue StandardError | ||
| return error_result(default_value, ::OpenFeature::SDK::Provider::ErrorCode::GENERAL) | ||
| rescue StandardError => e | ||
| return error_result(default_value, ::OpenFeature::SDK::Provider::ErrorCode::GENERAL, e.message) | ||
| end | ||
|
|
||
| if result.equal?(fallback) | ||
| # variant_source distinguishes local / remote / fallback. When fallback, | ||
| # fallback_reason carries the specific reason (PHP-aligned constants) | ||
| # 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The wrapper accepts Context Used: Our developers like concise reviews. Avoid verbose... (source) |
||
| when :flag_not_found | ||
| return ::OpenFeature::SDK::Provider::ResolutionDetails.new( | ||
| value: default_value, | ||
| error_code: ::OpenFeature::SDK::Provider::ErrorCode::FLAG_NOT_FOUND, | ||
| reason: ::OpenFeature::SDK::Provider::Reason::DEFAULT | ||
| ) | ||
| when :missing_context_key | ||
| return error_result( | ||
| default_value, | ||
| ::OpenFeature::SDK::Provider::ErrorCode::TARGETING_KEY_MISSING, | ||
| result.fallback_reason.message | ||
| ) | ||
| when :no_rollout_match | ||
| # Flag exists, user just didn't match any rollout — per the | ||
| # OpenFeature spec this is `reason: DEFAULT` with no error. | ||
| return ::OpenFeature::SDK::Provider::ResolutionDetails.new( | ||
| value: default_value, | ||
| reason: ::OpenFeature::SDK::Provider::Reason::DEFAULT | ||
| ) | ||
| when :backend_error | ||
| return error_result( | ||
| default_value, | ||
| ::OpenFeature::SDK::Provider::ErrorCode::GENERAL, | ||
| result.fallback_reason.message | ||
| ) | ||
| end | ||
|
|
||
| value = result.variant_value | ||
|
|
@@ -158,10 +183,11 @@ def flags_ready? | |
| end | ||
| end | ||
|
|
||
| def error_result(default_value, error_code) | ||
| def error_result(default_value, error_code, error_message = nil) | ||
| ::OpenFeature::SDK::Provider::ResolutionDetails.new( | ||
| value: default_value, | ||
| error_code: error_code, | ||
| error_message: error_message, | ||
| reason: ::OpenFeature::SDK::Provider::Reason::ERROR | ||
| ) | ||
| end | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a fallback result is converted with
SelectedVariant#to_h, this field remains aFallbackReasonobject rather than thekindandmessagehash exposed by its ownto_h. Callers serializing the result therefore receive an object representation instead of the new fallback metadata.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!