Improve error handler (#65)

* Use the new error handler of MeiliSearch API

* Improve the tests to check MS code and type

* Add comments

* Fix linter errors

* Add MeiliSearch::CommunicationError

* Upd rubocop_todo

* Remove useless byebug

* Change minor naming

* Remove useless error in get_index

* Update lib/meilisearch/error.rb

Co-authored-by: Thomas Payet <[email protected]>

* Update lib/meilisearch/error.rb

Co-authored-by: Thomas Payet <[email protected]>

Co-authored-by: Thomas Payet <[email protected]>

Author: curquiza

.rubocop_todo.yml

@@ -1,33 +1,28 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2020-06-04 16:06:38 +0200 using RuboCop version 0.85.0.
+# on 2020-07-05 14:27:43 UTC using RuboCop version 0.86.0.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
 # versions of RuboCop, may require this file to be generated again.
 
-# Offense count: 20
+# Offense count: 19
 # Configuration parameters: CountComments, ExcludedMethods.
 # ExcludedMethods: refine
 Metrics/BlockLength:
-  Max: 463
+  Max: 465
 
 # Offense count: 1
 # Configuration parameters: CountComments.
 Metrics/ClassLength:
   Max: 190
 
-# Offense count: 2
-# Configuration parameters: CountComments, ExcludedMethods.
-Metrics/MethodLength:
-  Max: 11
-
 # Offense count: 1
 Naming/AccessorMethodName:
   Exclude:
     - 'lib/meilisearch/index.rb'
 
-# Offense count: 6
+# Offense count: 7
 Style/Documentation:
   Exclude:
     - 'spec/**/*'

lib/meilisearch/client.rb

@@ -30,8 +30,6 @@ def delete_index(index_uid)
     # Usage:
     # client.index('indexUID')
     def index(index_uid)
-      raise IndexUidError if index_uid.nil?
-
       index_object(index_uid)
     end
     alias get_index index

lib/meilisearch/error.rb

@@ -1,45 +1,58 @@
 # frozen_string_literal: true
 
 module MeiliSearch
-  class MeiliSearchError < StandardError; end
-  class IndexUidError < MeiliSearchError; end
-  class MeiliSearchTimeoutError < MeiliSearchError
-    attr_reader :message
+  class ApiError < StandardError
+    attr_reader :http_code    # e.g. 400, 404...
+    attr_reader :http_message # e.g. Bad Request, Not Found...
+    attr_reader :http_body    # The response body received from the MeiliSearch API
+    attr_reader :ms_code      # The error code given by the MeiliSearch API
+    attr_reader :ms_type      # The error type given by the MeiliSearch API
+    attr_reader :ms_link      # The documentation link given by the MeiliSearch API
+    attr_reader :ms_message   # The error message given by the MeiliSearch API
+    attr_reader :message      # The detailed error message of this error class
 
-    def initialize
-      @message = "MeiliSearchTimeoutError: update wasn't processed in the expected time"
+    alias code ms_code
+    alias type ms_type
+    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>'
+      @message = "#{http_code} #{http_message} - #{@ms_message.capitalize}. See #{ms_link}."
+      super(details)
+    end
+
+    def get_meilisearch_error_info(http_body)
+      @http_body = JSON.parse(http_body)
+      @ms_code = @http_body['errorCode']
+      @ms_message = @http_body['message']
+      @ms_type = @http_body['errorType']
+      @ms_link = @http_body['errorLink']
     end
 
-    def to_s
-      "#{@message}."
+    def details
+      "MeiliSearch::ApiError - code: #{@ms_code} - type: #{ms_type} - message: #{@ms_message} - link: #{ms_link}"
     end
   end
 
-  class HTTPError < MeiliSearchError
-    attr_reader :status
+  class CommunicationError < StandardError
     attr_reader :message
-    attr_reader :http_body
-    attr_reader :http_body_message
-    attr_reader :details
-
-    alias code         status
-    alias body         http_body
-    alias body_message http_body_message
-
-    def initialize(status, message, http_body, details = nil)
-      @status = status
-      unless http_body.nil? || http_body.empty?
-        @http_body = JSON.parse(http_body)
-        @http_body_message = @http_body['message']
-      end
-      @message = message.capitalize
-      @message = "#{@message} - #{@http_body_message.capitalize}" unless @http_body_message.nil?
-      @details = details
+
+    def initialize(message)
+      @message = "An error occurred while trying to connect to the MeiliSearch instance: #{message}"
+      super(@message)
     end
+  end
 
-    def to_s
-      final_message = @details.nil? ? @message : "#{@message}. #{@details}"
-      "#{@status}: #{final_message}."
+  class TimeoutError < StandardError
+    attr_reader :message
+
+    def initialize
+      @message = 'The update was not processed in the expected time'
+      super(@message)
     end
   end
 end

lib/meilisearch/http_request.rb

@@ -16,57 +16,63 @@ def initialize(url, api_key = nil)
       }.compact
     end
 
