Merge pull request #55 from Flagsmith/fix/squash_exceptions_for_polling_manager

fix: Squash exceptions for polling manager

Author: zachaysan

lib/flagsmith.rb

@@ -45,7 +45,7 @@ class Client # rubocop:disable Metrics/ClassLength
     #
     # :environment_key, :api_url, :custom_headers, :request_timeout_seconds, :enable_local_evaluation,
     # :environment_refresh_interval_seconds, :retries, :enable_analytics, :default_flag_handler,
-    # :offline_mode, :offline_handler
+    # :offline_mode, :offline_handler, :polling_manager_failure_limit
     #
     # You can see full description in the Flagsmith::Config
 
@@ -107,7 +107,7 @@ def environment_data_polling_manager
       update_environment if @environment_data_polling_manager.nil?
 
       @environment_data_polling_manager ||= Flagsmith::EnvironmentDataPollingManager.new(
-        self, environment_refresh_interval_seconds
+        self, environment_refresh_interval_seconds, @config.polling_manager_failure_limit
       ).tap(&:start)
     end
 

lib/flagsmith/sdk/config.rb

@@ -7,7 +7,7 @@ class Config
     OPTIONS = %i[
       environment_key api_url custom_headers request_timeout_seconds enable_local_evaluation
       environment_refresh_interval_seconds retries enable_analytics default_flag_handler
-      offline_mode offline_handler logger
+      offline_mode offline_handler polling_manager_failure_limit logger
     ].freeze
 
     # Available Configs
@@ -38,6 +38,8 @@ class Config
     #                                            bypasses requests to the api.
     #   +offline_handler+                      - A file object that contains a JSON serialization of
     #                                            the entire environment, project, flags, etc.
+    #   +polling_manager_failure_limit+        - An integer to control how long to suppress errors in
+    #                                            the polling manager for local evaluation mode.
     #   +logger+                               - Pass your logger, default is Logger.new($stdout)
     #
     attr_reader(*OPTIONS)
@@ -89,6 +91,7 @@ def build_config(options)
       @default_flag_handler = opts[:default_flag_handler]
       @offline_mode = opts.fetch(:offline_mode, false)
       @offline_handler = opts[:offline_handler]
+      @polling_manager_failure_limit = opts.fetch(:polling_manager_failure_limit, 10)
       @logger = options.fetch(:logger, Logger.new($stdout).tap { |l| l.level = :debug })
     end
     # rubocop:enable Metrics/AbcSize, Metrics/MethodLength

lib/flagsmith/sdk/pooling_manager.rb

@@ -7,20 +7,33 @@ module Flagsmith
   class EnvironmentDataPollingManager
     include Flagsmith::SDK::Intervals
 
-    def initialize(main, refresh_interval_seconds)
+    attr_reader :failures_since_last_update
+
+    def initialize(main, refresh_interval_seconds, polling_manager_failure_limit)
       @main = main
       @refresh_interval_seconds = refresh_interval_seconds
+      @polling_manager_failure_limit = polling_manager_failure_limit
+      @failures_since_last_update = 0
     end
 
+    # rubocop:disable Metrics/MethodLength
     def start
       update_environment = lambda {
         stop
-        @interval = set_interval(@refresh_interval_seconds) { @main.update_environment }
+        @interval = set_interval(@refresh_interval_seconds) do
+          @main.update_environment
+          @failures_since_last_update = 0
+        rescue StandardError => e
+          @failures_since_last_update += 1
+          @main.config.logger.warn "Failure to update the environment due to an error: #{e}"
+          raise e if @failures_since_last_update > @polling_manager_failure_limit
+        end
       }
 
       # TODO: this call should be awaited for getIdentityFlags/getEnvironmentFlags when enableLocalEvaluation is true
       update_environment.call
     end
+    # rubocop:enable Metrics/MethodLength
 
     def stop
       return unless @interval

spec/sdk/datapooling_manager_spec.rb

