Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Merge pull request #21 from librato/retries

Retries
  • Loading branch information...
commit 0c54824b769e45ceca1595d438a1e9338afed9f6 2 parents 5af8209 + 28fd02b
@nextmat nextmat authored
View
2  lib/librato/metrics/connection.rb
@@ -29,8 +29,8 @@ def transport
raise NoClientProvided unless @client
@transport ||= Faraday::Connection.new(:url => api_endpoint + "/v1/") do |f|
#f.use FaradayMiddleware::EncodeJson
- f.use Librato::Metrics::Middleware::CountRequests
f.use Librato::Metrics::Middleware::Retry
+ f.use Librato::Metrics::Middleware::CountRequests
f.use Librato::Metrics::Middleware::ExpectsStatus
#f.use FaradayMiddleware::ParseJson, :content_type => /\bjson$/
f.adapter Faraday.default_adapter
View
10 lib/librato/metrics/errors.rb
@@ -8,6 +8,16 @@ class CredentialsMissing < MetricsError; end
class AgentInfoMissing < MetricsError; end
class NoMetricsQueued < MetricsError; end
class NoClientProvided < MetricsError; end
+
+ class NetworkError < StandardError; end
+
+ class ClientError < NetworkError; end
+ class Unauthorized < ClientError; end
+ class Forbidden < ClientError; end
+ class NotFound < ClientError; end
+ class EntityAlreadyExists < ClientError; end
+
+ class ServerError < NetworkError; end
end
end
View
14 lib/librato/metrics/middleware/expects_status.rb
@@ -8,10 +8,18 @@ def on_complete(env)
# TODO: clean up exception output
# TODO: catch specific status codes by request
case env[:status]
+ when 401
+ raise Unauthorized, env.to_s
+ when 403
+ raise Forbidden, env.to_s
when 404
- raise Faraday::Error::ResourceNotFound, env.to_s
- when 400..600
- raise Faraday::Error::ClientError, env.to_s
+ raise NotFound, env.to_s
+ when 422
+ raise EntityAlreadyExists, env.to_s
+ when 400..499
+ raise ClientError, env.to_s
+ when 500..599
+ raise ServerError, env.to_s
end
end
View
13 lib/librato/metrics/middleware/retry.rb
@@ -12,13 +12,14 @@ def initialize(app, retries = 3)
def call(env)
retries = @retries
- @app.call(env)
- rescue StandardError, Timeout::Error
- if retries > 0
- retries -= 1
- retry
+ begin
+ @app.call(env)
+ rescue Librato::Metrics::ServerError, Timeout::Error
+ if retries > 0
+ retries -= 1 and retry
+ end
+ raise
end
- raise
end
end
View
2  librato-metrics.gemspec
@@ -33,6 +33,8 @@ Gem::Specification.new do |s|
s.add_development_dependency 'pry'
s.add_development_dependency 'yard'
s.add_development_dependency 'rdiscount' # for yard
+ s.add_development_dependency 'sinatra'
+ s.add_development_dependency 'popen4'
s.files = `git ls-files`.split("\n")
s.test_files = `git ls-files -- {test,spec,features}/*`.split("\n")
View
9 spec/integration/metrics_spec.rb
@@ -7,11 +7,12 @@ module Librato
describe "#fetch" do
before(:all) do
delete_all_metrics
- Metrics.submit :my_counter => {:type => :counter, :value => 0}
+ Metrics.submit :my_counter => {:type => :counter, :value => 0, :measure_time => Time.now.to_i-60}
1.upto(2).each do |i|
- sleep 1.1
- Metrics.submit :my_counter => {:type => :counter, :value => i}
- Metrics.submit :my_counter => {:source => 'baz', :type => :counter, :value => i+1}
+ measure_time = Time.now.to_i - (5+i)
+ opts = {:measure_time => measure_time, :type => :counter}
+ Metrics.submit :my_counter => opts.merge(:value => i)
+ Metrics.submit :my_counter => opts.merge(:source => 'baz', :value => i+1)
end
end
View
21 spec/rackups/status.ru
@@ -0,0 +1,21 @@
+require 'sinatra'
+
+class App < Sinatra::Base
+ get('/v1/success') do
+ status 200
+ end
+
+ post('/v1/forbidden') do
+ status 403
+ end
+
+ get('/v1/not_found') do
+ status 404
+ end
+
+ post('/v1/service_unavailable') do
+ status 503
+ end
+end
+
+run App
View
41 spec/spec_helper.rb
@@ -2,6 +2,7 @@
# only load pry for MRI > 1.8
require 'pry' if RUBY_ENGINE == 'ruby' rescue nil
+require 'popen4'
require 'rspec'
require 'rspec/mocks/standalone'
require 'set'
@@ -9,7 +10,17 @@
require 'librato/metrics'
RSpec.configure do |config|
-
+
+ # purge all metrics from test account
+ def delete_all_metrics
+ connection = Librato::Metrics.client.connection
+ Librato::Metrics.list.each do |metric|
+ #puts "deleting #{metric['name']}..."
+ # expects 204
+ connection.delete("metrics/#{metric['name']}")
+ end
+ end
+
# set up test account credentials for integration tests
def prep_integration_tests
raise 'no TEST_API_USER specified in environment' unless ENV['TEST_API_USER']
@@ -19,14 +30,26 @@ def prep_integration_tests
end
Librato::Metrics.authenticate ENV['TEST_API_USER'], ENV['TEST_API_KEY']
end
-
- # purge all metrics from test account
- def delete_all_metrics
- connection = Librato::Metrics.client.connection
- Librato::Metrics.list.each do |metric|
- #puts "deleting #{metric['name']}..."
- # expects 204
- connection.delete("metrics/#{metric['name']}")
+
+ def rackup_path(*parts)
+ File.expand_path(File.join(File.dirname(__FILE__), 'rackups', *parts))
+ end
+
+ # fire up a given rackup file for the enclosed tests
+ def with_rackup(name)
+ if RUBY_PLATFORM == 'java'
+ pid, w, r, e = IO.popen4("rackup", rackup_path(name), '-p 9296')
+ else
+ GC.disable
+ pid, w, r, e = Open4.popen4("rackup", rackup_path(name), '-p 9296')
+ end
+ until e.gets =~ /HTTPServer#start:/; end
+ yield
+ ensure
+ Process.kill(9, pid)
+ if RUBY_PLATFORM != 'java'
+ GC.enable
+ Process.wait(pid)
end
end
View
48 spec/unit/metrics/connection_spec.rb
@@ -5,14 +5,6 @@ module Metrics
describe Connection do
- describe "network operations" do
- context "when missing client" do
- it "should raise exception" do
- lambda { subject.get 'metrics' }#.should raise(NoClientProvided)
- end
- end
- end
-
describe "#api_endpoint" do
context "when not provided" do
it "should be default" do
@@ -48,6 +40,46 @@ module Metrics
# TODO: verify user agent is being sent with rackup test
end
+ describe "network operations" do
+ context "when missing client" do
+ it "should raise exception" do
+ lambda { subject.get 'metrics' }.should raise_error(NoClientProvided)
+ end
+ end
+
+ context "with 400 class errors" do
+ it "should not retry" do
+ Middleware::CountRequests.reset
+ client = Client.new
+ client.api_endpoint = 'http://127.0.0.1:9296'
+ client.authenticate 'foo', 'bar'
+ with_rackup('status.ru') do
+ lambda {
+ client.connection.transport.post 'not_found'
+ }.should raise_error(NotFound)
+ lambda {
+ client.connection.transport.post 'forbidden'
+ }.should raise_error(ClientError)
+ end
+ Middleware::CountRequests.total_requests.should == 2
+ end
+ end
+
+ context "with 500 class errors" do
+ it "should retry" do
+ Middleware::CountRequests.reset
+ client = Client.new
+ client.api_endpoint = 'http://127.0.0.1:9296'
+ client.authenticate 'foo', 'bar'
+ with_rackup('status.ru') do
+ lambda {
+ client.connection.transport.post 'service_unavailable'
+ }.should raise_error(ServerError)
+ end
+ Middleware::CountRequests.total_requests.should == 4
+ end
+ end
+ end
end
Please sign in to comment.
Something went wrong with that request. Please try again.