Fixing #32 for incorrect handling http errors

This commit fix the error of rising a wrong HTTPParseException when
trying to parse the body for non 200 response status code

Now we first doublecheck the status codes for the response, and if it
was a 200 then it will try to parse the body.

lib/active_rest_client/request.rb

@@ -268,10 +268,22 @@ def handle_cached_response(cached)
     end
 
     def handle_response(response)
+      response.status ||= 200
       if response.status == 304
         ActiveRestClient::Logger.debug "  \033[1;4;32m#{ActiveRestClient::NAME}\033[0m #{@instrumentation_name} - Etag copy is the same as the server"
         return :not_modified
+      elsif response.status == 401
+        raise HTTPUnauthorisedClientException.new(status:response.status, url:@url)
+      elsif response.status == 403
+        raise HTTPForbiddenClientException.new(status:response.status, url:@url)
+      elsif response.status == 404
+        raise HTTPNotFoundClientException.new(status:response.status, url:@url)
+      elsif (400..499).include? response.status
+        raise HTTPClientException.new(status:response.status, url:@url)
+      elsif (500..599).include? response.status
+        raise HTTPServerException.new(status:response.status, url:@url)
       end
+
       if response.respond_to?(:proxied) && response.proxied
         ActiveRestClient::Logger.debug "  \033[1;4;32m#{ActiveRestClient::NAME}\033[0m #{@instrumentation_name} - Response was proxied, unable to determine size"
       else
@@ -308,19 +320,6 @@ def handle_response(response)
         end
       end
 
-      response.status ||= 200
-      if response.status == 401
-        raise HTTPUnauthorisedClientException.new(status:response.status, result:result, url:@url)
-      elsif response.status == 403
-        raise HTTPForbiddenClientException.new(status:response.status, result:result, url:@url)
-      elsif response.status == 404
-        raise HTTPNotFoundClientException.new(status:response.status, result:result, url:@url)
-      elsif (400..499).include? response.status
-        raise HTTPClientException.new(status:response.status, result:result, url:@url)
-      elsif (500..599).include? response.status
-        raise HTTPServerException.new(status:response.status, result:result, url:@url)
-      end
-
       result
     rescue MultiJson::ParseError
       raise ResponseParseException.new(status:response.status, body:response.body)

spec/lib/request_spec.rb

@@ -248,12 +248,11 @@ class FilteredBodyExampleClient < ExampleClient
     object = ExampleClient.new(first_name:"John", should_disappear:true)
     begin
       object.create
-    rescue ActiveRestClient::HTTPUnauthorisedClientException => e
+    rescue => e
       e
     end
     expect(e).to be_instance_of(ActiveRestClient::HTTPUnauthorisedClientException)
     expect(e.status).to eq(401)
-    expect(e.result.first_name).to eq("John")
   end
 
   it "should raise a forbidden client exception for 403 errors" do
@@ -265,12 +264,11 @@ class FilteredBodyExampleClient < ExampleClient
     object = ExampleClient.new(first_name:"John", should_disappear:true)
     begin
       object.create
-    rescue ActiveRestClient::HTTPForbiddenClientException => e
+    rescue => e
       e
     end
     expect(e).to be_instance_of(ActiveRestClient::HTTPForbiddenClientException)
     expect(e.status).to eq(403)
-  expect(e.result.first_name).to eq("John")
   end
 
   it "should raise a not found client exception for 404 errors" do
@@ -282,12 +280,11 @@ class FilteredBodyExampleClient < ExampleClient
     object = ExampleClient.new(first_name:"John", should_disappear:true)
     begin
       object.create
-    rescue ActiveRestClient::HTTPNotFoundClientException => e
+    rescue => e
       e
     end
     expect(e).to be_instance_of(ActiveRestClient::HTTPNotFoundClientException)
     expect(e.status).to eq(404)
-    expect(e.result.first_name).to eq("John")
   end
 
   it "should raise a client exceptions for 4xx errors" do
@@ -299,12 +296,11 @@ class FilteredBodyExampleClient < ExampleClient
     object = ExampleClient.new(first_name:"John", should_disappear:true)
     begin
       object.create
-    rescue ActiveRestClient::HTTPClientException => e
+    rescue => e
       e
     end
     expect(e).to be_instance_of(ActiveRestClient::HTTPClientException)
     expect(e.status).to eq(409)
-  expect(e.result.first_name).to eq("John")
   end
 
   it "should raise a server exception for 5xx errors" do
@@ -316,29 +312,28 @@ class FilteredBodyExampleClient < ExampleClient
     object = ExampleClient.new(first_name:"John", should_disappear:true)
     begin
       object.create
-    rescue ActiveRestClient::HTTPServerException => e
+    rescue => e
       e
     end
     expect(e).to be_instance_of(ActiveRestClient::HTTPServerException)
     expect(e.status).to eq(500)
-    expect(e.result.first_name).to eq("John")
   end
 
   it "should raise a parse exception for invalid JSON returns" do
-    error_content = "<h1>500 Server Error</h1>"
+    error_content = "<h1>invalid json content</h1>"
     ActiveRestClient::Connection.
       any_instance.
       should_receive(:post).
       with("/create", "first_name=John&should_disappear=true", an_instance_of(Hash)).
-      and_return(OpenStruct.new(body:error_content, headers:{}, status:500))
+      and_return(OpenStruct.new(body:error_content, headers:{}, status:200))
     object = ExampleClient.new(first_name:"John", should_disappear:true)
     begin
       object.create
-    rescue ActiveRestClient::ResponseParseException => e
+    rescue => e
       e
     end
     expect(e).to be_instance_of(ActiveRestClient::ResponseParseException)
-    expect(e.status).to eq(500)
+    expect(e.status).to eq(200)
     expect(e.body).to eq(error_content)
   end