@@ -1,5 +1,4 @@
 # frozen_string_literal: true
-
 require 'spec_helper'
 
 require_relative 'shared_mocks.rb'
@@ -8,18 +7,18 @@
   include_context "shared mocks"
 
   let(:api_environment_response) { File.read('spec/sdk/fixtures/environment.json') }
-  let(:environemnt_response) { OpenStruct.new(body: JSON.parse(api_environment_response, symbolize_names: true)) }
+  let(:environment_response) { OpenStruct.new(body: JSON.parse(api_environment_response, symbolize_names: true)) }
   let(:refresh_interval_seconds) { 0.01 }
   let(:delay_time) { 0.045 }
 
   before(:each) do
     allow(Thread).to receive(:new).and_call_original
     allow(Thread).to receive(:kill).and_call_original
-    allow(mock_api_client).to receive(:get).with('environment-document/').and_return(environemnt_response)
+    allow(mock_api_client).to receive(:get).with('environment-document/').and_return(environment_response)
     allow(double(Flagsmith::Config)).to receive(:environment_url).and_return("environment-document/")
   end
 
-  subject { Flagsmith::EnvironmentDataPollingManager.new(flagsmith, refresh_interval_seconds) }
+  subject { Flagsmith::EnvironmentDataPollingManager.new(flagsmith, refresh_interval_seconds, 10) }
 
   it 'test_polling_manager_calls_update_environment_on_start' do
     times = (delay_time / refresh_interval_seconds).to_i
@@ -29,3 +28,54 @@
     subject.stop
   end
 end
+
+
+class FakeFlagsmith
+  attr_accessor :raise_error, :config
+
+  def initialize config
+    @config = config
+  end
+
+  def update_environment
+    if @raise_error
+      raise StandardError, "Some networking issue"
+    else
+      # Perform update logic
+    end
+  end
+end
+
+RSpec.describe Flagsmith::EnvironmentDataPollingManager do
+  include_context "shared mocks"
+
+  let(:refresh_interval_seconds) { 0.01 }
+  let(:delay_time) { 0.045 }
+  let(:update_failures_limit) { 5 }
+  let(:fake_flagsmith) { FakeFlagsmith.new mock_config }
+
+  before :each do
+    allow(mock_config).to receive(:logger).and_return(Logger.new($stdout))
+  end
+
+  subject { Flagsmith::EnvironmentDataPollingManager.new(fake_flagsmith, refresh_interval_seconds, update_failures_limit) }
+
+  it "operates under an error prone environment" do
+    fake_flagsmith.raise_error = true
+
+    # Four invocations are processed without the error bubbling up.
+    times = (delay_time / refresh_interval_seconds).to_i
+    subject.start
+    sleep delay_time
+    # Show that the failures are recorded without raising.
+    expect(subject.failures_since_last_update).to eq(4)
+
+    # Now set flagsmith to respond as normal.
+    fake_flagsmith.raise_error = false
+    sleep delay_time
+
+    # Now the exception count is back to zero.
+    expect(subject.failures_since_last_update).to eq(0)
+    subject.stop
+  end
+end

spec/sdk/flagsmith_spec.rb

@@ -111,13 +111,13 @@
 
   describe '#get_identity_segments' do
     it 'returns an empty list given an identity which matches no segment conditions' do
-      polling_manager = Flagsmith::EnvironmentDataPollingManager.new(subject, 60)
+      polling_manager = Flagsmith::EnvironmentDataPollingManager.new(subject, 60, 10)
       subject.update_environment()
       expect(subject.get_identity_segments("identifier")).to eq([])
     end
 
     it 'returns the relevant segment given an identity with matching traits' do
-      polling_manager = Flagsmith::EnvironmentDataPollingManager.new(subject, 60)
+      polling_manager = Flagsmith::EnvironmentDataPollingManager.new(subject, 60, 10)
       subject.update_environment()
       expect(subject.get_identity_segments("identifier", {"age": 39}).length).to eq(1)
     end