Permalink
Browse files

Tweak retry policy. 40x errors aren't typically recoverable other tha…

…n 401s in the case of expired access tokens. Even then, 1 retry is enough
  • Loading branch information...
1 parent 0c8e067 commit bfa522576637fed3725d5c73305eb23f996f0b0f @sqrrrl sqrrrl committed Jan 31, 2014
Showing with 65 additions and 11 deletions.
  1. +30 −10 lib/google/api_client.rb
  2. +35 −1 spec/google/api_client_spec.rb
View
@@ -119,7 +119,7 @@ def initialize(options={})
faraday.ssl.ca_file = ca_file
faraday.ssl.verify = true
faraday.adapter Faraday.default_adapter
- end
+ end
return self
end
@@ -591,9 +591,12 @@ def execute!(*params)
connection = options[:connection] || self.connection
request.authorization = options[:authorization] || self.authorization unless options[:authenticated] == false
+
tries = 1 + (options[:retries] || self.retries)
+
Retriable.retriable :tries => tries,
- :on => [TransmissionError],
+ :on => [TransmissionError],
+ :on_retry => client_error_handler(request.authorization),
:interval => lambda {|attempts| (2 ** attempts) + rand} do
result = request.send(connection, true)
@@ -607,14 +610,6 @@ def execute!(*params)
}))
raise RedirectError.new(result.headers['location'], result)
when 400...500
- if result.status == 401 && request.authorization.respond_to?(:refresh_token) && auto_refresh_token
- begin
- logger.debug("Attempting refresh of access token & retry of request")
- request.authorization.fetch_access_token!
- rescue Signet::AuthorizationError
- # Ignore since we want the original error
- end
- end
raise ClientError.new(result.error_message || "A client error has occurred", result)
when 500...600
raise ServerError.new(result.error_message || "A server error has occurred", result)
@@ -665,6 +660,31 @@ def resolve_uri(template, mapping={})
return Addressable::Template.new(@base_uri + template).expand(mapping)
end
+
+ ##
+ # Returns on proc for special processing of retries as not all client errors
+ # are recoverable. Only 401s should be retried and only if the credentials
+ # are refreshable
+ #
+ # @param [#fetch_access_token!] authorization
+ # OAuth 2 credentials
+ # @return [Proc]
+ def client_error_handler(authorization)
+ can_refresh = authorization.respond_to?(:refresh_token) && auto_refresh_token
+ Proc.new do |exception, tries|
+ next unless exception.kind_of?(ClientError)
+ if exception.result.status == 401 && can_refresh && tries == 1
+ begin
+ logger.debug("Attempting refresh of access token & retry of request")
+ authorization.fetch_access_token!
+ next
+ rescue Signet::AuthorizationError
+ end
+ end
+ raise exception
+ end
+ end
+
end
end
@@ -168,7 +168,7 @@
end
end
- describe 'when retiries enabled' do
+ describe 'when retries enabled' do
before do
client.retries = 2
end
@@ -213,6 +213,40 @@
)
end
+
+ it 'should not attempt multiple token refreshes' do
+ client.authorization.access_token = '12345'
+ expect(client.authorization).to receive(:fetch_access_token!).once
+
+ @connection = stub_connection do |stub|
+ stub.get('/foo') do |env|
+ [401, {}, '{}']
+ end
+ end
+
+ client.execute(
+ :uri => 'https://www.gogole.com/foo',
+ :connection => @connection
+ )
+ end
+
+ it 'should not retry on client errors' do
+ count = 0
+ @connection = stub_connection do |stub|
+ stub.get('/foo') do |env|
+ count.should == 0
+ count += 1
+ [403, {}, '{}']
+ end
+ end
+
+ client.execute(
+ :uri => 'https://www.gogole.com/foo',
+ :connection => @connection,
+ :authenticated => false
+ )
+ end
+
it 'should retry on 500 errors' do
client.authorization = nil

0 comments on commit bfa5225

Please sign in to comment.