diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 56fcd4d..b35c130 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -12,12 +12,16 @@ jobs: strategy: matrix: - ruby-version: ['3.1', '3.0', '2.7'] + ruby-version: + - '3.3' + - '3.2' + - '3.1' + - '3.0' steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Set up Ruby ${{ matrix.ruby-version }} - uses: ruby/setup-ruby@359bebbc29cbe6c87da6bc9ea3bc930432750108 + uses: ruby/setup-ruby@v1 with: ruby-version: ${{ matrix.ruby-version }} - name: Install dependencies diff --git a/CHANGELOG.md b/CHANGELOG.md index 66a7e49..1242fc3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +## 4.3.0 + +* Add support for disabling retries on timeout for `GET` and `HEAD` requests + ## 4.2.0 * Bump ruby required version from 2.6 to 2.7 diff --git a/Gemfile.lock b/Gemfile.lock index aba0913..57ccb92 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - grac (4.1.0) + grac (4.3.0) oj (~> 3.13.23) typhoeus (~> 1) @@ -15,8 +15,8 @@ GEM ffi (>= 1.15.0) ffi (1.15.5) oj (3.13.23) - rack (3.0.6.1) - rack-test (2.0.2) + rack (3.1.8) + rack-test (2.1.0) rack (>= 1.3) rake (13.0.1) rspec (3.12.0) @@ -42,8 +42,8 @@ DEPENDENCIES benchmark-ips (~> 2.10) builder (~> 3.2) grac! - rack (~> 3.0.1) - rack-test (~> 2.0.2) + rack (~> 3.1) + rack-test (~> 2.1) rake (~> 13.0) rspec (~> 3.12) diff --git a/README.md b/README.md index bdec5d3..397f30c 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,7 @@ Loading GeoIP information for `github.com`: require 'grac' # => true geoip_client = Grac::Client.new('http://freegeoip.net/json', timeout: 5) -# => #0.1, :timeout=>15, :params=>{}, :headers=>{"User-Agent"=>"Grac v2.X.X","Content-Type"=>"application/json;charset=utf-8"}, :postprocessing=>{}}> +# => #0.1, :timeout=>15, :params=>{}, :headers=>{"User-Agent"=>"Grac v4.X.X","Content-Type"=>"application/json;charset=utf-8"}, :postprocessing=>{}, :retry_get_head=>true}> geoip_client.path('/{host}', host: 'github.com').get # => {"ip"=>"8.8.8.8", "country_code"=>"US", "country_name"=>"United States", "region_code"=>"CA", "region_name"=>"California", "city"=>"Mountain View", "zip_code"=>"94040", "time_zone"=>"America/Los_Angeles", "latitude"=>37.3845, "longitude"=>-122.0881, "metro_code"=>807} ``` @@ -33,7 +33,7 @@ geoip_client.path('/does/not/exist').get ```ruby client = geoip_client.set(postprocessing: { '\A(latitude|longitude)\z' => -> (v) { v.to_i } }) -# => #0.1, :timeout=>5, :params=>{}, :headers=>{"User-Agent"=>"Grac v2.X.X","Content-Type"=>"application/json;charset=utf-8"}, :postprocessing=>{"\\A(latitude|longitude)\\z"=>#}}> +# => #0.1, :timeout=>5, :params=>{}, :headers=>{"User-Agent"=>"Grac v4.X.X","Content-Type"=>"application/json;charset=utf-8"}, :postprocessing=>{"\\A(latitude|longitude)\\z"=>#}, :retry_get_head=>true}> client.path('/github.com').get # => {"ip"=>"192.30.252.128", "country_code"=>"US", "country_name"=>"United States", "region_code"=>"CA", "region_name"=>"California", "city"=>"San Francisco", "zip_code"=>"94107", "time_zone"=>"America/Los_Angeles", "latitude"=>37, "longitude"=>-122, "metro_code"=>807} ``` @@ -64,9 +64,10 @@ Available options (shown are the default values): connecttimeout: 0.1, # in seconds timeout: 15, # in seconds params: {}, # default query parameters to be attached to the URL - headers: { "User-Agent" => "Grac v2.X.X", "Content-Type" => "application/json;charset=utf-8" }, + headers: { "User-Agent" => "Grac v4.X.X", "Content-Type" => "application/json;charset=utf-8" }, postprocessing: {}, # see below - middleware: [] # see below + middleware: [], # see below + retry_get_head: true, # retrying get and head requests on timeout once } ``` @@ -146,7 +147,7 @@ Grac allows you to override options and append to the URI by chaining calls to ` ```ruby client = Grac::Client.new("http://localhost:80", timeout: 1) -# => #0.1, :timeout=>1, :params=>{}, :headers=>{"User-Agent"=>"Grac v2.X.X","Content-Type"=>"application/json;charset=utf-8"}, :postprocessing=>{}}> +# => #0.1, :timeout=>1, :params=>{}, :headers=>{"User-Agent"=>"Grac v4.X.X","Content-Type"=>"application/json;charset=utf-8"}, :postprocessing=>{}, :retry_get_head=>true}> client.set(timeout: 20).path("/v1/users").get(per_page: 1000) # => [...] ``` diff --git a/grac.gemspec b/grac.gemspec index edb56cf..09db18d 100644 --- a/grac.gemspec +++ b/grac.gemspec @@ -23,8 +23,8 @@ Gem::Specification.new do |spec| spec.add_development_dependency 'rspec', '~> 3.12' spec.add_development_dependency 'builder', '~> 3.2' spec.add_development_dependency 'benchmark-ips', '~> 2.10' - spec.add_development_dependency 'rack', '~> 3.0.1' - spec.add_development_dependency 'rack-test', '~> 2.0.2' + spec.add_development_dependency 'rack', '~> 3.1' + spec.add_development_dependency 'rack-test', '~> 2.1' spec.add_runtime_dependency 'oj', '~> 3.13.23' spec.add_runtime_dependency 'typhoeus', '~> 1' diff --git a/lib/grac/client.rb b/lib/grac/client.rb index 34f9f43..ee29558 100644 --- a/lib/grac/client.rb +++ b/lib/grac/client.rb @@ -8,6 +8,7 @@ module Grac class Client + attr_reader :uri def initialize(uri, options = {}) @@ -15,21 +16,22 @@ def initialize(uri, options = {}) @uri = uri @options = { - :connecttimeout => options[:connecttimeout] || 0.1, - :timeout => options[:timeout] || 15, - :params => options[:params] || {}, - :headers => { - "User-Agent" => "Grac v#{Grac::VERSION}", - "Content-Type" => "application/json;charset=utf-8" + connecttimeout: options[:connecttimeout] || 0.1, + timeout: options[:timeout] || 15, + params: options[:params] || {}, + headers: { + 'User-Agent' => "Grac v#{Grac::VERSION}", + 'Content-Type' => 'application/json;charset=utf-8' }.merge(options[:headers] || {}), - :postprocessing => {}, - :middleware => options[:middleware] || [] + postprocessing: {}, + middleware: options[:middleware] || [], + retry_get_head: options.fetch(:retry_get_head, true), } if options[:postprocessing] options[:postprocessing] .each_with_object(postprocessing = {}) do |(pattern, transformation), obj| - if pattern.kind_of?(Regexp) + if pattern.is_a?(Regexp) obj[pattern] = transformation else obj[Regexp.new(pattern)] = transformation @@ -49,10 +51,12 @@ def initialize(uri, options = {}) end def set(options = {}) - options = options.merge({ - headers: @options[:headers].merge(options[:headers] || {}), - middleware: @options[:middleware] + (options[:middleware] || []) - }) + options = options.merge( + { + headers: @options[:headers].merge(options[:headers] || {}), + middleware: @options[:middleware] + (options[:middleware] || []), + }, + ) self.class.new(@uri, @options.merge(options)) end @@ -64,35 +68,38 @@ def path(path, variables = {}) self.class.new("#{@uri}#{path}", @options) end - %w{post put patch}.each do |method| + ['post', 'put', 'patch'].each do |method| define_method method do |body = {}, params = {}| - response = build_and_run(method, { :body => body, :params => params }) + response = build_and_run(method, { body: body, params: params }) check_response(method, response) end end - %w{get delete}.each do |method| + ['get', 'delete'].each do |method| define_method method do |params = {}| - response = build_and_run(method, { :params => params }) + response = build_and_run(method, { params: params }) check_response(method, response) end end def call(opts, request_uri, method, params, body) request_hash = { - :method => method, - :params => params, # Query params are escaped by Typhoeus - :body => body, - :connecttimeout => opts[:connecttimeout], - :timeout => opts[:timeout], - :headers => opts[:headers] + method: method, + params: params, # Query params are escaped by Typhoeus + body: body, + connecttimeout: opts[:connecttimeout], + timeout: opts[:timeout], + headers: opts[:headers], } request = ::Typhoeus::Request.new(request_uri, request_hash) response = request.run # Retry GET and HEAD requests - modifying requests might not be idempotent - response = request.run if response.timed_out? && ['get', 'head'].include?(method) + # Only retry those requests if the feature is enabled + if response.timed_out? && ['get', 'head'].include?(method) && @options[:retry_get_head] + response = request.run + end # A request can time out while receiving data. In this case response.code might indicate # success although data hasn't been fully transferred. Thus rely on Typhoeus for @@ -119,122 +126,123 @@ def call(opts, request_uri, method, params, body) private - def build_and_run(method, options = {}) - body = prepare_body_by_content_type(options[:body]) - params = @options[:params].merge(options[:params] || {}) - return middleware_chain.call(@options, uri, method, params, body) - end - - def headers - @options[:headers] || {} - end - - def prepare_body_by_content_type(body) - return nil if body.nil? || body.empty? - - case headers['Content-Type'] - when /\Aapplication\/json/ - return body.to_json - when /\Aapplication\/x-www-form-urlencoded/ - # Typhoeus will take care of the encoding when receiving a hash - return body - else - # Do not encode other unknown Content-Types either. - # The default is JSON through the Content-Type header which is set by default. - return body + def build_and_run(method, options = {}) + body = prepare_body_by_content_type(options[:body]) + params = @options[:params].merge(options[:params] || {}) + return middleware_chain.call(@options, uri, method, params, body) end - end - def middleware_chain - callee = self + def headers + @options[:headers] || {} + end - @options[:middleware].reverse.each do |mw| - if mw.kind_of?(Array) - middleware_class = mw[0] - params = mw[1..-1] + def prepare_body_by_content_type(body) + return nil if body.nil? || body.empty? - callee = middleware_class.new(callee, *params) + case headers['Content-Type'] + when /\Aapplication\/json/ + return body.to_json + when /\Aapplication\/x-www-form-urlencoded/ + # Typhoeus will take care of the encoding when receiving a hash + return body else - callee = mw.new(callee) + # Do not encode other unknown Content-Types either. + # The default is JSON through the Content-Type header which is set by default. + return body end end - return callee - end + def middleware_chain + callee = self - def check_response(method, response) - case response.code - when 200..203, 206..299 - # unknown status codes must be treated as the x00 of their class, so 200 - if response.json_content? - return postprocessing(response.parsed_json) - end + @options[:middleware].reverse.each do |mw| + if mw.kind_of?(Array) + middleware_class = mw[0] + params = mw[1..-1] - return response.body - when 204, 205 - return true - when 0 - raise Exception::RequestFailed.new(method, response.effective_url, response.return_message) - else - begin - # The Response class doesn't have enough information to create a proper exception, so - # catch its exception and raise a proper one. - parsed_body = response.parsed_json - rescue Exception::InvalidContent - raise Exception::ErrorWithInvalidContent.new( - method, - response.effective_url, - response.code, - response.body, - 'json' - ) - end - case response.code - when 400 - raise Exception::BadRequest.new(method, response.effective_url, parsed_body) - when 403 - raise Exception::Forbidden.new(method, response.effective_url, parsed_body) - when 404 - raise Exception::NotFound.new(method, response.effective_url, parsed_body) - when 409 - raise Exception::Conflict.new(method, response.effective_url, parsed_body) + callee = middleware_class.new(callee, *params) else - raise Exception::ServiceError.new(method, response.effective_url, parsed_body) + callee = mw.new(callee) end + end + + return callee end - end - def postprocessing(data, processing = nil) - return data if @options[:postprocessing].nil? || @options[:postprocessing].empty? + def check_response(method, response) + case response.code + when 200..203, 206..299 + # unknown status codes must be treated as the x00 of their class, so 200 + if response.json_content? + return postprocessing(response.parsed_json) + end + + return response.body + when 204, 205 + return true + when 0 + raise Exception::RequestFailed.new(method, response.effective_url, response.return_message) + else + begin + # The Response class doesn't have enough information to create a proper exception, so + # catch its exception and raise a proper one. + parsed_body = response.parsed_json + rescue Exception::InvalidContent + raise Exception::ErrorWithInvalidContent.new( + method, + response.effective_url, + response.code, + response.body, + 'json' + ) + end + case response.code + when 400 + raise Exception::BadRequest.new(method, response.effective_url, parsed_body) + when 403 + raise Exception::Forbidden.new(method, response.effective_url, parsed_body) + when 404 + raise Exception::NotFound.new(method, response.effective_url, parsed_body) + when 409 + raise Exception::Conflict.new(method, response.effective_url, parsed_body) + else + raise Exception::ServiceError.new(method, response.effective_url, parsed_body) + end + end + end + + def postprocessing(data, processing = nil) + return data if @options[:postprocessing].nil? || @options[:postprocessing].empty? - if data.kind_of?(Hash) - data.each do |key, value| - processing = nil - regexp = @options[:postprocessing].keys.detect { |pattern| pattern.match?(key) } + if data.kind_of?(Hash) + data.each do |key, value| + processing = nil + regexp = @options[:postprocessing].keys.detect { |pattern| pattern.match?(key) } - if !regexp.nil? - processing = @options[:postprocessing][regexp] + if !regexp.nil? + processing = @options[:postprocessing][regexp] + end + data[key] = postprocessing(value, processing) end - data[key] = postprocessing(value, processing) - end - elsif data.kind_of?(Array) - data.each_with_index do |value, index| - data[index] = postprocessing(value, processing) + elsif data.kind_of?(Array) + data.each_with_index do |value, index| + data[index] = postprocessing(value, processing) + end + else + data = processing.nil? ? data : processing.call(data) end - else - data = processing.nil? ? data : processing.call(data) + + return data end - return data - end + def escape_url_param(value) + # We don't want spaces to be encoded as plus sign - a plus sign can be ambiguous in a URL and + # either represent a plus sign or a space. + # CGI::escape replaces all plus signs with their percent-encoding representation, so all + # remaining plus signs are spaces. Replacing these with a space's percent encoding makes the + # encoding unambiguous. + CGI::escape(value).gsub('+', '%20') + end - def escape_url_param(value) - # We don't want spaces to be encoded as plus sign - a plus sign can be ambiguous in a URL and - # either represent a plus sign or a space. - # CGI::escape replaces all plus signs with their percent-encoding representation, so all - # remaining plus signs are spaces. Replacing these with a space's percent encoding makes the - # encoding unambiguous. - CGI::escape(value).gsub('+', '%20') - end end end diff --git a/lib/grac/version.rb b/lib/grac/version.rb index e79206b..51107a9 100644 --- a/lib/grac/version.rb +++ b/lib/grac/version.rb @@ -1,3 +1,3 @@ module Grac - VERSION = "4.2.0" + VERSION = "4.3.0" end diff --git a/spec/lib/grac/client_spec.rb b/spec/lib/grac/client_spec.rb index ef39553..29c4b9d 100644 --- a/spec/lib/grac/client_spec.rb +++ b/spec/lib/grac/client_spec.rb @@ -2,38 +2,49 @@ require 'bigdecimal' describe Grac::Client do - let(:grac) { described_class.new("http://localhost:80") } + let(:grac) do + described_class.new( + 'http://localhost:80', + options, + ) + end + + let(:options) do + {} + end def check_options(client, field, value) expect(client.instance_variable_get("@options")[field]).to eq(value) end context "#initialize" do - it "initializes the client with default values" do + it 'initializes the client with default values' do client = described_class.new("http://localhost:80") expect(client.instance_variable_get("@options")).to eq({ - :connecttimeout => 0.1, - :timeout => 15, - :params => {}, - :headers => { - "User-Agent" => "Grac v#{Grac::VERSION}", - "Content-Type" => "application/json;charset=utf-8" + connecttimeout: 0.1, + timeout: 15, + params: {}, + headers: { + 'User-Agent' => "Grac v#{Grac::VERSION}", + 'Content-Type' => 'application/json;charset=utf-8', }, - :postprocessing => {}, - :middleware => [] + postprocessing: {}, + middleware: [], + retry_get_head: true, }) - expect(client.uri).to eq("http://localhost:80") + expect(client.uri).to eq('http://localhost:80') end { - :connecttimeout => 0.4, - :timeout => 10, - :params => { "abc" => "def" }, - :headers => { "User-Agent" => "Test", "Content-Type" => "something" }, - :postprocessing => { "amount" => ->(value){ BigDecimal(value.to_s) } } + connecttimeout: 0.4, + timeout: 10, + params: { 'abc' => 'def' }, + headers: { 'User-Agent' => 'Test', 'Content-Type' => 'something' }, + postprocessing: { 'amount' => -> (value) { BigDecimal(value.to_s) } }, + retry_get_head: false, }.each do |param, value| it "allows setting the #{param}" do - client = described_class.new("http://localhost:80", param => value) + client = described_class.new('http://localhost:80', param => value) if param == :postprocessing (value, key) = *(value).flatten @@ -204,16 +215,46 @@ def check_options(client, field, value) .and_return(@request = double('request', url: request_uri)) end - context "the request timed out" do + context 'when the request timed out' do + context 'when the client is configured to not retry' do + let(:options) do + { + retry_get_head: false, + } + end + + it 'raises an exception if the retry was not successful either' do + expect(@request) + .to receive(:run) + .and_return( + response = double('response', body: body, return_message: 'msg'), + ) + expect(response).to receive(:timed_out?).twice.and_return(true) + + expect { + grac.call(opts, request_uri, method, params, body) + }.to raise_error( + ::Grac::Exception::ServiceTimeout, + "GET 'http://example.com' timed out: msg", + ) + end + end + it "raises an exception if the retry was not successful either" do - expect(@request).to receive(:run).twice.and_return( - response = double('response', body: body, return_message: "msg") - ) + expect(@request) + .to receive(:run) + .twice + .and_return( + response = double('response', body: body, return_message: 'msg'), + ) expect(response).to receive(:timed_out?).twice.and_return(true) - expect{ + expect { grac.call(opts, request_uri, method, params, body) - }.to raise_error(::Grac::Exception::ServiceTimeout, "GET 'http://example.com' timed out: msg") + }.to raise_error( + ::Grac::Exception::ServiceTimeout, + "GET 'http://example.com' timed out: msg", + ) end context "post" do