Multiple-endpoints: handle failure of one endpoint

If one endpoint failed, it was previously returning [-1, {}] even with
multiple endpoints. Now each failing endpoint returns [-1, {}], without
breaking the format of the return ([[200, {}], [-1, {}]]).

This also removes an Exception when options was empty for
update_comment, since it was the only option doing that.

lib/dogapi/common.rb

@@ -125,27 +125,30 @@ def suppress_error_if_silent(e)
     def request(method, url, extra_params, body, send_json, with_app_key=true)
       resp = nil
       connect do |conn, api_key, app_key|
-        app_key = nil unless with_app_key
-        current_url = url + prepare_params(extra_params, api_key, app_key)
-        req = method.new(current_url)
+        begin
+          app_key = nil unless with_app_key
+          current_url = url + prepare_params(extra_params, api_key, app_key)
+          req = method.new(current_url)
+
+          if send_json
+            req.content_type = 'application/json'
+            req.body = MultiJson.dump(body)
+          end
 
-        if send_json
-          req.content_type = 'application/json'
-          req.body = MultiJson.dump(body)
+          resp = conn.request(req)
+          next handle_response(resp)
+        rescue Exception => e
+          next suppress_error_if_silent e
         end
-        resp = conn.request(req)
-        next handle_response(resp)
       end
-    rescue Exception => e
-      suppress_error_if_silent e
     end
 
     def prepare_params(extra_params, api_key, app_key)
       params = { api_key: api_key }
       params[:application_key] = app_key unless app_key.nil?
       params = extra_params.merge params unless extra_params.nil?
-      qs_params = params.map { |k, v| k.to_s + "=" + v.to_s }
-      qs = "?" + qs_params.join("&")
+      qs_params = params.map { |k, v| k.to_s + '=' + v.to_s }
+      qs = '?' + qs_params.join('&')
       qs
     end
 
@@ -177,5 +180,4 @@ def Dogapi.find_localhost
       raise "Cannot determine local hostname via hostname -f"
     end
   end
-
 end

lib/dogapi/v1/comment.rb

@@ -18,10 +18,6 @@ def comment(message, options = {})
 
       # Update a comment.
       def update_comment(comment_id, options = {})
-        if options.empty?
-          raise "Must update something."
-        end
-
         request(Net::HTTP::Put, "/api/#{API_VERSION}/comments/#{comment_id}", nil, options, true)
       end
 

spec/integration/common_spec.rb

@@ -2,41 +2,70 @@
 
 describe Dogapi::APIService do
   describe '#connect' do
+    let(:url) { api_url + '/awesome' }
+    let(:second_url) { 'http://app.example.com/api/v1/awesome' }
+
     context 'when only the default endpoint is used' do
