Issue 47 - Improve error handling on result

lib/google/api_client.rb

@@ -772,27 +772,18 @@ module Google
     # @see Google::APIClient#execute
     def execute!(*params)
       result = self.execute(*params)
-      if result.data?
-        if result.data.respond_to?(:error) &&
-             result.data.error.respond_to?(:message)
-          # You're going to get a terrible error message if the response isn't
-          # parsed successfully as an error.
-          error_message = result.data.error.message
-        elsif result.data['error'] && result.data['error']['message']
-          error_message = result.data['error']['message']
-        end
-      end
-      if result.response.status >= 400
+      if result.error?
+        error_message = result.error_message
         case result.response.status
-        when 400...500
-          exception_type = ClientError
-          error_message ||= "A client error has occurred."
-        when 500...600
-          exception_type = ServerError
-          error_message ||= "A server error has occurred."
-        else
-          exception_type = TransmissionError
-          error_message ||= "A transmission error has occurred."
+          when 400...500
+            exception_type = ClientError
+            error_message ||= "A client error has occurred."
+          when 500...600
+            exception_type = ServerError
+            error_message ||= "A server error has occurred."
+          else
+            exception_type = TransmissionError
+            error_message ||= "A transmission error has occurred."
         end
         raise exception_type, error_message
       end

lib/google/api_client/result.rb

@@ -53,6 +53,28 @@ module Google
         content_type[/^([^;]*);?.*$/, 1].strip.downcase
       end
       
+      def error?
+        return self.response.status >= 400
+      end
+
+      def success?
+        return !self.error?
+      end
+      
+      def error_message
+        if self.data?
+          if self.data.respond_to?(:error) &&
+             self.data.error.respond_to?(:message)
+            # You're going to get a terrible error message if the response isn't
+            # parsed successfully as an error.
+            return self.data.error.message
+          elsif self.data['error'] && self.data['error']['message']
+            return self.data['error']['message']
+          end
+        end
+        return self.body
+      end
+      
       def data?
         self.media_type == 'application/json'
       end

spec/google/api_client/result_spec.rb

@@ -71,6 +71,10 @@ describe Google::APIClient::Result do
         @result = Google::APIClient::Result.new(@reference, @request, @response)
       end
 
+      it 'should indicate a successful response' do
+        @result.error?.should be_false
+      end
+
       it 'should return the correct next page token' do
         @result.next_page_token.should == 'NEXT+PAGE+TOKEN'
       end
@@ -146,5 +150,38 @@ describe Google::APIClient::Result do
         @result.data.items.should be_empty
       end
     end
+    
+    describe 'with JSON error response' do
+      before do
+        @response.stub(:body).and_return(
+         <<-END_OF_STRING
+         {
+          "error": {
+           "errors": [
+            {
+             "domain": "global",
+             "reason": "parseError",
+             "message": "Parse Error"
+            }
+           ],
+           "code": 400,
+           "message": "Parse Error"
+          }
+         }
+         END_OF_STRING
+        )
+        @response.stub(:status).and_return(400)
+        @result = Google::APIClient::Result.new(@reference, @request, @response)
+      end
+      
+      it 'should return error status correctly' do
+        @result.error?.should be_true
+      end
+
+      it 'should return the correct error message' do
+        @result.error_message.should == 'Parse Error'
+      end
+
+    end
   end
 end