Use max_retries and give a bigger default timeout value

Author: brunoocasali

.rubocop.yml

@@ -23,3 +23,7 @@ Style/WordArray:
 
 Style/Documentation:
   Enabled: false
+
+Metrics/BlockLength:
+  Exclude:
+    - 'spec/**/*.rb'

.rubocop_todo.yml

@@ -1,22 +1,26 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2025-02-05 09:53:00 UTC using RuboCop version 1.69.2.
+# on 2025-03-31 12:39:31 UTC using RuboCop version 1.71.2.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
 # versions of RuboCop, may require this file to be generated again.
 
-# Offense count: 74
-# Configuration parameters: CountComments, CountAsOne, AllowedMethods, AllowedPatterns.
-# AllowedMethods: refine
-Metrics/BlockLength:
-  Max: 695
+# Offense count: 1
+# Configuration parameters: AllowedMethods, AllowedPatterns, CountRepeatedAttributes.
+Metrics/AbcSize:
+  Max: 21
 
 # Offense count: 4
 # Configuration parameters: CountComments, CountAsOne.
 Metrics/ClassLength:
   Max: 537
 
+# Offense count: 4
+# Configuration parameters: CountComments, CountAsOne, AllowedMethods, AllowedPatterns.
+Metrics/MethodLength:
+  Max: 15
+
 # Offense count: 3
 # Configuration parameters: Max, CountKeywordArgs.
 Metrics/ParameterLists:

lib/meilisearch/http_request.rb

@@ -10,8 +10,9 @@ class HTTPRequest
     attr_reader :options, :headers
 
     DEFAULT_OPTIONS = {
-      timeout: 1,
-      max_retries: 0,
+      timeout: 10,
+      max_retries: 2,
+      retry_multiplier: 1.2,
       convert_body?: true
     }.freeze
 
@@ -29,7 +30,8 @@ def http_get(relative_path = '', query_params = {}, options = {})
         config: {
           query_params: query_params,
           headers: remove_headers(@headers.dup.merge(options[:headers] || {}), 'Content-Type'),
-          options: @options.merge(options)
+          options: @options.merge(options),
+          method_type: :get
         }
       )
     end
@@ -42,7 +44,8 @@ def http_post(relative_path = '', body = nil, query_params = nil, options = {})
           query_params: query_params,
           body: body,
           headers: @headers.dup.merge(options[:headers] || {}),
-          options: @options.merge(options)
+          options: @options.merge(options),
+          method_type: :post
         }
       )
     end
@@ -55,7 +58,8 @@ def http_put(relative_path = '', body = nil, query_params = nil, options = {})
           query_params: query_params,
           body: body,
           headers: @headers.dup.merge(options[:headers] || {}),
-          options: @options.merge(options)
+          options: @options.merge(options),
+          method_type: :put
         }
       )
     end
@@ -68,7 +72,8 @@ def http_patch(relative_path = '', body = nil, query_params = nil, options = {})
           query_params: query_params,
           body: body,
           headers: @headers.dup.merge(options[:headers] || {}),
-          options: @options.merge(options)
+          options: @options.merge(options),
+          method_type: :patch
         }
       )
     end
@@ -80,7 +85,8 @@ def http_delete(relative_path = '', query_params = nil, options = {})
         config: {
           query_params: query_params,
           headers: remove_headers(@headers.dup.merge(options[:headers] || {}), 'Content-Type'),
-          options: @options.merge(options)
+          options: @options.merge(options),
+          method_type: :delete
         }
       )
     end
@@ -102,15 +108,23 @@ def remove_headers(data, *keys)
       data.delete_if { |k| keys.include?(k) }
     end
 
-    def send_request(http_method, relative_path, config: {})
-      config = http_config(config[:query_params], config[:body], config[:options], config[:headers])
+    def send_request(http_method, relative_path, config:)
+      attempts = 0
+      retry_multiplier = config.dig(:options, :retry_multiplier)
+      max_retries = config.dig(:options, :max_retries)
+      request_config = http_config(config[:query_params], config[:body], config[:options], config[:headers])
 
       begin
