RUBYAPI-39 add request_path to ClientError

The downstream clients are able to glean API error codes, messages, and
statuses now, but there are certain circumstances where it's not clear
which API request failed without turning debug logging on.

This change attempts to offer a happy medium, where an API application
can log the request path, HTTP status, and API error code without having
to resort to an all-or-nothing logging approach.

Author: bhornseth

lib/spark_api/authentication/oauth2_impl/faraday_middleware.rb

@@ -27,7 +27,7 @@ def on_complete(env)
             # OAuth2 implementations and wouldn't hurt to log.
             auth_header_error = env[:request_headers]["WWW-Authenticate"]
             SparkApi.logger.warn { "Authentication error #{auth_header_error}" } unless auth_header_error.nil?
-            raise ClientError, {:message => body["error"], :code =>0, :status => env[:status]}
+            raise ClientError, {:message => body["error"], :code =>0, :status => env[:status], :request_path => env[:url]}
           end
           SparkApi.logger.debug { "[oauth2] Session=#{session.inspect}" }
           env[:body] = session
@@ -58,7 +58,7 @@ def on_complete(env)
               return
             end
           end
-          raise ClientError, {:message => "Unable to process sparkbar token #{body.inspect}", :code =>0, :status => env[:status]}
+          raise ClientError, {:message => "Unable to process sparkbar token #{body.inspect}", :code =>0, :status => env[:status], :request_path => env[:url]}
         end
 
       end

lib/spark_api/errors.rb

@@ -21,13 +21,14 @@ module ResponseCodes
   # Errors built from API responses
   class InvalidResponse < StandardError; end
   class ClientError < StandardError
-    attr_reader :code, :status, :details
+    attr_reader :code, :status, :details, :request_path
     def initialize (options = {})
       # Support the standard initializer for errors
       opts = options.is_a?(Hash) ? options : {:message => options.to_s}
       @code = opts[:code]
       @status = opts[:status]
       @details = opts[:details]
+      @request_path = opts[:request_path]
       super(opts[:message])
     end
 
@@ -45,4 +46,4 @@ def self.ssl_verification_error
       "set 'ssl_verify' to false in the configuration or add '--no_verify' to the CLI command."
     end
   end
-end
+end

lib/spark_api/faraday_middleware.rb

@@ -15,7 +15,6 @@ def initialize(app)
     # Handles pretty much all the api response parsing and error handling.  All responses that
     # indicate a failure will raise a SparkApi::ClientError exception
     def on_complete(env)
-
       env[:body] = decompress_body(env)
 
       body = MultiJson.decode(env[:body])
@@ -25,6 +24,7 @@ def on_complete(env)
       end
       response = ApiResponse.new body
       paging = response.pagination
+
       if paging.nil?
         results = response
       else
@@ -35,34 +35,40 @@ def on_complete(env)
           results = paginate_response(response, paging)
         end
       end
+
+      error_hash = {
+        :request_path => env[:url],
+        :message => response.message,
+        :code => response.code,
+        :status => env[:status]
+      }
+
       case env[:status]
       when 400
-        hash = {:message => response.message, :code => response.code, :status => env[:status]}
-
         # constraint violation
         if response.code == 1053
           details = body['D']['Details']
-          hash[:details] = details
+          error_hash[:details] = details
         end
-        raise BadResourceRequest,hash
+        raise BadResourceRequest, error_hash
       when 401
         # Handle the WWW-Authenticate Response Header Field if present. This can be returned by 
         # OAuth2 implementations and wouldn't hurt to log.
         auth_header_error = env[:request_headers]["WWW-Authenticate"]
         SparkApi.logger.warn("Authentication error #{auth_header_error}") unless auth_header_error.nil?
-        raise PermissionDenied, {:message => response.message, :code => response.code, :status => env[:status]}
+        raise PermissionDenied, error_hash
       when 404
-        raise NotFound, {:message => response.message, :code => response.code, :status => env[:status]}
+        raise NotFound, error_hash
       when 405
-        raise NotAllowed, {:message => response.message, :code => response.code, :status => env[:status]}
+        raise NotAllowed, error_hash
       when 409
-        raise BadResourceRequest, {:message => response.message, :code => response.code, :status => env[:status]}
+        raise BadResourceRequest, error_hash
       when 500
-        raise ClientError, {:message => response.message, :code => response.code, :status => env[:status]}
+        raise ClientError, error_hash
       when 200..299
         SparkApi.logger.debug { "Success!" }
       else 
-        raise ClientError, {:message => response.message, :code => response.code, :status => env[:status]}
+        raise ClientError, error_hash
       end
       env[:body] = results
     end

spec/unit/spark_api/request_spec.rb

@@ -2,7 +2,7 @@
 
 describe SparkApi do
   describe SparkApi::ClientError do
-    subject { SparkApi::ClientError.new({:message=>"OMG FAIL", :code=>1234, :status=>500}) }
+    subject { SparkApi::ClientError.new({:message=>"OMG FAIL", :code=>1234, :status=>500, :request_path => '/v1/foo'}) }
     it "should print a helpful to_s" do
       subject.to_s.should == "OMG FAIL"
       subject.message.should == "OMG FAIL"
@@ -13,6 +13,11 @@
     it "should have an http status" do
       subject.status.should == 500
     end
+
+    it "should have a request_path" do
+      subject.request_path.should == '/v1/foo'
+    end
+
     it "should raise and exception with attached message" do
       expect { raise subject.class, {:message=>"My Message", :code=>1000, :status=>404}}.to raise_error(SparkApi::ClientError)  do |e| 
         e.message.should == "My Message" 
@@ -339,12 +344,19 @@ def reauthenticated
         r
       end
       it "should fail horribly on a get" do
-        expect { subject.get('/system')}.to raise_error(SparkApi::PermissionDenied){ |e| e.code.should == SparkApi::ResponseCodes::SESSION_TOKEN_EXPIRED }
+        expect { subject.get('/system')}.to raise_error(SparkApi::PermissionDenied) do |e| 
+          e.code.should == SparkApi::ResponseCodes::SESSION_TOKEN_EXPIRED
+          e.request_path.should == '/system'
+        end
         subject.reauthenticated.should == 2
+
       end
       it "should fail horribly on a post" do
         data = {"Contacts" => [{"DisplayName"=>"Wades Contact","PrimaryEmail"=>"wade11@fbsdata.com"}]}
-        expect { subject.post('/contacts', data)}.to raise_error(SparkApi::PermissionDenied){ |e| e.code.should == SparkApi::ResponseCodes::SESSION_TOKEN_EXPIRED }
+        expect { subject.post('/contacts', data)}.to raise_error(SparkApi::PermissionDenied) do |e| 
+          e.code.should == SparkApi::ResponseCodes::SESSION_TOKEN_EXPIRED
+          e.request_path.should == '/contacts'
+        end
         subject.reauthenticated.should == 2
       end
     end