diff --git a/hub_client.gemspec b/hub_client.gemspec index 3cc3732..e325202 100644 --- a/hub_client.gemspec +++ b/hub_client.gemspec @@ -25,4 +25,5 @@ Gem::Specification.new do |spec| spec.add_development_dependency "rspec" spec.add_development_dependency "webmock" spec.add_development_dependency "pry" + spec.add_development_dependency 'pry-byebug' end diff --git a/lib/hub_client.rb b/lib/hub_client.rb index 5cb32cc..226c56a 100644 --- a/lib/hub_client.rb +++ b/lib/hub_client.rb @@ -1,36 +1,56 @@ +require "hub_client/exponential_backoff_interval" require "hub_client/configuration" require "hub_client/logger" require "hub_client/version" require "rest-client" module HubClient + REQUEST_HEADERS = { + content_type: :json, + accept: :json, + } + def self.publish(metadata, content, env = nil) - raise ConfigArgumentMissing, "endpoint_url missing" unless HubClient.configuration.endpoint_url + config = HubClient.configuration + raise ConfigArgumentMissing, "endpoint_url missing" unless config.endpoint_url payload = (metadata.is_a?(String) || metadata.is_a?(Symbol)) ? { type: metadata.to_s } : metadata payload[:content] = content - payload[:env] ||= env || payload[:env] || payload['env'] || HubClient.configuration.env + payload[:env] ||= env || payload[:env] || payload['env'] || config.env + + retry_intervals = config.retry_intervals - hub_url = build_hub_url(HubClient.configuration.endpoint_url) + retries = 0 + begin + RestClient::Request.execute(request_opts(config, payload)) + rescue RestClient::Exception => e + HubClient.logger.warn("HubClient Exception #{e.class}: #{e.message} Code: #{e.http_code} Response: #{e.response} Request: #{payload}") - RestClient.post(hub_url, payload.to_json, content_type: :json, accept: :json) do |response, request, result| - handle_response(response, request, result) + retries += 1 + sleep_interval = retry_intervals && retry_intervals.next(retries) + raise unless sleep_interval + Kernel.sleep(sleep_interval) + retry end end private + def self.request_opts(config, payload) + { + method: :post, + url: build_hub_url(config.endpoint_url), + payload: payload.to_json, + headers: REQUEST_HEADERS, + timeout: config.timeout, + open_timeout: config.open_timeout, + }.reject {|_k,v| v.nil?} + end + def self.build_hub_url(endpoint_url) endpoint_url = endpoint_url.gsub(/\/$/, '') # remove last '/' if exists "#{endpoint_url}/api/#{HUB_VERSION}/messages" end - def self.handle_response(response, request, result) - # When request didn't succeed we log it - unless result.code.start_with?("2") - HubClient.logger.info("HubClient Code: #{result.code} Response: #{response} Request: #{request.args}") - end - end - class ConfigArgumentMissing < StandardError; end end diff --git a/lib/hub_client/configuration.rb b/lib/hub_client/configuration.rb index 6ecbdfd..c2e7ec9 100644 --- a/lib/hub_client/configuration.rb +++ b/lib/hub_client/configuration.rb @@ -1,6 +1,6 @@ module HubClient class Configuration - attr_accessor :env, :access_token, :endpoint_url + attr_accessor :env, :access_token, :endpoint_url, :retry_intervals, :open_timeout, :timeout end def self.configuration diff --git a/lib/hub_client/exponential_backoff_interval.rb b/lib/hub_client/exponential_backoff_interval.rb new file mode 100644 index 0000000..013e97b --- /dev/null +++ b/lib/hub_client/exponential_backoff_interval.rb @@ -0,0 +1,27 @@ +module HubClient + class ExponentialBackoffInterval + attr_accessor :initial, :multiplier, :rand_factor, :max_count + + DEFAULT_OPTS = { + initial: 0.5, + multiplier: 1.5, + rand_factor: 0.05, + max_count: 10, + } + + def initialize(opts = {}) + opts = DEFAULT_OPTS.merge(opts) + @initial = opts[:initial] + @multiplier = opts[:multiplier] + @rand_factor = opts[:rand_factor] + @max_count = opts[:max_count] + end + + def next(count) + return nil if count > max_count + result = @initial * (@multiplier ** count) + r = Kernel.rand(-@rand_factor..@rand_factor) + result + result * r + end + end +end \ No newline at end of file diff --git a/lib/hub_client/logger.rb b/lib/hub_client/logger.rb index fc5c6ee..ee2af6e 100644 --- a/lib/hub_client/logger.rb +++ b/lib/hub_client/logger.rb @@ -9,5 +9,6 @@ def method_missing(method_sym, *args, &block) def self.logger @logger ||= Logger.new + @logger end end \ No newline at end of file diff --git a/lib/hub_client/version.rb b/lib/hub_client/version.rb index bb71e01..b7e77d8 100644 --- a/lib/hub_client/version.rb +++ b/lib/hub_client/version.rb @@ -1,4 +1,4 @@ module HubClient - VERSION = "0.0.7" + VERSION = "0.0.8" HUB_VERSION = "v1" end diff --git a/spec/exponential_backoff_interval_spec.rb b/spec/exponential_backoff_interval_spec.rb new file mode 100644 index 0000000..f9e911c --- /dev/null +++ b/spec/exponential_backoff_interval_spec.rb @@ -0,0 +1,20 @@ +require 'spec_helper' +require "hub_client/exponential_backoff_interval" + +describe HubClient::ExponentialBackoffInterval do + let(:default_opts) {described_class.const_get(:DEFAULT_OPTS)} + + describe '#next' do + subject {described_class.new(initial: 12, max_count: 3)} + + context 'when called with 2' do + it 'returns a correct value' do + multiplier = default_opts[:multiplier] + med = 12 * (multiplier ** 2) + rand_factor = default_opts[:rand_factor] + expect(Kernel).to receive(:rand).with(-rand_factor..rand_factor) {0.1} + expect(subject.next(2)).to eq(med + 0.1*med) + end + end + end +end diff --git a/spec/hub_client_spec.rb b/spec/hub_client_spec.rb index e380037..5f9eacb 100644 --- a/spec/hub_client_spec.rb +++ b/spec/hub_client_spec.rb @@ -2,9 +2,13 @@ require 'hub_client' describe HubClient do + before do + HubClient.configure {|config| config.retry_intervals = nil} + end + describe "#publish" do context "not configured" do - after(:each) { HubClient.reset_configuration } + before {HubClient.reset_configuration} it "raises an error when endpoint_url is not configured" do HubClient.configure { |config| config.env = "il-qa2" } @@ -29,16 +33,6 @@ assert_requested :post, HubClient.build_hub_url(HubClient.configuration.endpoint_url) end - it "logs the request when hub didn't return success code" do - stub_request(:post, HubClient.build_hub_url(HubClient.configuration.endpoint_url)). - with(body: { env: "il-qa2", type: "order_created", content: { some: "content" } }). - to_return(status: 500) - - expect(HubClient.logger).to receive(:info) - HubClient.publish(:order_created, { some: "content" }) - assert_requested :post, HubClient.build_hub_url(HubClient.configuration.endpoint_url) - end - it "overrides the env when supplied" do stub_request(:post, HubClient.build_hub_url(HubClient.configuration.endpoint_url)). with(body: { env: "batman-env", type: "order_created", content: { some: "content" } }). @@ -67,6 +61,190 @@ end after(:all) { HubClient.reset_configuration } + + describe 'configured timeout' do + before do + allow(RestClient::Request).to receive(:new).and_call_original + stub_request(:post, HubClient.build_hub_url(HubClient.configuration.endpoint_url)) + end + + context 'when specified' do + before do + HubClient.configure do |config| + config.timeout = 31 + config.open_timeout = 33 + end + HubClient.publish(:order_created, { some: "content" }) + end + + it 'is passed to RestClient' do + expect(RestClient::Request).to have_received(:new).with(hash_including(timeout: 31, open_timeout: 33)) + end + end + + context 'when not specified' do + HubClient.reset_configuration + HubClient.configure do |config| + config.env = "il-qa2" + config.endpoint_url = "http://service-hub.com" + end + + before do + HubClient.publish(:order_created, { some: "content" }) + end + + it 'is not passed to RestClient' do + expect(RestClient::Request).to have_received(:new).with(hash_not_including(:timeout, :open_timeout)) + end + end + end + + describe 'exception handling' do + def action + HubClient.publish(:order_created, { some: "content" }) + end + + def stub_publish_request + @net_response = stub_request(:post, HubClient.build_hub_url(HubClient.configuration.endpoint_url)). + with(body: { env: "il-qa2", type: "order_created", content: { some: "content" } }) + end + + shared_examples_for 'raises the exception' do |exception_class| + it 'raises the exception' do + expect {action}.to raise_error(exception_class) + end + + it 'logs the exception' do + logger = double('logger', warn: nil) + allow(HubClient).to receive(:logger).and_return logger + expect(logger).to receive(:warn).with(/#{exception_class}/) + action rescue nil + end + end + + context 'when no retry_intervals is configured (default)' do + context 'when a timeout error occurs' do + before do + stub_publish_request.to_timeout + end + + it_behaves_like 'raises the exception', RestClient::RequestTimeout + end + + context 'when a HTTP errors occurs' do + before do + stub_publish_request.to_return(status: 500) + end + + it_behaves_like 'raises the exception', RestClient::InternalServerError + end + end + + context 'when retry_interval is configured' do + before do + @my_retry_interval = double('my_retry_interval') + + HubClient.configure do |config| + config.retry_intervals = @my_retry_interval + end + end + + context 'when a timeout error occurs' do + before do + stub_publish_request.to_timeout + end + + context 'when the retry_interval.next method returns nil' do + before do + allow(@my_retry_interval).to receive(:next).with(1).and_return(nil) + end + + it_behaves_like 'raises the exception', RestClient::RequestTimeout + + it 'does not sleep' do + expect(Kernel).not_to receive(:sleep) + action rescue nil + end + end + + context 'when the retry_interval.next method returns a number' do + before do + expect(@my_retry_interval).to receive(:next).with(1).and_return(12) + allow(Kernel).to receive(:sleep) + end + + context 'and the next network call succeeds' do + before do + @net_response.then.to_return({body: 'ok'}) + end + + it 'sleeps for the returned number' do + action + expect(Kernel).to have_received(:sleep).with(12).once + end + + it 'does not raise an exception' do + action + end + end + + context 'and the next network call times out again' do + before do + @net_response.then.to_timeout + end + + context 'and the the retry_interval.next call returns nil' do + before do + expect(@my_retry_interval).to receive(:next).with(2).and_return(nil) + end + + it_behaves_like 'raises the exception', RestClient::RequestTimeout + end + + context 'and the the retry_interval.next call returns a number' do + before do + expect(@my_retry_interval).to receive(:next).with(2).and_return(13) + end + + context 'and the next network call succeeds' do + before do + @net_response.then.to_return({body: 'ok'}) + end + + it 'sleeps for the returned number' do + action + expect(Kernel).to have_received(:sleep).with(13).once + end + + it 'does not raise an exception' do + action + end + end + end + end + end + end + + context 'when a HTTP errors occurs' do + before do + stub_publish_request.to_return(status: 500) + end + + context 'when the retry_interval.next method returns nil' do + before do + allow(@my_retry_interval).to receive(:next).with(1).and_return(nil) + end + + it_behaves_like 'raises the exception', RestClient::InternalServerError + + it 'does not sleep' do + expect(Kernel).not_to receive(:sleep) + action rescue nil + end + end + end + end + end end end