feat: add semantic error classes

lib/zammad_api.rb

@@ -1,4 +1,5 @@
 require 'zammad_api/version'
+require 'zammad_api/errors'
 require 'zammad_api/client'
 
 module ZammadAPI

lib/zammad_api/client.rb

@@ -31,28 +31,28 @@ def method_missing(method, *_args)
       begin
         class_object = Kernel.const_get(class_name)
       rescue
-        raise "Resource for #{method} does not exist"
+        raise ResourceNotFoundError, "Resource for #{method} does not exist"
       end
       ZammadAPI::Dispatcher.new(@transport, class_object)
     end
 
     private
 
     def check_config
-      raise 'missing url in config' if !@config[:url]
-      raise 'config url needs to start with http:// or https://' if !%r{^(http|https)://}.match?(@config[:url])
+      raise ConfigurationError, 'missing url in config' if !@config[:url]
+      raise ConfigurationError, 'config url needs to start with http:// or https://' if !%r{^(http|https)://}.match?(@config[:url])
 
       # check for token auth
       return if @config[:http_token] && !@config[:http_token].empty?
       return if @config[:oauth2_token] && !@config[:oauth2_token].empty?
 
       if !@config[:user] || @config[:user].empty?
-        raise 'missing user in config'
+        raise ConfigurationError, 'missing user in config'
       end
 
       return if @config[:password] && !@config[:password].empty?
 
-      raise 'missing password in config'
+      raise ConfigurationError, 'missing password in config'
     end
 
     def modulize(string)

lib/zammad_api/errors.rb

@@ -0,0 +1,62 @@
+require 'json'
+
+module ZammadAPI
+  class Error < RuntimeError; end
+
+  class ConfigurationError < Error; end
+
+  class ResourceNotFoundError < Error; end
+
+  class ResponseError < Error
+    attr_reader :response, :operation, :resource_class
+
+    def initialize(operation:, response:, resource_class: nil)
+      @operation      = operation
+      @response       = response
+      @resource_class = resource_class
+      super(default_message)
+    end
+
+    def status
+      response&.status
+    end
+
+    def body
+      response&.body
+    end
+
+    def self.from(response, **kwargs)
+      klass = if response.nil?
+                self
+              elsif response.status >= 500
+                ServerError
+              else
+                ClientError
+              end
+      klass.new(response: response, **kwargs)
+    end
+
+    private
+
+    def default_message
+      subject = resource_class ? "#{operation} (#{resource_class.name})" : operation
+      "Can't #{subject}: #{detail}"
+    end
+
+    def detail
+      body_error || "HTTP #{status}"
+    end
+
+    def body_error
+      return nil if body.to_s.strip.empty?
+
+      parsed = JSON.parse(body)
+      parsed.is_a?(Hash) ? parsed['error'] : nil
+    rescue JSON::ParserError
+      nil
+    end
+  end
+
+  class ClientError < ResponseError; end
+  class ServerError < ResponseError; end
+end

lib/zammad_api/list_base.rb

@@ -62,11 +62,12 @@ def request(request, url, parameter)
       }.join('&')
 
       response = @transport.get(url: url)
-      data = safe_json_parse(response.body)
       if response.status != 200
-        raise "Can't get .#{request} of object (#{@resource.class.name}): #{data['error']}"
+        raise ResponseError.from(response, operation: "get .#{request} of object", resource_class: @resource.class)
       end
 
+      data = safe_json_parse(response.body)
+
       list = []
       data.each do |local_data|
         item = @resource.new(@transport, local_data)
@@ -77,7 +78,7 @@ def request(request, url, parameter)
     end
 
     def perform_request(_parameter)
-      raise "no perform_request implementation for #{self.class.name} found"
+      raise Error, "no perform_request implementation for #{self.class.name} found"
     end
   end
 end

lib/zammad_api/resources/base.rb

@@ -43,12 +43,9 @@ def changed?
 
       def destroy
         response = @transport.delete(url: "#{@url}/#{@attributes[:id]}")
-        if response.body.to_s != '' && response.body.to_s != ' '
-          data = safe_json_parse(response.body)
-        end
         return true if response.status == 200
 
-        raise "Can't destroy object (#{self.class.name}): #{data['error']}"
+        raise ResponseError.from(response, operation: 'destroy object', resource_class: self.class)
       end
 
       def save
@@ -79,11 +76,11 @@ def self.search(transport, parameter)
 
       def self.find(transport, id)
         response = transport.get(url: "#{@url}/#{id}?expand=true")
-        data = safe_json_parse(response.body)
         if response.status != 200
-          raise "Can't find object (#{self.class.name}): #{data['error']}"
+          raise ResponseError.from(response, operation: 'find object', resource_class: self)
         end
 
+        data = safe_json_parse(response.body)
         item = new(transport, data)
         item.new_instance = false
         item
@@ -110,28 +107,25 @@ def saved_attributes
       end
 
       def save_new
-        response   = @transport.post(url: "#{@url}?expand=true", params: @attributes)
-        attributes = safe_json_parse(response.body)
-        return attributes if response.status == 201
+        response = @transport.post(url: "#{@url}?expand=true", params: @attributes)
+        return safe_json_parse(response.body) if response.status == 201
 
-        save_error(attributes)
+        save_error(response)
       end
 
       def save_existing
         attributes_to_post = {}
         @changes.each do |name, values|
           attributes_to_post[name] = values[1]
         end
-        response   = @transport.put(url: "#{@url}/#{@attributes[:id]}?expand=true", params: attributes_to_post)
-        attributes = safe_json_parse(response.body)
-
-        return attributes if response.status == 200
+        response = @transport.put(url: "#{@url}/#{@attributes[:id]}?expand=true", params: attributes_to_post)
+        return safe_json_parse(response.body) if response.status == 200
 
-        save_error(attributes)
+        save_error(response)
       end
 
-      def save_error(attributes)
-        raise "Can't save object (#{self.class.name}): #{attributes['error']}"
+      def save_error(response)
+        raise ResponseError.from(response, operation: 'save object', resource_class: self.class)
       end
 
       def symbolize_keys_deep!(hash)

lib/zammad_api/resources/ticket.rb

@@ -3,11 +3,12 @@ class ZammadAPI::Resources::Ticket < ZammadAPI::Resources::Base
 
   def articles
     response = @transport.get(url: "/api/v1/ticket_articles/by_ticket/#{id}?expand=true")
-    data = safe_json_parse(response.body)
     if response.status != 200
-      raise "Can't get articles (#{self.class.name}): #{data['error']}"
+      raise ZammadAPI::ResponseError.from(response, operation: 'get articles', resource_class: self.class)
     end
 
+    data = safe_json_parse(response.body)
+
     data.collect do |raw|
       item = ZammadAPI::Resources::TicketArticle.new(@transport, raw)
       item.new_instance = false

lib/zammad_api/resources/ticket_article_attachment.rb

@@ -13,7 +13,6 @@ def download
     response = @transport.get(url: "/api/v1/ticket_attachment/#{ticket_id}/#{article_id}/#{id}")
     return response.body if response.status == 200
 
-    data = safe_json_parse(response.body)
-    raise "Can't get articles (#{self.class.name}): #{data['error']}"
+    raise ZammadAPI::ResponseError.from(response, operation: 'get articles', resource_class: self.class)
   end
 end

spec/zammad_api/client_spec.rb

@@ -17,6 +17,48 @@
   let(:config) { Helper.config }
   let(:instance) { described_class.new(config) }
 
+  describe '.new' do
+    it 'raises ConfigurationError when url is missing' do
+      expect { described_class.new(config.merge(url: nil)) }
+        .to raise_error(ZammadAPI::ConfigurationError, 'missing url in config')
+    end
+
+    it 'raises ConfigurationError when url scheme is unsupported' do
+      expect { described_class.new(config.merge(url: 'ftp://example.com')) }
+        .to raise_error(ZammadAPI::ConfigurationError, 'config url needs to start with http:// or https://')
+    end
+
+    it 'raises ConfigurationError when user is missing' do
+      expect { described_class.new(config.merge(user: nil)) }
+        .to raise_error(ZammadAPI::ConfigurationError, 'missing user in config')
+    end
+
+    it 'raises ConfigurationError when password is missing' do
+      expect { described_class.new(config.merge(password: nil)) }
+        .to raise_error(ZammadAPI::ConfigurationError, 'missing password in config')
+    end
+
+    it 'does not require user/password when http_token is supplied' do
+      expect { described_class.new(url: config[:url], http_token: 'token') }.not_to raise_error
+    end
+
+    it 'does not require user/password when oauth2_token is supplied' do
+      expect { described_class.new(url: config[:url], oauth2_token: 'token') }.not_to raise_error
+    end
+  end
+
+  describe '#method_missing' do
+    it 'raises ResourceNotFoundError for unknown resources' do
+      expect { instance.does_not_exist }.to raise_error(ZammadAPI::ResourceNotFoundError, /Resource for DoesNotExist does not exist/)
+    end
+
+    it 'attaches the underlying NameError as #cause' do
+      instance.does_not_exist
+    rescue ZammadAPI::ResourceNotFoundError => e
+      expect(e.cause).to be_a(NameError)
+    end
+  end
+
   describe '#perform_on_behalf_of' do
     it 'performs a given block on behalft of a given user' do
       on_behalf_of_identifier = 'some_login'