-        response = http_method.call(@base_url + relative_path, config)
+        response = http_method.call(@base_url + relative_path, request_config)
       rescue Errno::ECONNREFUSED, Errno::EPIPE => e
         raise CommunicationError, e.message
-      rescue Net::ReadTimeout, Net::OpenTimeout => e
-        raise TimeoutError, e.message
+      rescue Net::OpenTimeout, Net::ReadTimeout => e
+        attempts += 1
+        raise TimeoutError, e.message unless attempts <= max_retries && safe_to_retry?(config[:method_type], e)
+
+        sleep(retry_multiplier**attempts)
+
+        retry
       end
 
       validate(response)
@@ -132,5 +146,10 @@ def validate(response)
 
       response.parsed_response
     end
+
+    # Ensures the only retryable error is a timeout didn't reached the server
+    def safe_to_retry?(method_type, error)
+      method_type == :get || ([:post, :put, :patch, :delete].include?(method_type) && error.is_a?(Net::OpenTimeout))
+    end
   end
 end

spec/meilisearch/index/base_spec.rb

@@ -100,7 +100,7 @@
   end
 
   it 'supports options' do
-    options = { timeout: 2, max_retries: 1 }
+    options = { timeout: 2, retry_multiplier: 1.2, max_retries: 1 }
     expected_headers = {
       'Authorization' => "Bearer #{MASTER_KEY}",
       'User-Agent' => Meilisearch.qualified_version
@@ -109,7 +109,7 @@
     new_client = Meilisearch::Client.new(URL, MASTER_KEY, options)
     new_client.create_index('books').await
     index = new_client.fetch_index('books')
-    expect(index.options).to eq({ max_retries: 1, timeout: 2, convert_body?: true })
+    expect(index.options).to eq({ max_retries: 1, retry_multiplier: 1.2, timeout: 2, convert_body?: true })
 
     expect(described_class).to receive(:get).with(
       "#{URL}/indexes/books",
@@ -128,7 +128,7 @@
 
   it 'supports client_agents' do
     custom_agent = 'Meilisearch Rails (v0.0.1)'
-    options = { timeout: 2, max_retries: 1, client_agents: [custom_agent] }
+    options = { timeout: 2, retry_multiplier: 1.2, max_retries: 1, client_agents: [custom_agent] }
     expected_headers = {
       'Authorization' => "Bearer #{MASTER_KEY}",
       'User-Agent' => "#{custom_agent};#{Meilisearch.qualified_version}"

spec/meilisearch_spec.rb

@@ -35,6 +35,32 @@
     expect(new_client.headers).to have_key('User-Agent')
     expect(new_client.headers['User-Agent']).to eq(described_class.qualified_version)
   end
+
+  it 'retries the request when the request is retryable' do
+    allow(Meilisearch::HTTPRequest).to receive(:get).and_raise(Net::ReadTimeout)
+
+    begin
+      new_client = Meilisearch::Client.new(URL, MASTER_KEY, max_retries: 3, retry_multiplier: 0.1)
+      new_client.indexes
+    rescue Meilisearch::TimeoutError
+      nil
+    end
+
+    expect(Meilisearch::HTTPRequest).to have_received(:get).exactly(4).times
+  end
+
+  it 'does not retry the request when the request is not retryable' do
+    allow(Meilisearch::HTTPRequest).to receive(:get).and_raise(Errno::ECONNREFUSED)
+
+    begin
+      new_client = Meilisearch::Client.new(URL, MASTER_KEY, max_retries: 10)
+      new_client.indexes
+    rescue Meilisearch::CommunicationError
+      nil
+    end
+
+    expect(Meilisearch::HTTPRequest).to have_received(:get).once
+  end
 end
 
 RSpec.describe MeiliSearch do