Merge #515

515: Fix error propagating logic r=ellnix a=ellnix # Pull Request ## Related issue Fixes #513 ## What does this PR do? - Meilisearch::ApiError parses a server's JSON response into its ``@http_body`` instance variable which stores a hash. - When `version_error_handler` propagates `ApiError`, it passes the ``@http_body`` hash as http body which causes `ApiError` to attempt to parse it again as JSON, throwing a `TypeError`. - This PR makes the `http_body` parameter polymorphic, allowing either a hash or a JSON string to be passed. Co-authored-by: ellnix <103502144+ellnix@users.noreply.github.com>
meilisearch · Jan 29, 2024 · 5e2d354 · 5e2d354
2 parents 5adacb7 + bbcfa7f
commit 5e2d354
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 12 deletions.
diff --git a/lib/meilisearch/error.rb b/lib/meilisearch/error.rb
@@ -21,26 +21,30 @@ class ApiError < Error
     alias link ms_link
 
     def initialize(http_code, http_message, http_body)
-      get_meilisearch_error_info(http_body) unless http_body.nil? || http_body.empty?
       @http_code = http_code
       @http_message = http_message
-      @ms_message ||= 'MeiliSearch API has not returned any error message'
-      @ms_link ||= '<no documentation link found>'
+      @http_body = parse_body(http_body)
+      @ms_code = @http_body['code']
+      @ms_type = @http_body['type']
+      @ms_message = @http_body.fetch('message', 'MeiliSearch API has not returned any error message')
+      @ms_link = @http_body.fetch('link', '<no documentation link found>')
       @message = "#{http_code} #{http_message} - #{@ms_message}. See #{ms_link}."
       super(details)
     end
 
-    def get_meilisearch_error_info(http_body)
-      @http_body = JSON.parse(http_body)
-      @ms_code = @http_body['code']
-      @ms_message = @http_body['message']
-      @ms_type = @http_body['type']
-      @ms_link = @http_body['link']
+    def parse_body(http_body)
+      if http_body.respond_to?(:to_hash)
+        http_body.to_hash
+      elsif http_body.respond_to?(:to_str)
+        JSON.parse(http_body.to_str)
+      else
+        {}
+      end
     rescue JSON::ParserError
       # We might receive a JSON::ParserError when, for example, MeiliSearch is running behind
       # some proxy (ELB or Nginx, for example), and the request timeouts, returning us
       # a raw HTML body instead of a JSON as we were expecting
-      @ms_message = "The server has not returned a valid JSON HTTP body: #{http_body}"
+      { 'message' => "The server has not returned a valid JSON HTTP body: #{http_body}" }
     end
 
     def details

diff --git a/spec/meilisearch/utils_spec.rb b/spec/meilisearch/utils_spec.rb
@@ -67,18 +67,41 @@
   end
 
   describe '.version_error_handler' do
+    let(:http_body) do
+      { 'message' => 'Was expecting an operation',
+        'code' => 'invalid_document_filter',
+        'type' => 'invalid_request',
+        'link' => 'https://docs.meilisearch.com/errors#invalid_document_filter' }
+    end
+
     it 'spawns same error message' do
       expect do
         described_class.version_error_handler(:my_method) do
-          raise MeiliSearch::ApiError.new(405, 'I came from Meili server', {})
+          raise MeiliSearch::ApiError.new(405, 'I came from Meili server', http_body)
+        end
+      end.to raise_error(MeiliSearch::ApiError, /I came from Meili server/)
+    end
+
+    it 'spawns same error message with html body' do
+      expect do
+        described_class.version_error_handler(:my_method) do
+          raise MeiliSearch::ApiError.new(405, 'I came from Meili server', '<html><h1>405 Error</h1></html>')
+        end
+      end.to raise_error(MeiliSearch::ApiError, /I came from Meili server/)
+    end
+
+    it 'spawns same error message with no body' do
+      expect do
+        described_class.version_error_handler(:my_method) do
+          raise MeiliSearch::ApiError.new(405, 'I came from Meili server', nil)
         end
       end.to raise_error(MeiliSearch::ApiError, /I came from Meili server/)
     end
 
     it 'spawns message with version hint' do
       expect do
         described_class.version_error_handler(:my_method) do
-          raise MeiliSearch::ApiError.new(405, 'I came from Meili server', {})
+          raise MeiliSearch::ApiError.new(405, 'I came from Meili server', http_body)
         end
       end.to raise_error(MeiliSearch::ApiError, /that `my_method` call requires/)
     end