Address review comments

lib/koala/api/graph_batch_api.rb

@@ -59,8 +59,9 @@ def execute(http_options = {})
             # from graph_call so this needs to be parsed
             # as generate_results method handles only JSON response
             if http_options[:http_component] && http_options[:http_component] == :response
-              response = JSON.load(response.body)
-              raise bad_response("Facebook returned an invalid body") unless response.is_a?(Array)
+              response = json_body(response.body)
+
+              raise bad_response('Facebook returned an invalid body') unless response.is_a?(Array)
             end
 
             batch_results += generate_results(response, batch)
@@ -131,10 +132,13 @@ def batch_args(calls_for_batch)
         JSON.dump calls
       end
 
-      def json_body(response)
-        # quirks_mode is needed because Facebook sometimes returns a raw true or false value --
-        # in Ruby 2.4 we can drop that.
-        JSON.parse(response.fetch("body"), quirks_mode: true)
+      def json_body(body)
+        return if body.nil?
+
+        JSON.parse(body)
+      rescue JSON::ParserError => e
+        Koala::Utils.logger.error("#{e.class}: #{e.message} while parsing #{body}")
+        nil
       end
 
       def desired_component(component:, response:, headers:)
@@ -146,11 +150,7 @@ def desired_component(component:, response:, headers:)
         # facebook returns the headers as an array of k/v pairs, but we want a regular hash
         when :headers then headers
         # (see note in regular api method about JSON parsing)
-        when :response
-          Koala::HTTPService::Response.new(
-              response["code"].to_i,
-              response["body"],
-              headers)
+        when :response then result
         else GraphCollection.evaluate(result, original_api)
         end
       end

spec/cases/graph_api_batch_spec.rb

@@ -417,32 +417,61 @@
             }.to raise_exception(Koala::Facebook::BadFacebookResponse, /Facebook returned an empty body \[HTTP 200\]/)
           end
 
-          describe 'handle invalid body error' do
+          describe 'handle invalid body errors' do
             describe 'with http_component set to :response' do
               it 'raises a BadFacebookResponse if the body is non-empty, non-array' do
                 allow(Koala).to receive(:make_request).and_return(Koala::HTTPService::Response.new(200, '200', {}))
                 expect {
-                  Koala::Facebook::API.new('foo').batch(http_component: :response) { |batch_api| batch_api.get_object('me') }
+                  Koala::Facebook::API.new('foo').batch(http_component: :response) do |batch_api|
+                    batch_api.get_object('me')
+                  end
+                }.to raise_exception(Koala::Facebook::BadFacebookResponse, /Facebook returned an invalid body \[HTTP 200\]/)
+              end
+
+              it 'raises a BadFacebookResponse if the body is invalid JSON' do
+                allow(Koala).to receive(:make_request).and_return(Koala::HTTPService::Response.new(200, '{"\"id\":\1234\"}"}', {}))
+                expect {
+                  Koala::Facebook::API.new('foo').batch(http_component: :response) do |batch_api|
+                    batch_api.get_object('me')
+                  end
                 }.to raise_exception(Koala::Facebook::BadFacebookResponse, /Facebook returned an invalid body \[HTTP 200\]/)
               end
             end
 
             describe 'with http_component set to :headers' do
-              it 'should not raise a BadFacebookResponse' do
+              it 'should not raise a BadFacebookResponse if the body is non-empty, non-array' do
                 allow(Koala).to receive(:make_request).and_return(Koala::HTTPService::Response.new(200, '200', {}))
                 expect {
                   Koala::Facebook::API.new('foo').batch(http_component: :headers) { |batch_api| batch_api.get_object('me') }
                 }.not_to raise_exception(Koala::Facebook::BadFacebookResponse, /Facebook returned an invalid body \[HTTP 200\]/)
               end
+
+              it 'should not raise a BadFacebookResponse if the body is invalid JSON' do
+                allow(Koala).to receive(:make_request).and_return(Koala::HTTPService::Response.new(200, '{"\"id\":\1234\"}"}', {}))
+                expect {
+                  Koala::Facebook::API.new('foo').batch(http_component: :headers) do |batch_api|
+                    batch_api.get_object('me')
+                  end
+                }.not_to raise_exception(Koala::Facebook::BadFacebookResponse, /Facebook returned an invalid body \[HTTP 200\]/)
+              end
             end
 
             describe 'with http_component set to :status' do
-              it 'should not raise a BadFacebookResponse' do
+              it 'should not raise a BadFacebookResponse if the body is non-empty, non-array' do
                 allow(Koala).to receive(:make_request).and_return(Koala::HTTPService::Response.new(200, '200', {}))
                 expect {
                   Koala::Facebook::API.new('foo').batch(http_component: :status) { |batch_api| batch_api.get_object('me') }
                 }.not_to raise_exception(Koala::Facebook::BadFacebookResponse, /Facebook returned an invalid body \[HTTP 200\]/)
               end
+
+              it 'should not raise a BadFacebookResponse if the body is invalid JSON' do
+                allow(Koala).to receive(:make_request).and_return(Koala::HTTPService::Response.new(200, '{"\"id\":\1234\"}"}', {}))
+                expect {
+                  Koala::Facebook::API.new('foo').batch(http_component: :status) do |batch_api|
+                    batch_api.get_object('me')
+                  end
+                }.not_to raise_exception(Koala::Facebook::BadFacebookResponse, /Facebook returned an invalid body \[HTTP 200\]/)
+              end
             end
           end