Skip to content
This repository
Browse code

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...
commit bfa522576637fed3725d5c73305eb23f996f0b0f 1 parent 0c8e067
Steve Bazyl authored January 30, 2014
40  lib/google/api_client.rb
@@ -119,7 +119,7 @@ def initialize(options={})
119 119
         faraday.ssl.ca_file = ca_file
120 120
         faraday.ssl.verify = true
121 121
         faraday.adapter Faraday.default_adapter
122  
-      end      
  122
+      end
123 123
       return self
124 124
     end
125 125
 
@@ -591,9 +591,12 @@ def execute!(*params)
591 591
 
592 592
       connection = options[:connection] || self.connection
593 593
       request.authorization = options[:authorization] || self.authorization unless options[:authenticated] == false
  594
+      
594 595
       tries = 1 + (options[:retries] || self.retries)
  596
+
595 597
       Retriable.retriable :tries => tries, 
596  
-                          :on => [TransmissionError], 
  598
+                          :on => [TransmissionError],
  599
+                          :on_retry => client_error_handler(request.authorization),
597 600
                           :interval => lambda {|attempts| (2 ** attempts) + rand} do
598 601
         result = request.send(connection, true)
599 602
 
@@ -607,14 +610,6 @@ def execute!(*params)
607 610
             }))
608 611
             raise RedirectError.new(result.headers['location'], result)
609 612
           when 400...500
610  
-            if result.status == 401 && request.authorization.respond_to?(:refresh_token) && auto_refresh_token
611  
-              begin
612  
-                logger.debug("Attempting refresh of access token & retry of request")
613  
-                request.authorization.fetch_access_token!
614  
-              rescue Signet::AuthorizationError
615  
-                 # Ignore since we want the original error
616  
-              end
617  
-            end
618 613
             raise ClientError.new(result.error_message || "A client error has occurred", result)
619 614
           when 500...600
620 615
             raise ServerError.new(result.error_message || "A server error has occurred", result)
@@ -665,6 +660,31 @@ def resolve_uri(template, mapping={})
665 660
       return Addressable::Template.new(@base_uri + template).expand(mapping)
666 661
     end
667 662
     
  663
+
  664
+    ##
  665
+    # Returns on proc for special processing of retries as not all client errors
  666
+    # are recoverable. Only 401s should be retried and only if the credentials
  667
+    # are refreshable
  668
+    #
  669
+    # @param [#fetch_access_token!] authorization
  670
+    #   OAuth 2 credentials
  671
+    # @return [Proc] 
  672
+    def client_error_handler(authorization)  
  673
+      can_refresh = authorization.respond_to?(:refresh_token) && auto_refresh_token 
  674
+      Proc.new do |exception, tries|
  675
+        next unless exception.kind_of?(ClientError)
  676
+        if exception.result.status == 401 && can_refresh && tries == 1
  677
+          begin
  678
+            logger.debug("Attempting refresh of access token & retry of request")
  679
+            authorization.fetch_access_token!
  680
+            next
  681
+          rescue Signet::AuthorizationError
  682
+          end
  683
+        end
  684
+        raise exception
  685
+      end
  686
+    end
  687
+
668 688
   end
669 689
 
670 690
 end
36  spec/google/api_client_spec.rb
@@ -168,7 +168,7 @@
168 168
      end
169 169
   end  
170 170
 
171  
-  describe 'when retiries enabled' do
  171
+  describe 'when retries enabled' do
172 172
     before do
173 173
       client.retries = 2
174 174
     end
@@ -213,6 +213,40 @@
213 213
       )
214 214
     end
215 215
 
  216
+
  217
+    it 'should not attempt multiple token refreshes' do
  218
+      client.authorization.access_token = '12345'
  219
+      expect(client.authorization).to receive(:fetch_access_token!).once
  220
+
  221
+      @connection = stub_connection do |stub|
  222
+        stub.get('/foo') do |env|
  223
+          [401, {}, '{}']
  224
+        end
  225
+      end
  226
+
  227
+      client.execute(  
  228
+        :uri => 'https://www.gogole.com/foo',
  229
+        :connection => @connection
  230
+      )
  231
+    end
  232
+
  233
+    it 'should not retry on client errors' do
  234
+      count = 0
  235
+      @connection = stub_connection do |stub|
  236
+        stub.get('/foo') do |env|
  237
+          count.should == 0
  238
+          count += 1
  239
+          [403, {}, '{}']
  240
+        end
  241
+      end
  242
+
  243
+      client.execute(  
  244
+        :uri => 'https://www.gogole.com/foo',
  245
+        :connection => @connection,
  246
+        :authenticated => false
  247
+      )
  248
+    end
  249
+
216 250
     it 'should retry on 500 errors' do
217 251
       client.authorization = nil
218 252
 

0 notes on commit bfa5225

Please sign in to comment.
Something went wrong with that request. Please try again.