From 1819e16f1981edf6d40ac05be99c301f52aa4309 Mon Sep 17 00:00:00 2001 From: Steven Bazyl Date: Wed, 22 Jan 2014 13:54:06 -0800 Subject: [PATCH] Add retry support & redirect following --- Gemfile | 1 + lib/google/api_client.rb | 85 +++++++++++++++--------- lib/google/api_client/errors.rb | 11 +++ spec/google/api_client/discovery_spec.rb | 17 +++++ spec/google/api_client/service_spec.rb | 6 ++ spec/google/api_client_spec.rb | 84 +++++++++++++++++++++++ 6 files changed, 172 insertions(+), 32 deletions(-) diff --git a/Gemfile b/Gemfile index 0397f1c1a..8b4cb4c17 100644 --- a/Gemfile +++ b/Gemfile @@ -10,6 +10,7 @@ gem 'faraday', '>= 0.9.0' gem 'multi_json', '>= 1.0.0' gem 'extlib', '>= 0.9.15' gem 'jwt', '~> 0.1.5' +gem 'retriable', '>= 1.4' gem 'jruby-openssl', :platforms => :jruby group :development do diff --git a/lib/google/api_client.rb b/lib/google/api_client.rb index bb82000c3..1d8b170e0 100644 --- a/lib/google/api_client.rb +++ b/lib/google/api_client.rb @@ -17,6 +17,7 @@ require 'faraday' require 'multi_json' require 'compat/multi_json' require 'stringio' +require 'retriable' require 'google/api_client/version' require 'google/api_client/logging' @@ -107,6 +108,7 @@ module Google self.auto_refresh_token = options.fetch(:auto_refresh_token) { true } self.key = options[:key] self.user_ip = options[:user_ip] + self.retries = options.fetch(:retries) { 0 } @discovery_uris = {} @discovery_documents = {} @discovered_apis = {} @@ -230,6 +232,13 @@ module Google # The base path. Should almost always be '/discovery/v1'. attr_accessor :discovery_path + ## + # Number of times to retry on recoverable errors + # + # @return [FixNum] + # Number of retries + attr_accessor :retries + ## # Returns the URI for the directory document. # @@ -424,6 +433,8 @@ module Google # Verifies an ID token against a server certificate. Used to ensure that # an ID token supplied by an untrusted client-side mechanism is valid. # Raises an error if the token is invalid or missing. + # + # @deprecated Use the google-id-token gem for verifying JWTs def verify_id_token! require 'jwt' require 'openssl' @@ -533,6 +544,8 @@ module Google # authenticated, `false` otherwise. # - (TrueClass, FalseClass) :gzip (default: true) - # `true` if gzip enabled, `false` otherwise. + # - (FixNum) :retries - + # # of times to retry on recoverable errors # # @return [Google::APIClient::Result] The result from the API, nil if batch. # @@ -547,7 +560,7 @@ module Google # ) # # @see Google::APIClient#generate_request - def execute(*params) + def execute!(*params) if params.first.kind_of?(Google::APIClient::Request) request = params.shift options = params.shift || {} @@ -572,53 +585,60 @@ module Google request.headers['User-Agent'] ||= '' + self.user_agent unless self.user_agent.nil? request.headers['Accept-Encoding'] ||= 'gzip' unless options[:gzip] == false + request.headers['Content-Type'] ||= '' request.parameters['key'] ||= self.key unless self.key.nil? request.parameters['userIp'] ||= self.user_ip unless self.user_ip.nil? 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], + :interval => lambda {|attempts| (2 ** attempts) + Random.rand} do + result = request.send(connection, true) - result = request.send(connection) - 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! - result = request.send(connection, true) - rescue Signet::AuthorizationError - # Ignore since we want the original error + case result.status + when 200...300 + result + when 301, 302, 303, 307 + request = generate_request(request.to_hash.merge({ + :uri => result.headers['location'], + :api_method => nil + })) + 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) + else + raise TransmissionError.new(result.error_message || "A transmission error has occurred", result) end end - - return result end ## - # Same as Google::APIClient#execute, but raises an exception if there was - # an error. + # Same as Google::APIClient#execute!, but does not raise an exception for + # normal API errros. # # @see Google::APIClient#execute - def execute!(*params) - result = self.execute(*params) - if result.error? - error_message = result.error_message - case result.response.status - when 400...500 - exception_type = ClientError - error_message ||= "A client error has occurred." - when 500...600 - exception_type = ServerError - error_message ||= "A server error has occurred." - else - exception_type = TransmissionError - error_message ||= "A transmission error has occurred." - end - raise exception_type, error_message + def execute(*params) + begin + return self.execute!(*params) + rescue TransmissionError => e + return e.result end - return result end - + protected - + ## # Resolves a URI template against the client's configured base. # @@ -646,6 +666,7 @@ module Google end end + end require 'google/api_client/version' diff --git a/lib/google/api_client/errors.rb b/lib/google/api_client/errors.rb index c85eeec7c..f12ad6daa 100644 --- a/lib/google/api_client/errors.rb +++ b/lib/google/api_client/errors.rb @@ -19,6 +19,17 @@ module Google # An error which is raised when there is an unexpected response or other # transport error that prevents an operation from succeeding. class TransmissionError < StandardError + attr_reader :result + def initialize(message = nil, result = nil) + super(message) + @result = result + end + end + + ## + # An exception that is raised if a redirect is required + # + class RedirectError < TransmissionError end ## diff --git a/spec/google/api_client/discovery_spec.rb b/spec/google/api_client/discovery_spec.rb index fb1927b30..0b0f89c22 100644 --- a/spec/google/api_client/discovery_spec.rb +++ b/spec/google/api_client/discovery_spec.rb @@ -89,6 +89,7 @@ describe Google::APIClient do conn = stub_connection do |stub| stub.get('/discovery/v1/apis/prediction/v1.2/rest?userIp=127.0.0.1') do |env| + [200, {}, '{}'] end end CLIENT.execute( @@ -104,6 +105,7 @@ describe Google::APIClient do CLIENT.key = 'qwerty' conn = stub_connection do |stub| stub.get('/discovery/v1/apis/prediction/v1.2/rest?key=qwerty') do |env| + [200, {}, '{}'] end end request = CLIENT.execute( @@ -120,6 +122,7 @@ describe Google::APIClient do CLIENT.user_ip = '127.0.0.1' conn = stub_connection do |stub| stub.get('/discovery/v1/apis/prediction/v1.2/rest?key=qwerty&userIp=127.0.0.1') do |env| + [200, {}, '{}'] end end request = CLIENT.execute( @@ -187,6 +190,7 @@ describe Google::APIClient do conn = stub_connection do |stub| stub.post('/prediction/v1.2/training?data=12345') do |env| env[:body].should == '' + [200, {}, '{}'] end end request = CLIENT.execute( @@ -204,6 +208,7 @@ describe Google::APIClient do # to a CGI-escaped semicolon (%3B) instead. stub.post('/prediction/v1.2/training?data=12345%3B67890') do |env| env[:body].should == '' + [200, {}, '{}'] end end request = CLIENT.execute( @@ -218,6 +223,7 @@ describe Google::APIClient do conn = stub_connection do |stub| stub.post('/prediction/v1.2/training?data=1&data=2') do |env| env.params['data'].should include('1', '2') + [200, {}, '{}'] end end request = CLIENT.execute( @@ -231,6 +237,7 @@ describe Google::APIClient do it 'should generate requests against the correct URIs' do conn = stub_connection do |stub| stub.post('/prediction/v1.2/training?data=12345') do |env| + [200, {}, '{}'] end end request = CLIENT.execute( @@ -244,6 +251,7 @@ describe Google::APIClient do it 'should generate requests against the correct URIs' do conn = stub_connection do |stub| stub.post('/prediction/v1.2/training?data=12345') do |env| + [200, {}, '{}'] end end request = CLIENT.execute( @@ -264,6 +272,7 @@ describe Google::APIClient do conn = stub_connection do |stub| stub.post('/prediction/v1.2/training') do |env| env[:url].host.should == 'testing-domain.example.com' + [200, {}, '{}'] end end @@ -284,6 +293,7 @@ describe Google::APIClient do stub.post('/prediction/v1.2/training?data=12345') do |env| env[:request_headers].should have_key('Authorization') env[:request_headers]['Authorization'].should =~ /^OAuth/ + [200, {}, '{}'] end end @@ -303,6 +313,7 @@ describe Google::APIClient do stub.post('/prediction/v1.2/training?data=12345') do |env| env[:request_headers].should have_key('Authorization') env[:request_headers]['Authorization'].should =~ /^Bearer/ + [200, {}, '{}'] end end @@ -363,6 +374,7 @@ describe Google::APIClient do stub.post('/prediction/v1.2/training') do |env| env[:request_headers].should have_key('Content-Type') env[:request_headers]['Content-Type'].should == 'application/json' + [200, {}, '{}'] end end CLIENT.authorization = :oauth_2 @@ -415,6 +427,7 @@ describe Google::APIClient do it 'should generate requests against the correct URIs' do conn = stub_connection do |stub| stub.get('/plus/v1/people/107807692475771887386/activities/public') do |env| + [200, {}, '{}'] end end @@ -485,6 +498,7 @@ describe Google::APIClient do it 'should generate requests against the correct URIs' do conn = stub_connection do |stub| stub.get('/adsense/v1.3/adclients') do |env| + [200, {}, '{}'] end end request = CLIENT.execute( @@ -515,6 +529,7 @@ describe Google::APIClient do it 'should succeed when validating parameters in a correct call' do conn = stub_connection do |stub| stub.get('/adsense/v1.3/reports?dimension=DATE&endDate=2010-01-01&metric=PAGE_VIEWS&startDate=2000-01-01') do |env| + [200, {}, '{}'] end end (lambda do @@ -553,6 +568,7 @@ describe Google::APIClient do stub.get('/adsense/v1.3/reports?dimension=DATE&dimension=PRODUCT_CODE'+ '&endDate=2010-01-01&metric=CLICKS&metric=PAGE_VIEWS&'+ 'startDate=2000-01-01') do |env| + [200, {}, '{}'] end end (lambda do @@ -593,6 +609,7 @@ describe Google::APIClient do 'startDate=2000-01-01') do |env| env.params['dimension'].should include('DATE', 'PRODUCT_CODE') env.params['metric'].should include('CLICKS', 'PAGE_VIEWS') + [200, {}, '{}'] end end request = CLIENT.execute( diff --git a/spec/google/api_client/service_spec.rb b/spec/google/api_client/service_spec.rb index e322797c6..b5cb3d058 100644 --- a/spec/google/api_client/service_spec.rb +++ b/spec/google/api_client/service_spec.rb @@ -49,6 +49,7 @@ describe Google::APIClient::Service do it 'should make a valid call for a method with no parameters' do conn = stub_connection do |stub| stub.get('/adsense/v1.3/adclients') do |env| + [200, {}, '{}'] end end adsense = Google::APIClient::Service.new( @@ -69,6 +70,7 @@ describe Google::APIClient::Service do it 'should make a valid call for a method with parameters' do conn = stub_connection do |stub| stub.get('/adsense/v1.3/adclients/1/adunits') do |env| + [200, {}, '{}'] end end adsense = Google::APIClient::Service.new( @@ -87,6 +89,7 @@ describe Google::APIClient::Service do it 'should make a valid call for a deep method' do conn = stub_connection do |stub| stub.get('/adsense/v1.3/accounts/1/adclients') do |env| + [200, {}, '{}'] end end adsense = Google::APIClient::Service.new( @@ -147,6 +150,7 @@ describe Google::APIClient::Service do conn = stub_connection do |stub| stub.post('/prediction/v1.5/trainedmodels?project=1') do |env| env.body.should == '{"id":"1"}' + [200, {}, '{}'] end end prediction = Google::APIClient::Service.new( @@ -167,6 +171,7 @@ describe Google::APIClient::Service do conn = stub_connection do |stub| stub.post('/prediction/v1.5/trainedmodels?project=1') do |env| env.body.should == '{"id":"1"}' + [200, {}, '{}'] end end prediction = Google::APIClient::Service.new( @@ -224,6 +229,7 @@ describe Google::APIClient::Service do conn = stub_connection do |stub| stub.post('/upload/drive/v1/files?uploadType=multipart') do |env| env.body.should be_a Faraday::CompositeReadIO + [200, {}, '{}'] end end drive = Google::APIClient::Service.new( diff --git a/spec/google/api_client_spec.rb b/spec/google/api_client_spec.rb index 937df7b82..40e8d7ed2 100644 --- a/spec/google/api_client_spec.rb +++ b/spec/google/api_client_spec.rb @@ -114,6 +114,7 @@ describe Google::APIClient do @connection = stub_connection do |stub| stub.post('/prediction/v1.2/training?data=12345') do |env| env[:request_headers]['Authorization'].should == 'Bearer 12345' + [200, {}, '{}'] end end end @@ -166,4 +167,87 @@ describe Google::APIClient do ) end end + + describe 'when retiries enabled' do + before do + client.retries = 2 + end + + after do + @connection.verify + end + + it 'should follow redirects' do + client.authorization = nil + @connection = stub_connection do |stub| + stub.get('/foo') do |env| + [302, {'location' => 'https://www.google.com/bar'}, '{}'] + end + stub.get('/bar') do |env| + [200, {}, '{}'] + end + end + + client.execute( + :uri => 'https://www.gogole.com/foo', + :connection => @connection + ) + end + + it 'should refresh tokens on 401 tokens' do + client.authorization.access_token = '12345' + expect(client.authorization).to receive(:fetch_access_token!) + + @connection = stub_connection do |stub| + stub.get('/foo') do |env| + [401, {}, '{}'] + end + stub.get('/foo') do |env| + [200, {}, '{}'] + end + end + + client.execute( + :uri => 'https://www.gogole.com/foo', + :connection => @connection + ) + end + + it 'should retry on 500 errors' do + client.authorization = nil + + @connection = stub_connection do |stub| + stub.get('/foo') do |env| + [500, {}, '{}'] + end + stub.get('/foo') do |env| + [200, {}, '{}'] + end + end + + client.execute( + :uri => 'https://www.gogole.com/foo', + :connection => @connection + ).status.should == 200 + + end + + it 'should fail after max retries' do + client.authorization = nil + count = 0 + @connection = stub_connection do |stub| + stub.get('/foo') do |env| + count += 1 + [500, {}, '{}'] + end + end + + client.execute( + :uri => 'https://www.gogole.com/foo', + :connection => @connection + ).status.should == 500 + count.should == 3 + end + + end end