fix(flags): treat failed remote records as unevaluated

.changeset/fix-failed-flag-records.md

@@ -0,0 +1,5 @@
+---
+"posthog-ruby": patch
+---
+
+Treat failed feature flag records as unevaluated, report evaluation failure diagnostics, and preserve successful local values and payloads when remote fallback fails.

lib/posthog/client.rb

@@ -784,11 +784,14 @@ def evaluate_flags(
 
             metadata = ff.metadata
             reason = ff.reason
+            enabled = ff.enabled ? true : false
+            # nil marks no evaluated value; retain the record only for failure diagnostics.
+            enabled = nil if ff.failed == true
             records[key_str] = FeatureFlagEvaluations::EvaluatedFlagRecord.new(
               key: key_str,
-              enabled: ff.enabled ? true : false,
-              variant: ff.variant,
-              payload: FeatureFlagResult.parse_payload(ff.payload),
+              enabled: enabled,
+              variant: ff.failed == true ? nil : ff.variant,
+              payload: ff.failed == true ? nil : FeatureFlagResult.parse_payload(ff.payload),
               id: metadata&.id,
               version: metadata&.version,
               reason: reason ? (reason.description || reason.code) : nil,
@@ -813,7 +816,7 @@ def evaluate_flags(
         errors_while_computing: errors_while_computing,
         quota_limited: quota_limited,
         minimal_flag_called_events: minimal_flag_called_events,
-        unresolved_flags: unresolved_flags.reject { |key, _reason| records.key?(key) }
+        unresolved_flags: unresolved_flags.reject { |key, _reason| records.key?(key) && !records[key].enabled.nil? }
       )
     end
 

lib/posthog/feature_flag_evaluations.rb

@@ -76,7 +76,10 @@ def initialize(
     )
       @host = host
       @distinct_id = distinct_id || ''
-      @flags = flags || {}
+      # Remote failures have no enabled value, unlike healthy disabled records.
+      failed, evaluated = (flags || {}).partition { |_, flag| flag.enabled.nil? }
+      @failed_flags = failed.to_h
+      @flags = evaluated.to_h
       @groups = groups
       @disable_geoip = disable_geoip
       @request_id = request_id
@@ -98,9 +101,8 @@ def keys
     # @param default_value [Boolean, nil] Returned when the flag has no value in this
     #   snapshot — it was never loaded, the `/flags` request failed, or no flag with that
     #   key exists. A flag that does have a value, including `false` and variant strings,
-    #   always wins over this default; so does a flag the server returned but marked as
-    #   failed, which resolves to `false`. Defaults to `nil`, which keeps the historical
-    #   `false` result for a missing flag.
+    #   always wins over this default. A failed remote record has no value. Defaults to
+    #   `nil`, which keeps the historical `false` result for a missing flag.
     # @return [Boolean] true when the flag is enabled, false when disabled, and the
     #   caller-supplied default (or false when none was supplied) when the flag is missing.
     def enabled?(key, default_value: nil)
@@ -177,7 +179,7 @@ def _get_event_properties
     # Variant string when present, else boolean enabled status; `nil` for
     # unknown flags.
     def _flag_value(flag)
-      return nil if flag.nil?
+      return nil if flag.nil? || flag.enabled.nil?
       return flag.variant if flag.variant
 
       flag.enabled ? true : false
@@ -187,6 +189,8 @@ def _record_access(key, flag)
       @accessed.add(key)
       return if @distinct_id.nil? || @distinct_id.to_s.empty?
 
+      failed = @failed_flags.key?(key)
+      flag ||= @failed_flags[key]
       response = _flag_value(flag)
       properties = {
         '$feature_flag' => key,
@@ -212,7 +216,9 @@ def _record_access(key, flag)
       errors = []
       errors << 'errors_while_computing_flags' if @errors_while_computing
       errors << 'quota_limited' if @quota_limited
-      if flag.nil? && @unresolved_flags.key?(key)
+      if failed
+        errors << 'evaluation_failed'
+      elsif flag.nil? && @unresolved_flags.key?(key)
         errors << FeatureFlagError::LOCAL_EVALUATION_INCONCLUSIVE
       elsif flag.nil?
         errors << FeatureFlagError::FLAG_MISSING

lib/posthog/feature_flags.rb

@@ -302,14 +302,19 @@ def get_feature_flag(
           errors << FeatureFlagError.api_error(status) if status && status >= 400
           errors << FeatureFlagError::ERRORS_WHILE_COMPUTING if flags_data[:errorsWhileComputingFlags]
           errors << FeatureFlagError::QUOTA_LIMITED if flags_data[:quotaLimited]&.include?('feature_flags')
-          errors << FeatureFlagError::FLAG_MISSING unless flags.key?(key.to_s)
+          flag_detail = flags_data[:flagDetails]&.[](key.to_sym)
+          failed = flag_detail&.failed == true && !flags.key?(key)
+          if failed
+            errors << 'evaluation_failed'
+          elsif !flags.key?(key)
+            errors << FeatureFlagError::FLAG_MISSING
+          end
 
           response = flags[key]
-          response = false if response.nil?
+          response = false if response.nil? && !failed
           payload = payloads[key]
           feature_flag_error = errors.join(',') unless errors.empty?
 
-          flag_detail = flags_data[:flagDetails]&.[](key.to_sym)
           has_experiment = flag_detail&.metadata&.has_experiment
           minimal_flag_called_events = flags_data[:minimalFlagCalledEvents]
 
@@ -422,8 +427,11 @@ def get_all_flags_and_payloads(
               flags = stringify_keys(flags).merge(server_flags)
               payloads = stringify_keys(payloads).merge(server_payloads)
             else
-              flags = server_flags
-              payloads = server_payloads
+              # A failed record must preserve a successful same-context local pair,
+              # independently of the response-wide error bit.
+              failed_keys = (flag_details || {}).select { |_, flag| flag.failed == true }.keys.map(&:to_s)
+              flags = stringify_keys(flags).slice(*failed_keys).merge(server_flags)
+              payloads = stringify_keys(payloads).slice(*failed_keys).merge(server_payloads)
             end
           end
         rescue StandardError => e

spec/posthog/failed_flag_records_spec.rb

@@ -0,0 +1,170 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+module PostHog
+  describe 'failed flag records (spec 211)' do
+    let(:flags_endpoint) { 'https://us.i.posthog.com/flags/?v=2' }
+    let(:definitions_endpoint) { 'https://us.i.posthog.com/flags/definitions?token=testsecret&send_cohorts=true' }
+    let(:client) { Client.new(api_key: API_KEY, test_mode: true) }
+
+    def failed_record(key, variant = nil)
+      {
+        key: key, enabled: false, variant: variant, failed: true,
+        reason: { code: 'dependency_error', description: 'Evaluation failed' },
+        metadata: { id: 1, version: 2, payload: '{"copy":"remote"}' }
+      }
+    end
+
+    def local_flag(key, value)
+      {
+        key: key, active: value != false,
+        filters: {
+          groups: [{ properties: [], rollout_percentage: 100, variant: value.is_a?(String) ? value : nil }],
+          multivariate: value.is_a?(String) ? { variants: [{ key: 'blue', rollout_percentage: 100 }] } : nil,
+          payloads: { value.to_s => '{"copy":"local"}' }
+        }
+      }
+    end
+
+    it 'does not label a retained successful fallback value as failed' do
+      poller = client.instance_variable_get(:@feature_flags_poller)
+      allow(poller).to receive(:get_all_flags_and_payloads).and_return(
+        featureFlags: { checkout: 'blue' }, featureFlagPayloads: { checkout: '{"copy":"local"}' },
+        flagDetails: { checkout: FeatureFlag.new(failed_record('checkout')) }, errorsWhileComputingFlags: true
+      )
+      result = client.get_feature_flag_result('checkout', 'user')
+      expect(result.value).to eq('blue')
+      expect(result.payload).to eq('copy' => 'local')
+      error = client.dequeue_last_message[:properties]['$feature_flag_error']
+      expect(error).to eq('errors_while_computing_flags')
+    end
+
+    it 'delivers failed diagnostics through the public getter and flush transport path' do
+      stub_request(:post, flags_endpoint).to_return(
+        status: 200, body: { flags: { checkout: failed_record('checkout', 'placeholder') },
+                             errorsWhileComputingFlags: true }.to_json
+      )
+      delivered = []
+      ingestion = stub_request(:post, 'https://us.i.posthog.com/batch/').to_return do |request|
+        delivered.concat(JSON.parse(request.body).fetch('batch'))
+        { status: 200, body: '{}' }
+      end
+      wire_client = Client.new(api_key: API_KEY, sync_mode: true, compress_request: false)
+      expect(wire_client.get_feature_flag_result('checkout', 'user')).to be_nil
+      expect(wire_client.flush).to be(true)
+      expect(ingestion).to have_been_requested.once
+      expect(delivered.size).to eq(1)
+      props = delivered.first.fetch('properties')
+      expect(props['$feature_flag_response']).to be_nil
+      expect(props['$feature_flag_error'].split(',')).to contain_exactly(
+        'evaluation_failed', 'errors_while_computing_flags'
+      )
+    ensure
+      wire_client&.shutdown
+    end
+
+    [nil, false, true].each do |error_bit|
+      [nil, 'placeholder'].each do |variant|
+        context "with global errors #{error_bit.inspect} and failed variant #{variant.inspect}" do
+          let(:response) do
+            result = {
+              flags: {
+                checkout: failed_record('checkout', variant),
+                off: { key: 'off', enabled: false, failed: false }
+              },
+              featureFlags: { checkout: 'mirror' }, featureFlagPayloads: { checkout: '"mirror"' }
+            }
+            result[:errorsWhileComputingFlags] = error_bit unless error_bit.nil?
+            result
+          end
+
+          before do
+            stub_request(:post, flags_endpoint).to_return(status: 200, body: response.to_json)
+          end
+
+          it 'returns no rich/scalar result and reports failure with the missing response, dedupe, and suppression' do
+            expect(client.get_feature_flag_result('checkout', 'user', send_feature_flag_events: false)).to be_nil
+            expect(client.queued_messages).to eq(0)
+            expect(client.get_feature_flag_result('checkout', 'user')).to be_nil
+            expect(client.get_feature_flag('checkout', 'user')).to be_nil
+            expect(client.is_feature_enabled('checkout', 'user')).to be_nil
+            expect(client.queued_messages).to eq(1)
+            event = client.dequeue_last_message
+            expect(event[:properties]['$feature_flag_response']).to be_nil
+            errors = event[:properties]['$feature_flag_error'].split(',')
+            expect(errors).to include('evaluation_failed')
+            expect(errors).not_to include('flag_missing')
+            expect(errors.include?('errors_while_computing_flags')).to eq(error_bit == true)
+            expect(client.get_feature_flag_result('off', 'user').enabled?).to be(false)
+            healthy_error = client.dequeue_last_message[:properties]['$feature_flag_error'].to_s
+            expect(healthy_error).not_to include('evaluation_failed')
+            combined = client.get_all_flags_and_payloads('user')
+            expect(combined[:featureFlags]).to eq('off' => false)
+            expect(combined[:featureFlagPayloads]).not_to have_key('checkout')
+          end
+
+          it 'honors snapshot defaults, omits failed enrichment, and retains diagnostics' do
+            snapshot = client.evaluate_flags('user')
+            expect(snapshot.enabled?('checkout', default_value: true)).to be(true)
+            expect(snapshot.get_flag('checkout')).to be_nil
+            expect(snapshot.get_flag_payload('checkout')).to be_nil
+            event = client.dequeue_last_message
+            expect(event[:properties]['$feature_flag_response']).to be_nil
+            errors = event[:properties]['$feature_flag_error'].split(',')
+            expect(errors).to include('evaluation_failed')
+            expect(errors).not_to include('flag_missing')
+            expect(errors.include?('errors_while_computing_flags')).to eq(error_bit == true)
+            expect(snapshot.keys).to eq(['off'])
+            expect(snapshot.enabled?('off', default_value: true)).to be(false)
+            client.capture(distinct_id: 'user', event: 'enriched', flags: snapshot)
+            expect(client.dequeue_last_message[:properties]).to include('$feature/off' => false)
+            expect(snapshot._get_event_properties).not_to have_key('$feature/checkout')
+          end
+
+          it 'reports evaluation_failed after inconclusive local evaluation' do
+            definition = local_flag('checkout', true).merge(ensure_experience_continuity: true)
+            stub_request(:get, definitions_endpoint).to_return(status: 200, body: { flags: [definition] }.to_json)
+            local_client = Client.new(api_key: API_KEY, personal_api_key: API_KEY, test_mode: true)
+            expect(local_client.get_feature_flag_result('checkout', 'user')).to be_nil
+            props = local_client.dequeue_last_message[:properties]
+            expect(props['$feature_flag_error']).to include('evaluation_failed')
+            expect(props['$feature_flag_error']).not_to include('flag_missing')
+            expect(props['$feature_flag_response']).to be_nil
+          ensure
+            local_client&.shutdown
+          end
+        end
+      end
+
+      it "preserves local true/false/variant pairs and capture enrichment with global errors #{error_bit.inspect}" do
+        values = { 'on' => true, 'off' => false, 'variant' => 'blue', 'success' => true }
+        definitions = values.map { |key, value| local_flag(key, value) }
+        definitions << local_flag('checkout', true).merge(ensure_experience_continuity: true)
+        stub_request(:get, definitions_endpoint).to_return(status: 200, body: { flags: definitions }.to_json)
+        remote = { flags: (values.keys + ['checkout']).to_h { |key| [key, failed_record(key, 'placeholder')] } }
+        remote[:flags]['success'] = {
+          key: 'success', enabled: true, variant: 'remote', metadata: { payload: '{"copy":"success"}' }
+        }
+        remote[:errorsWhileComputingFlags] = error_bit unless error_bit.nil?
+        stub_request(:post, flags_endpoint).to_return(status: 200, body: remote.to_json)
+        local_client = Client.new(api_key: API_KEY, personal_api_key: API_KEY, test_mode: true)
+        result = local_client.get_all_flags_and_payloads('user')
+        expect(result[:featureFlags]).to eq(values.merge('success' => 'remote'))
+        expected_payloads = values.to_h do |key, _|
+          [key, key == 'success' ? '{"copy":"success"}' : '{"copy":"local"}']
+        end
+        expect(result[:featureFlagPayloads]).to eq(expected_payloads)
+        local_client.capture(distinct_id: 'user', event: 'enriched', send_feature_flags: true)
+        props = local_client.dequeue_last_message[:properties]
+        expect(props).to include('$feature/on' => true, '$feature/off' => false, '$feature/variant' => 'blue')
+        expect(props).not_to have_key('$feature/checkout')
+        expect(local_client.get_feature_flag_result('on', 'user').enabled?).to be(true)
+        local_error = local_client.dequeue_last_message[:properties]['$feature_flag_error'].to_s
+        expect(local_error).not_to include('evaluation_failed')
+      ensure
+        local_client&.shutdown
+      end
+    end
+  end
+end

spec/posthog/feature_flag_error_spec.rb

@@ -302,12 +302,11 @@ module PostHog
           stub_request(:post, flags_endpoint)
             .to_return(status: 200, body: flags_response.to_json)
 
-          # The failed flag has enabled=true, but should be filtered out and treated as missing.
-          # Without filtering, this would return true.
+          # The failed flag has no evaluated value, despite its enabled placeholder.
           result = client.get_feature_flag('my-flag', 'test-user')
-          expect(result).to eq(false)
+          expect(result).to be_nil
           captured_message = client.dequeue_last_message
-          expect(captured_message[:properties]['$feature_flag_error']).to include(FeatureFlagError::FLAG_MISSING)
+          expect(captured_message[:properties]['$feature_flag_error']).to include('evaluation_failed')
 
           # The non-failed flag should return its value normally
           good_result = client.get_feature_flag('good-flag', 'test-user')
@@ -335,9 +334,9 @@ module PostHog
 
           # Without filtering, this would return 'test-variant'.
           result = client.get_feature_flag('variant-flag', 'test-user')
-          expect(result).to eq(false)
+          expect(result).to be_nil
           captured_message = client.dequeue_last_message
-          expect(captured_message[:properties]['$feature_flag_error']).to include(FeatureFlagError::FLAG_MISSING)
+          expect(captured_message[:properties]['$feature_flag_error']).to include('evaluation_failed')
         end
 
         it 'excludes failed flags from get_all_flags results' do