Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(retry): better error #501

Open
wants to merge 9 commits into
base: v2stable
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions lib/algolia/error.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,15 @@ class AlgoliaError < StandardError
# Used when hosts are unreachable
#
class AlgoliaUnreachableHostError < AlgoliaError
attr_reader :errors

def initialize(message, errors = [])
errors.last&.tap do |last_error|
message += " Last error for #{last_error[:host]}: #{last_error[:error]}"
end
super(message)
@errors = errors
end
end

# An exception class raised when the REST API returns an error.
Expand Down
13 changes: 11 additions & 2 deletions lib/algolia/logger_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,17 @@ class LoggerHelper
# @param debug_file [nil|String] file used to output the logs
#
def self.create(debug_file = nil)
file = debug_file || (ENV['ALGOLIA_DEBUG'] ? File.new('debug.log') : $stderr)
instance = ::Logger.new file
file = debug_file

if file.nil? && ENV['ALGOLIA_DEBUG']
begin
file = File.new('debug.log', 'a+')
rescue Errno::EACCES, Errno::ENOENT => e
puts "Failed to open debug.log: #{e.message}. Falling back to $stderr."
end
end

instance = ::Logger.new(file || $stderr)
instance.progname = 'algolia'
instance
end
Expand Down
2 changes: 1 addition & 1 deletion lib/algolia/transport/retry_strategy.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module Algolia
module Transport
# Class RetryStatregy
# Class RetryStrategy
class RetryStrategy
include RetryOutcomeType

Expand Down
10 changes: 8 additions & 2 deletions lib/algolia/transport/transport.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ def write(method, path, body = {}, opts = {})
# @return [Response] response of the request
#
def request(call_type, method, path, body = {}, opts = {})
retry_errors = []

@retry_strategy.get_tryable_hosts(call_type).each do |host|
opts[:timeout] ||= get_timeout(call_type) * (host.retry_count + 1)
opts[:connect_timeout] ||= @config.connect_timeout * (host.retry_count + 1)
Expand All @@ -73,10 +75,14 @@ def request(call_type, method, path, body = {}, opts = {})
decoded_error = json_to_hash(response.error, @config.symbolize_keys)
raise AlgoliaHttpError.new(get_option(decoded_error, 'status'), get_option(decoded_error, 'message'))
end
return json_to_hash(response.body, @config.symbolize_keys) unless outcome == RETRY
if outcome == RETRY
retry_errors << {host: host.url, error: response.error}
else
return json_to_hash(response.body, @config.symbolize_keys)
end
end

raise AlgoliaUnreachableHostError, 'Unreachable hosts'
raise AlgoliaUnreachableHostError.new("Unreachable hosts.", retry_errors)
end

private
Expand Down
10 changes: 7 additions & 3 deletions test/algolia/unit/retry_strategy_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def test_resets_all_hosts_when_expired_according_to_write_type

describe 'All hosts are unreachable' do
def test_failure_when_all_hosts_are_down
stateful_hosts = ['0.0.0.0']
stateful_hosts = ['0.0.0.0', '1.0.0.0']
@config = Algolia::Search::Config.new(application_id: 'foo', api_key: 'bar', custom_hosts: stateful_hosts)
client = Algolia::Search::Client.create_with_config(@config)
index = client.init_index(get_test_index_name('failure'))
Expand All @@ -82,11 +82,15 @@ def test_failure_when_all_hosts_are_down
index.save_object({ objectID: 'one' })
end

assert_equal 'Unreachable hosts', exception.message
assert_includes exception.message, 'Unreachable hosts. Last error for 1.0.0.0: SSL_connect'
assert_equal exception.errors, [
{:host=>"0.0.0.0", :error=>"SSL_connect SYSCALL returned=5 errno=0 peeraddr=127.0.0.1:443 state=error: certificate verify failed"},
otomatik marked this conversation as resolved.
Show resolved Hide resolved
{:host=>"1.0.0.0", :error=>"SSL_connect returned=1 errno=0 peeraddr=1.0.0.0:443 state=error: ssl/tls alert handshake failure"}
]
end
end

describe 'retry stategy decisions' do
describe 'retry strategy decisions' do
def before_all
super
@app_id = 'app_id'
Expand Down