Merge pull request #194 from afonso-aguas/fix/parse-body-before-error-check

Fix false-positive HttpResponseError on bodies containing "error" substring

Author: ignacio-chiazzo

lib/whatsapp_sdk/api/client.rb

@@ -58,13 +58,13 @@ def send_request(endpoint: "", full_url: nil, http_method: "post", params: {}, h
 
         response = faraday_request.public_send(http_method, endpoint, request_params(params, headers), headers)
 
-        if response.status > 499 || Api::Responses::GenericErrorResponse.response_error?(response: response.body)
-          raise Api::Responses::HttpResponseError.new(http_status: response.status, body: JSON.parse(response.body))
-        end
+        parsed_body = parse_response_body(response.body)
 
-        return nil if response.body == ""
+        if response.status > 499 || Api::Responses::GenericErrorResponse.response_error?(response: parsed_body)
+          raise Api::Responses::HttpResponseError.new(http_status: response.status, body: parsed_body)
+        end
 
-        JSON.parse(response.body)
+        parsed_body
       end
 
       def download_file(url:, content_type_header:, file_path: nil)
@@ -85,6 +85,12 @@ def download_file(url:, content_type_header:, file_path: nil)
 
       private
 
+      def parse_response_body(body)
+        return nil if body.nil? || body.empty?
+
+        JSON.parse(body)
+      end
+
       def request_params(params, headers)
         return params.to_json if params.is_a?(Hash) && headers['Content-Type'] == 'application/json'
 

lib/whatsapp_sdk/api/responses/generic_error_response.rb

@@ -16,6 +16,8 @@ def initialize(response:)
         end
 
         def self.response_error?(response:)
+          return false unless response.is_a?(Hash)
+
           response["error"]
         end
 

test/whatsapp/api/client_test.rb

@@ -51,6 +51,43 @@ def test_send_request_delete_with_success_response
         assert_nil(response_body)
       end
 
+      def test_send_request_does_not_raise_when_body_text_contains_error_substring
+        payload = {
+          'data' => [{
+            'name' => 'dia_push',
+            'components' => [{ 'type' => 'BODY', 'text' => 'si tu cupón te da algún error, avísanos' }]
+          }]
+        }
+        stub_test_request(:get, response_body: payload)
+
+        response_body = @client.send_request(endpoint: 'test', http_method: 'get')
+
+        assert_equal(payload, response_body)
+      end
+
+      def test_send_request_raises_when_response_has_error_key
+        error_body = { 'error' => { 'message' => 'Invalid OAuth token', 'code' => 190 } }
+        stub_test_request(:get, response_status: 400, response_body: error_body)
+
+        error = assert_raises(Api::Responses::HttpResponseError) do
+          @client.send_request(endpoint: 'test', http_method: 'get')
+        end
+
+        assert_equal(400, error.http_status)
+        assert_equal(error_body, error.body)
+      end
+
+      def test_send_request_raises_for_5xx_responses
+        error_body = { 'error' => { 'message' => 'Internal server error' } }
+        stub_test_request(:get, response_status: 502, response_body: error_body)
+
+        error = assert_raises(Api::Responses::HttpResponseError) do
+          @client.send_request(endpoint: 'test', http_method: 'get')
+        end
+
+        assert_equal(502, error.http_status)
+      end
+
       def test_set_api_version_in_config
         WhatsappSdk.configure do |config|
           config.api_version = 'v16.0'

test/whatsapp/api/responses/generic_error_response_test.rb

@@ -0,0 +1,33 @@
+# typed: true
+# frozen_string_literal: true
+
+require 'test_helper'
+require 'api/responses/generic_error_response'
+
+module WhatsappSdk
+  module Api
+    module Responses
+      class GenericErrorResponseTest < Minitest::Test
+        def test_response_error_returns_truthy_for_hash_with_error_key
+          response = { "error" => { "message" => "Invalid OAuth token", "code" => 190 } }
+          assert_equal({ "message" => "Invalid OAuth token", "code" => 190 },
+                       GenericErrorResponse.response_error?(response: response))
+        end
+
+        def test_response_error_returns_nil_for_hash_without_error_key
+          response = { "data" => [{ "id" => "1" }] }
+          assert_nil(GenericErrorResponse.response_error?(response: response))
+        end
+
+        def test_response_error_returns_false_for_string_input
+          body = '{"data":[{"text":"si tu cupón te da algún error, avísanos"}]}'
+          assert_equal(false, GenericErrorResponse.response_error?(response: body))
+        end
+
+        def test_response_error_returns_false_for_nil_input
+          assert_equal(false, GenericErrorResponse.response_error?(response: nil))
+        end
+      end
+    end
+  end
+end