-      it 'only queries one endpoint' do
-        service = Dogapi::APIService.new api_key, app_key
-        url = api_url + '/awesome'
+      let(:service) { Dogapi::APIService.new api_key, app_key }
+      context 'and it is up' do
+        it 'only queries one endpoint' do
+          stub_request(:get, /#{url}/).to_return(body: '{}').then.to_raise(StandardError)
+          expect(service.request(Net::HTTP::Get, '/api/v1/awesome', nil, nil, true, true)).to eq(['200', {}])
 
-        stub_request(:get, /#{url}/).to_return(body: '{}').then.to_raise(StandardError)
-        expect(service.request(Net::HTTP::Get, '/api/v1/awesome', nil, nil, true, true)).to eq(['200', {}])
+          expect(WebMock).to have_requested(:get, url).with(
+            query: default_query
+          )
+        end
+      end
+      context 'and it is down' do
+        it 'only queries one endpoint' do
+          stub_request(:get, /#{url}/).to_timeout
+          expect(service.request(Net::HTTP::Get, '/api/v1/awesome', nil, nil, true, true)).to eq([-1, {}])
 
-        expect(WebMock).to have_requested(:get, url).with(
-          query: default_query
-        )
+          expect(WebMock).to have_requested(:get, url).with(
+            query: default_query
+          )
+        end
       end
     end
 
     context 'when multiple endpoints are used' do
-      it 'queries both endpoints' do
-        endpoints = { 'http://app.datadoghq.com' => [[api_key, app_key]],
-                      'http://app.example.com' => [%w(api_key2 app_key2)] }
-        service = Dogapi::APIService.new api_key, app_key, true, nil, endpoints
-
-        url = api_url + '/awesome'
-        second_url = 'http://app.example.com/api/v1/awesome'
+      let(:endpoints) do
+        { 'http://app.datadoghq.com' => [[api_key, app_key]],
+          'http://app.example.com' => [%w(api_key2 app_key2)] }
+      end
+      let(:service) { Dogapi::APIService.new api_key, app_key, true, nil, endpoints }
+      context 'and both are up' do
+        it 'queries both endpoints' do
+          stub_request(:get, /#{url}/).to_return(body: '{}').then.to_raise(StandardError)
+          stub_request(:get, /#{second_url}/).to_return(body: '{}').then.to_raise(StandardError)
+          expect(service.request(Net::HTTP::Get, '/api/v1/awesome', nil, nil, true, true)).to eq(
+            [['200', {}], ['200', {}]]
+          )
 
-        stub_request(:get, /#{url}/).to_return(body: '{}').then.to_raise(StandardError)
-        stub_request(:get, /#{second_url}/).to_return(body: '{}').then.to_raise(StandardError)
-        expect(service.request(Net::HTTP::Get, '/api/v1/awesome', nil, nil, true, true)).to eq(
-          [['200', {}], ['200', {}]]
-        )
+          expect(WebMock).to have_requested(:get, url).with(
+            query: default_query
+          )
+          expect(WebMock).to have_requested(:get, second_url).with(
+            query: { api_key: 'api_key2', application_key: 'app_key2' }
+          )
+        end
+      end
+      context 'and one is down' do
+        it 'queries both endpoints' do
+          stub_request(:get, /#{url}/).to_return(body: '{}').then.to_raise(StandardError)
+          stub_request(:get, /#{second_url}/).to_timeout
+          expect(service.request(Net::HTTP::Get, '/api/v1/awesome', nil, nil, true, true)).to eq(
+            [['200', {}], [-1, {}]]
+          )
 
-        expect(WebMock).to have_requested(:get, url).with(
-          query: default_query
-        )
-        expect(WebMock).to have_requested(:get, second_url).with(
-          query: { api_key: 'api_key2', application_key: 'app_key2' }
-        )
+          expect(WebMock).to have_requested(:get, url).with(
+            query: default_query
+          )
+          expect(WebMock).to have_requested(:get, second_url).with(
+            query: { api_key: 'api_key2', application_key: 'app_key2' }
+          )
+        end
       end
     end
 
@@ -45,8 +74,6 @@
         endpoints = { 'http://app.datadoghq.com' => [[api_key, app_key], %w(api_key2 app_key2)] }
         service = Dogapi::APIService.new api_key, app_key, true, nil, endpoints
 
-        url = api_url + '/awesome'
-
         stub_request(:get, /#{url}/).to_return(body: '{}').times(2).then.to_raise(StandardError)
         expect(service.request(Net::HTTP::Get, '/api/v1/awesome', nil, nil, true, true)).to eq(
           [['200', {}], ['200', {}]]
@@ -67,9 +94,6 @@
                       'http://app.example.com' => [%w(api_key3 app_key3)] }
         service = Dogapi::APIService.new api_key, app_key, true, nil, endpoints
 
-        url = api_url + '/awesome'
-        second_url = 'http://app.example.com/api/v1/awesome'
-
         stub_request(:get, /#{url}/).to_return(body: '{}').times(2).then.to_raise(StandardError)
         stub_request(:get, /#{second_url}/).to_return(body: '{}').then.to_raise(StandardError)
         expect(service.request(Net::HTTP::Get, '/api/v1/awesome', nil, nil, true, true)).to eq(