-    def http_get(path = '', query_params = {})
-      response = self.class.get(
-        @base_url + path,
-        query: query_params,
-        headers: @headers,
-        timeout: 1
+    def http_get(relative_path = '', query_params = {})
+      send_request(
+        proc { |path, config| self.class.get(path, config) },
+        relative_path,
+        query_params
       )
-      validate(response)
     end
 
-    def http_post(path = '', body = nil, query_params = nil)
-      body = body.to_json unless body.nil?
-      response = self.class.post(
-        @base_url + path,
-        {
-          body: body,
-          query: query_params,
-          headers: @headers,
-          timeout: 1
-        }.compact
+    def http_post(relative_path = '', body = nil, query_params = nil)
+      send_request(
+        proc { |path, config| self.class.post(path, config) },
+        relative_path,
+        query_params,
+        body
       )
-      validate(response)
     end
 
-    def http_put(path = '', body = nil, query_params = nil)
-      body = body.to_json unless body.nil?
-      response = self.class.put(
-        @base_url + path,
-        {
-          body: body,
-          query: query_params,
-          headers: @headers,
-          timeout: 1
-        }.compact
+    def http_put(relative_path = '', body = nil, query_params = nil)
+      send_request(
+        proc { |path, config| self.class.put(path, config) },
+        relative_path,
+        query_params,
+        body
       )
-      validate(response)
     end
 
-    def http_delete(path = '')
-      response = self.class.delete(
-        @base_url + path,
-        headers: @headers,
-        timeout: 1
+    def http_delete(relative_path = '')
+      send_request(
+        proc { |path, config| self.class.delete(path, config) },
+        relative_path
       )
-      validate(response)
     end
 
     private
 
+    def send_request(http_method, relative_path, query_params = nil, body = nil)
+      config = http_config(query_params, body)
+      begin
+        response = http_method.call(@base_url + relative_path, config)
+      rescue Errno::ECONNREFUSED => e
+        raise CommunicationError, e.message
+      end
+      validate(response)
+    end
+
+    def http_config(query_params, body)
+      body = body.to_json unless body.nil?
+      {
+        headers: @headers,
+        query: query_params,
+        body: body,
+        timeout: 1
+      }.compact
+    end
+
     def validate(response)
-      raise HTTPError.new(response.code, response.message, response.body) unless response.success?
+      raise ApiError.new(response.code, response.message, response.body) unless response.success?
 
       response.parsed_response
     end

lib/meilisearch/index.rb

@@ -113,7 +113,7 @@ def wait_for_pending_update(update_id, timeout_in_ms = 5000, interval_in_ms = 50
         end
       end
     rescue Timeout::Error
-      raise MeiliSearch::MeiliSearchTimeoutError
+      raise MeiliSearch::TimeoutError
     end
 
     ### STATS

spec/meilisearch/client/indexes_spec.rb

@@ -34,13 +34,13 @@
   it 'fails to create an index with an uid already taken' do
     expect do
       @client.create_index(@uid1)
-    end.to raise_meilisearch_http_error_with(400)
+    end.to raise_meilisearch_api_error_with(400, 'index_already_exists', 'invalid_request_error')
   end
 
   it 'fails to create an index with bad UID format' do
     expect do
       @client.create_index('two words')
-    end.to raise_meilisearch_http_error_with(400)
+    end.to raise_meilisearch_api_error_with(400, 'invalid_index_uid', 'invalid_request_error')
   end
 
   it 'gets list of indexes' do
@@ -67,11 +67,11 @@
 
   it 'deletes index' do
     expect(@client.delete_index(@uid1)).to be_nil
-    expect { @client.show_index(@uid1) }.to raise_meilisearch_http_error_with(404)
+    expect { @client.show_index(@uid1) }.to raise_index_not_found_meilisearch_api_error
     expect(@client.delete_index(@uid2)).to be_nil
-    expect { @client.show_index(@uid2) }.to raise_meilisearch_http_error_with(404)
+    expect { @client.show_index(@uid2) }.to raise_index_not_found_meilisearch_api_error
     expect(@client.delete_index(@uid3)).to be_nil
-    expect { @client.show_index(@uid3) }.to raise_meilisearch_http_error_with(404)
+    expect { @client.show_index(@uid3) }.to raise_index_not_found_meilisearch_api_error
     expect(@client.indexes.count).to eq(0)
   end
 

spec/meilisearch/client/keys_spec.rb

@@ -21,18 +21,24 @@
   it 'fails to get settings if public key used' do
     public_key = @client.keys['public']
     new_client = MeiliSearch::Client.new($URL, public_key)
-    expect { new_client.index(@uid).settings }.to raise_meilisearch_http_error_with(403)
+    expect do
+      new_client.index(@uid).settings
+    end.to raise_meilisearch_api_error_with(403, 'invalid_token', 'authentication_error')
   end
 
   it 'fails to get keys if private key used' do
     private_key = @client.keys['private']
     new_client = MeiliSearch::Client.new($URL, private_key)
-    expect { new_client.keys }.to raise_meilisearch_http_error_with(403)
+    expect do
+      new_client.keys
+    end.to raise_meilisearch_api_error_with(403, 'invalid_token', 'authentication_error')
   end
 
   it 'fails to search if no key used' do
     new_client = MeiliSearch::Client.new($URL)
-    expect { new_client.index(@uid).settings }.to raise_meilisearch_http_error_with(401)
+    expect do
+      new_client.index(@uid).settings
+    end.to raise_meilisearch_api_error_with(401, 'missing_authorization_header', 'authentication_error')
   end
 
   it 'succeeds to search when using public key' do

spec/meilisearch/index/base_spec.rb

@@ -44,21 +44,22 @@
     new_primary_key = 'id_test'
     expect do
       @index2.update(primaryKey: new_primary_key)
-    end.to raise_meilisearch_http_error_with(400)
+    end.to raise_bad_request_meilisearch_api_error
+    # temporary, will be raise a 'primary_key_already_present' code in the next MS release
   end
 
   it 'deletes index' do
     expect(@index1.delete).to be_nil
-    expect { @index1.show }.to raise_meilisearch_http_error_with(404)
+    expect { @index1.show }.to raise_index_not_found_meilisearch_api_error
     expect(@index2.delete).to be_nil
-    expect { @index2.show }.to raise_meilisearch_http_error_with(404)
+    expect { @index2.show }.to raise_index_not_found_meilisearch_api_error
   end
 
   it 'fails to manipulate index object after deletion' do
-    expect { @index2.primary_key }.to raise_meilisearch_http_error_with(404)
-    expect { @index2.show }.to raise_meilisearch_http_error_with(404)
-    expect { @index2.update(primaryKey: 'id_test') }.to raise_meilisearch_http_error_with(404)
-    expect { @index2.delete }.to raise_meilisearch_http_error_with(404)
+    expect { @index2.primary_key }.to raise_index_not_found_meilisearch_api_error
+    expect { @index2.show }.to raise_index_not_found_meilisearch_api_error
+    expect { @index2.update(primaryKey: 'id_test') }.to raise_index_not_found_meilisearch_api_error
+    expect { @index2.delete }.to raise_index_not_found_meilisearch_api_error
   end
 
   it 'works with method aliases' do

spec/meilisearch/index/documents_spec.rb

@@ -150,7 +150,7 @@
       expect(response).to be_a(Hash)
       expect(response).to have_key('updateId')
       expect(index.documents.size).to eq(documents.count - 1)
-      expect { index.document(id) }.to raise_meilisearch_http_error_with(404)
+      expect { index.document(id) }.to raise_document_not_found_meilisearch_api_error
     end
 
     it 'does nothing when trying to delete a document which does not exist' do
@@ -160,7 +160,7 @@
       expect(response).to be_a(Hash)
       expect(response).to have_key('updateId')
       expect(index.documents.size).to eq(documents.count - 1)
-      expect { index.document(id) }.to raise_meilisearch_http_error_with(404)
+      expect { index.document(id) }.to raise_document_not_found_meilisearch_api_error
     end
 
     it 'deletes one document from index (with delete-batch route)' do
@@ -170,7 +170,7 @@
       expect(response).to be_a(Hash)
       expect(response).to have_key('updateId')
       expect(index.documents.size).to eq(documents.count - 2)
-      expect { index.document(id) }.to raise_meilisearch_http_error_with(404)
+      expect { index.document(id) }.to raise_document_not_found_meilisearch_api_error
     end
 
     it 'deletes one document from index (with delete-batch route as an array of one uid)' do
@@ -180,7 +180,7 @@
       expect(response).to be_a(Hash)
       expect(response).to have_key('updateId')
       expect(index.documents.size).to eq(documents.count - 3)
-      expect { index.document(id) }.to raise_meilisearch_http_error_with(404)
+      expect { index.document(id) }.to raise_document_not_found_meilisearch_api_error
     end
 
     it 'deletes multiples documents from index' do
@@ -347,7 +347,10 @@
     end
 
     it 'returns a 400' do
-      expect { index.add_documents(documents) }.to raise_meilisearch_http_error_with(400)
+      expect do
+        index.add_documents(documents)
+      end.to raise_bad_request_meilisearch_api_error
+      # temporary, will be return a 'missing_primary_key' code in the next MS release
     end
   end