fix: Retry flags requests on 502 and 504

.changeset/gentle-flags-retry.md

@@ -0,0 +1,6 @@
+---
+'posthog-ruby': patch
+'posthog-rails': patch
+---
+
+Retry remote feature flag requests after transient 502 and 504 responses.

lib/posthog/feature_flags.rb

@@ -36,8 +36,9 @@ class FeatureFlagsPoller
     # @param feature_flag_request_timeout_seconds [Integer] Timeout for feature flag requests.
     # @param on_error [Proc, nil] Callback invoked as `on_error.call(status, error)`.
     # @param flag_definition_cache_provider [Object, nil] Optional {FlagDefinitionCacheProvider} implementation.
-    # @param feature_flag_request_max_retries [Integer, nil] Retries after a transient network error on a flag
-    #   request. Defaults to {Defaults::FeatureFlags::FLAG_REQUEST_MAX_RETRIES}. Set to 0 to disable retrying.
+    # @param feature_flag_request_max_retries [Integer, nil] Retries after a transient network error or retryable
+    #   HTTP response status on a flag request. Defaults to {Defaults::FeatureFlags::FLAG_REQUEST_MAX_RETRIES}.
+    #   Set to 0 to disable retrying.
     def initialize(
       polling_interval,
       personal_api_key,
@@ -1234,7 +1235,12 @@ def _request_feature_flag_evaluation(data = {})
       data['token'] = @project_api_key
       req.body = data.to_json
 
-      _request(uri, req, @feature_flag_request_timeout_seconds)
+      _request(
+        uri,
+        req,
+        @feature_flag_request_timeout_seconds,
+        retry_status_codes: RETRYABLE_FLAGS_REQUEST_STATUS_CODES
+      )
     end
 
     def _request_remote_config_payload(flag_key)
@@ -1256,14 +1262,15 @@ def _request_remote_config_payload(flag_key)
       Errno::ECONNRESET,
       EOFError
     ].freeze
+    RETRYABLE_FLAGS_REQUEST_STATUS_CODES = [502, 504].freeze
 
-    def _request(uri, request_object, timeout = nil, include_etag: false)
+    def _request(uri, request_object, timeout = nil, include_etag: false, retry_status_codes: [])
       request_object['User-Agent'] = "posthog-ruby/#{PostHog::VERSION}"
       request_timeout = timeout || 10
       backoff_policy = nil
       attempts = 0
 
-      begin
+      loop do
         attempts += 1
         Net::HTTP.start(
           uri.hostname,
@@ -1277,6 +1284,16 @@ def _request(uri, request_object, timeout = nil, include_etag: false)
           status_code = res.code.to_i
           etag = include_etag ? res['ETag'] : nil
 
+          if retry_status_codes.include?(status_code) && attempts <= @feature_flag_request_max_retries
+            backoff_policy ||= BackoffPolicy.new
+            interval = backoff_policy.next_interval.to_f / 1000
+            logger.debug(
+              "Retrying request to #{_mask_tokens_in_url(uri.to_s)} after HTTP #{status_code} (attempt #{attempts})"
+            )
+            sleep(interval)
+            next
+          end
+
           # Handle 304 Not Modified - return special response indicating no change
           if status_code == 304
             logger.debug("#{request_object.method} #{_mask_tokens_in_url(uri.to_s)} returned 304 Not Modified")
@@ -1304,7 +1321,7 @@ def _request(uri, request_object, timeout = nil, include_etag: false)
           interval = backoff_policy.next_interval.to_f / 1000
           logger.debug("Retrying request to #{_mask_tokens_in_url(uri.to_s)} after #{e.class} (attempt #{attempts})")
           sleep(interval)
-          retry
+          next
         end
         logger.debug("Unable to complete request to #{_mask_tokens_in_url(uri.to_s)}")
         raise

spec/posthog/flags_spec.rb

@@ -253,6 +253,33 @@ module PostHog
         expect(poller).to have_received(:sleep).once
       end
 
+      [502, 504].each do |status|
+        it "retries once and succeeds after HTTP #{status}" do
+          stub_request(:post, flags_endpoint)
+            .to_return(status: status, body: { error: 'temporary' }.to_json).then
+            .to_return(status: 200, body: flags_response.to_json)
+
+          result = poller.get_flags('test-distinct-id')
+
+          expect(result[:status]).to eq(200)
+          expect(result[:featureFlags]).to eq({ 'my-flag': true })
+          expect(a_request(:post, flags_endpoint)).to have_been_made.times(2)
+          expect(poller).to have_received(:sleep).once
+        end
+      end
+
+      it 'does not retry other HTTP statuses' do
+        stub_request(:post, flags_endpoint)
+          .to_return(status: 500, body: { error: 'internal error' }.to_json).then
+          .to_return(status: 200, body: flags_response.to_json)
+
+        result = poller.get_flags('test-distinct-id')
+
+        expect(result).to eq({ error: 'internal error', status: 500 })
+        expect(a_request(:post, flags_endpoint)).to have_been_made.times(1)
+        expect(poller).not_to have_received(:sleep)
+      end
+
       it 'retries on Errno::ECONNRESET then re-raises once retries are exhausted' do
         stub_request(:post, flags_endpoint)
           .to_raise(Errno::ECONNRESET)
@@ -298,6 +325,18 @@ module PostHog
             expect(a_request(:post, flags_endpoint)).to have_been_made.times(1)
             expect(poller).not_to have_received(:sleep)
           end
+
+          it 'does not retry HTTP 502 responses' do
+            stub_request(:post, flags_endpoint)
+              .to_return(status: 502, body: { error: 'temporary' }.to_json).then
+              .to_return(status: 200, body: flags_response.to_json)
+
+            result = poller.get_flags('test-distinct-id')
+
+            expect(result).to eq({ error: 'temporary', status: 502 })
+            expect(a_request(:post, flags_endpoint)).to have_been_made.times(1)
+            expect(poller).not_to have_received(:sleep)
+          end
         end
 
         context 'set to a higher count' do
@@ -310,6 +349,19 @@ module PostHog
             expect { poller.get_flags('test-distinct-id') }.to raise_error(Errno::ECONNRESET)
             expect(a_request(:post, flags_endpoint)).to have_been_made.times(4)
           end
+
+          [502, 504].each do |status|
+            it "retries HTTP #{status} up to the configured number of times before returning the response" do
+              stub_request(:post, flags_endpoint)
+                .to_return(status: status, body: { error: 'temporary' }.to_json)
+
+              result = poller.get_flags('test-distinct-id')
+
+              expect(result).to eq({ error: 'temporary', status: status })
+              expect(a_request(:post, flags_endpoint)).to have_been_made.times(4)
+              expect(poller).to have_received(:sleep).exactly(3).times
+            end
+          end
         end
       end
     end