feat: added open timeout

lib/experiment/persistent_http_client.rb

@@ -5,7 +5,7 @@ module AmplitudeExperiment
   # WARNING: these connections are not safe for concurrent requests. Callers
   # must synchronize requests per connection.
   class PersistentHttpClient
-    DEFAULT_OPTIONS = { read_timeout: 80 }.freeze
+    DEFAULT_OPTIONS = { open_timeout: 60, read_timeout: 80 }.freeze
 
     class << self
       # url: URI / String

lib/experiment/remote/client.rb

@@ -89,7 +89,7 @@ def fetch_async_v2(user, &callback)
     # @param [User] user
     def fetch_internal(user)
       @logger.debug("[Experiment] Fetching variants for user: #{user.as_json}")
-      do_fetch(user, @config.fetch_timeout_millis)
+      do_fetch(user, @config.connect_timeout_millis, @config.fetch_timeout_millis)
     rescue StandardError => e
       @logger.error("[Experiment] Fetch failed: #{e.message}")
       if should_retry_fetch?(e)
@@ -112,7 +112,7 @@ def retry_fetch(user)
       @config.fetch_retries.times do
         sleep(delay_millis.to_f / 1000.0)
         begin
-          return do_fetch(user, @config.fetch_retry_timeout_millis)
+          return do_fetch(user, @config.connect_timeout_millis, @config.fetch_retry_timeout_millis)
         rescue StandardError => e
           @logger.error("[Experiment] Retry failed: #{e.message}")
           err = e
@@ -123,16 +123,18 @@ def retry_fetch(user)
     end
 
     # @param [User] user
-    # @param [Integer] timeout_millis
-    def do_fetch(user, timeout_millis)
+    # @param [Integer] connect_timeout_millis
+    # @param [Integer] fetch_timeout_millis
+    def do_fetch(user, connect_timeout_millis, fetch_timeout_millis)
       start_time = Time.now
       user_context = add_context(user)
       headers = {
         'Authorization' => "Api-Key #{@api_key}",
         'Content-Type' => 'application/json;charset=utf-8'
       }
-      read_timeout = timeout_millis.to_f / 1000 if (timeout_millis.to_f / 1000) > 0
-      http = PersistentHttpClient.get(@uri, { read_timeout: read_timeout }, @api_key)
+      connect_timeout = connect_timeout_millis.to_f / 1000 if (connect_timeout_millis.to_f / 1000) > 0
+      read_timeout = fetch_timeout_millis.to_f / 1000 if (fetch_timeout_millis.to_f / 1000) > 0
+      http = PersistentHttpClient.get(@uri, { open_timeout: connect_timeout, read_timeout: read_timeout }, @api_key)
       request = Net::HTTP::Post.new(@uri, headers)
       request.body = user_context.to_json
       @logger.warn("[Experiment] encoded user object length #{request.body.length} cannot be cached by CDN; must be < 8KB") if request.body.length > 8000

lib/experiment/remote/config.rb

@@ -12,6 +12,10 @@ class RemoteEvaluationConfig
     # @return [Boolean] the value of server url
     attr_accessor :server_url
 
+    # The request connection open timeout, in milliseconds, used when fetching variants triggered by calling start() or setUser().
+    # @return [Integer] the value of connect_timeout_millis
+    attr_accessor :connect_timeout_millis
+
     # The request timeout, in milliseconds, used when fetching variants triggered by calling start() or setUser().
     # @return [Integer] the value of fetch_timeout_millis
     attr_accessor :fetch_timeout_millis
@@ -40,6 +44,8 @@ class RemoteEvaluationConfig
 
     # @param [Boolean] debug Set to true to log some extra information to the console.
     # @param [String] server_url The server endpoint from which to request variants.
+    # @param [Integer] connect_timeout_millis The request connection open timeout, in milliseconds, used when
+    #  fetching variants triggered by calling start() or setUser().
     # @param [Integer] fetch_timeout_millis The request timeout, in milliseconds, used when fetching variants
     #  triggered by calling start() or setUser().
     # @param [Integer] fetch_retries The number of retries to attempt before failing.
@@ -49,11 +55,12 @@ class RemoteEvaluationConfig
     #  greater than the max, the max is used for all subsequent retries.
     # @param [Float] fetch_retry_backoff_scalar Scales the minimum backoff exponentially.
     # @param [Integer] fetch_retry_timeout_millis The request timeout for retrying fetch requests.
-    def initialize(debug: false, server_url: DEFAULT_SERVER_URL, fetch_timeout_millis: 10_000, fetch_retries: 0,
+    def initialize(debug: false, server_url: DEFAULT_SERVER_URL, connect_timeout_millis: 60_000, fetch_timeout_millis: 10_000, fetch_retries: 0,
                    fetch_retry_backoff_min_millis: 500, fetch_retry_backoff_max_millis: 10_000,
                    fetch_retry_backoff_scalar: 1.5, fetch_retry_timeout_millis: 10_000)
       @debug = debug
       @server_url = server_url
+      @connect_timeout_millis = connect_timeout_millis
       @fetch_timeout_millis = fetch_timeout_millis
       @fetch_retries = fetch_retries
       @fetch_retry_backoff_min_millis = fetch_retry_backoff_min_millis

spec/experiment/remote/client_spec.rb

@@ -62,6 +62,16 @@ def self.test_fetch_v2_shared(response, test_user, variant_name, debug, expected
       test_fetch_shared response_without_payload, test_user, variant_name, false, Variant.new(key: 'on')
       test_fetch_shared response_with_value_without_payload, test_user, variant_name, false, Variant.new(value: 'on')
 
+      it 'open timeout failure' do
+        stub_request(:post, server_url)
+          .to_timeout
+        test_user = User.new(user_id: 'test_user')
+        client = RemoteEvaluationClient.new(api_key, RemoteEvaluationConfig.new(connect_timeout_millis: 1, fetch_retries: 1, debug: true))
+        variants = nil
+        expect { variants = client.fetch(test_user) }.to output(/Retrying fetch/).to_stdout_from_any_process
+        expect(variants).to eq({})
+      end
+
       it 'fetch timeout failure' do
         stub_request(:post, server_url)
           .to_timeout
@@ -146,6 +156,16 @@ def self.test_fetch_async_shared(response, test_user, variant_name, debug, expec
       test_fetch_v2_shared response_without_payload, test_user, variant_name, false, Variant.new(key: 'on')
       test_fetch_v2_shared response_with_value_without_payload, test_user, variant_name, false, Variant.new(value: 'on')
 
+      it 'fetch v2 open timeout failure' do
+        stub_request(:post, server_url)
+          .to_timeout
+        test_user = User.new(user_id: 'test_user')
+        client = RemoteEvaluationClient.new(api_key, RemoteEvaluationConfig.new(connect_timeout_millis: 1, fetch_retries: 1, debug: true))
+        variants = nil
+        expect { variants = client.fetch_v2(test_user) }.to output(/Retrying fetch/).to_stdout_from_any_process
+        expect(variants).to eq({})
+      end
+
       it 'fetch v2 timeout failure' do
         stub_request(:post, server_url)
           .to_timeout