Better: Handles the invalid JSON response from Facebook when the requ…

…est's http_options[:http_component] is set to ':response'. #685

changelog.md

@@ -8,6 +8,7 @@ New features:
 Updated features:
 
  * Add fbtrace_id, x-fb-rev, x-fb-debug to error messages and error class ([#668](https://github.com/arsduo/koala/pull/686))
+ * Handles the invalid JSON response from Facebook when the request's http_options[:http_component] is set to ':response' ([#689](https://github.com/arsduo/koala/pull/689))
 
 Removed features:
 

lib/koala/api/graph_batch_api.rb

@@ -53,7 +53,16 @@ def execute(http_options = {})
           end
 
           original_api.graph_call("/", args, "post", http_options) do |response|
-            raise bad_response if response.nil?
+            raise bad_response('Facebook returned an empty body') if response.nil?
+
+            # when http_component is set we receive Koala::Http_service response object
+            # 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_body(response.body)
+
+              raise bad_response('Facebook returned an invalid body') unless response.is_a?(Array)
+            end
 
             batch_results += generate_results(response, batch)
           end
@@ -81,9 +90,9 @@ def generate_results(response, batch)
         end
       end
 
-      def bad_response
+      def bad_response(message)
         # Facebook sometimes reportedly returns an empty body at times
-        BadFacebookResponse.new(200, "", "Facebook returned an empty body")
+        BadFacebookResponse.new(200, '', message)
       end
 
       def result_from_response(response, options)
@@ -123,21 +132,25 @@ 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:)
-        result = Koala::HTTPService::Response.new(response['status'], response['body'], headers)
+        result = Koala::HTTPService::Response.new(response['code'], response['body'], headers)
 
         # Get the HTTP component they want
         case component
         when :status  then response["code"].to_i
         # 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 then result
         else GraphCollection.evaluate(result, original_api)
         end
       end

spec/cases/graph_api_batch_spec.rb

@@ -383,28 +383,85 @@
         end
       end
 
-      describe "processing the request" do
-        it "returns the result headers as a hash if http_component is headers" do
+      describe 'processing the request' do
+        let(:response_status) { 203 }
+        let(:response_body) { '{\"id\":\"1234\"}'.gsub('\\', '') }
+        let(:response_headers) { { 'Content-Type' => 'text/javascript; charset=UTF-8' } }
+
+        it 'returns the result headers as a hash if http_component is headers' do
           allow(Koala).to receive(:make_request).and_return(Koala::HTTPService::Response.new(200, '[{"code":203,"headers":[{"name":"Content-Type","value":"text/javascript; charset=UTF-8"}],"body":"{\"id\":\"1234\"}"}]', {}))
           result = @api.batch do |batch_api|
             batch_api.get_object(KoalaTest.user1, {}, :http_component => :headers)
           end
-          expect(result[0]).to eq({"Content-Type" => "text/javascript; charset=UTF-8"})
+          expect(response_headers).to eq(result[0])
+        end
+
+        it 'returns the complete response if http_component is response' do
+          allow(Koala).to receive(:make_request).and_return(Koala::HTTPService::Response.new(200, '[{"code":203,"headers":[{"name":"Content-Type","value":"text/javascript; charset=UTF-8"}],"body":"{\"id\":\"1234\"}"}]', {}))
+          result = @api.batch do |batch_api|
+            batch_api.get_object(KoalaTest.user1, {}, :http_component => :response)
+          end
+
+          expect(response_status).to eq(result[0].status)
+          expect(response_body).to eq(result[0].body)
+          expect(response_headers).to eq(result[0].headers)
         end
 
-        describe "if it errors" do
-          it "raises an APIError if the response is not 200" do
-            allow(Koala).to receive(:make_request).and_return(Koala::HTTPService::Response.new(500, "[]", {}))
+        describe 'if it errors' do
+          it 'raises an APIError if the response is not 200' do
+            allow(Koala).to receive(:make_request).and_return(Koala::HTTPService::Response.new(500, '[]', {}))
             expect {
-              Koala::Facebook::API.new("foo").batch {|batch_api| batch_api.get_object('me') }
+              Koala::Facebook::API.new('foo').batch { |batch_api| batch_api.get_object('me') }
             }.to raise_exception(Koala::Facebook::APIError)
           end
 
-          it "raises a BadFacebookResponse if the body is empty" do
-            allow(Koala).to receive(:make_request).and_return(Koala::HTTPService::Response.new(200, "", {}))
+          it 'raises a BadFacebookResponse if the body is empty' do
+            allow(Koala).to receive(:make_request).and_return(Koala::HTTPService::Response.new(200, '', {}))
             expect {
-              Koala::Facebook::API.new("foo").batch {|batch_api| batch_api.get_object('me') }
-            }.to raise_exception(Koala::Facebook::BadFacebookResponse)
+              Koala::Facebook::API.new('foo').batch { |batch_api| batch_api.get_object('me') }
+            }.to raise_exception(Koala::Facebook::BadFacebookResponse, /Facebook returned an empty body \[HTTP 200\]/)
+          end
+
+          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) 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
+
+            %i[headers status].each do |component|
+              describe "with http_component set to #{component}" 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: component) { |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: component) 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
           end
 
           context "with error info" do