Fix error status mapping and improve error handling

lib/zoom/error.rb

@@ -2,19 +2,19 @@
 
 module Zoom
   class Error < StandardError
-    attr_reader :error_hash
+    attr_reader :code, :errors
 
-    def initialize(msg, error_hash={})
-      @error_hash = error_hash
-      super(msg)
+    def initialize(message, code = nil, errors = nil)
+      @code = code
+      @errors = errors
+      super(message)
     end
   end
   class GatewayTimeout < Error; end
   class NotImplemented < Error; end
   class ParameterMissing < Error; end
   class ParameterNotPermitted < Error; end
   class ParameterValueNotPermitted < Error; end
-  class AuthenticationError < Error; end
   class BadRequest < Error; end
   class Unauthorized < Error; end
   class Forbidden < Error; end

lib/zoom/utils.rb

@@ -11,31 +11,34 @@ def exclude_argument_error(name)
         name ? ArgumentError.new("Unrecognized parameter #{name}") : ArgumentError.new
       end
 
-      def raise_if_error!(response, http_code=200)
-        return response unless response.is_a?(Hash) && response.key?('code')
-
+      def raise_error(response, http_code=nil)
         code = response['code']
-        error_hash = build_error(response)
-
-        raise AuthenticationError, error_hash if code == 124
-        raise BadRequest, error_hash if code == 400
-        raise Unauthorized, error_hash if code == 401
-        raise Forbidden, error_hash if code == 403
-        raise NotFound, error_hash if code == 404
-        raise Conflict, error_hash if code == 409
-        raise TooManyRequests, error_hash if code == 429
-        raise InternalServerError, error_hash if code == 500
-        raise Error.new(error_hash, error_hash)
-      end
+        message = response['message']
+        errors = response['errors']
 
-      def build_error(response)
-        error_hash = { base: response['message']}
-        error_hash[response['message']] = response['errors'] if response['errors']
-        error_hash
+        case http_code
+        when 400
+          raise BadRequest.new(message, code, errors)
+        when 401
+          raise Unauthorized.new(message, code, errors)
+        when 403
+          raise Forbidden.new(message, code, errors)
+        when 404
+          raise NotFound.new(message, code, errors)
+        when 409
+          raise Conflict.new(message, code, errors)
+        when 429
+          raise TooManyRequests.new(message, code, errors)
+        when 500
+          raise InternalServerError.new(message, code, errors)
+        else
+          raise Error.new(message, code, errors)
+        end
       end
 
       def parse_response(http_response)
-        raise_if_error!(http_response.parsed_response, http_response.code) || http_response.code
+        return http_response.parsed_response || http_response.code if http_response.success?
+        raise_error(http_response.parsed_response, http_response.code)
       end
 
       def extract_options!(array)

spec/fixtures/error/unauthorized_request.json

@@ -1,4 +1,4 @@
 {
-  "code": 400,
-  "message": "Unauthorized request. You do not have permission to access this user’s channel information."
+  "code": 200,
+  "message": "Unauthorized request. You do not have permission to access this user's channel information."
 }

spec/lib/zoom/actions/groups/get_spec.rb

@@ -36,9 +36,11 @@
       end
 
       it 'raises Zoom::Error exception with text' do
-        expect { zc.groups_get(wrong_args) }.to raise_error(Zoom::Error, {
-          base: 'A group with this 999999 does not exist.'
-        }.to_s)
+        expect { zc.groups_get(wrong_args) }.to raise_error { |error|
+          expect(error).to be_a(Zoom::NotFound)
+          expect(error.message).to eq('A group with this 999999 does not exist.')
+          expect(error.code).to eq(4130)
+        }
       end
     end
 
@@ -53,9 +55,11 @@
       end
 
       it 'raises Zoom::Error exception with text' do
-        expect { zc.groups_get(wrong_args) }.to raise_error(Zoom::Error, {
-          base: 'Group does not belong to your account.'
-        }.to_s)
+        expect { zc.groups_get(wrong_args) }.to raise_error { |error|
+          expect(error).to be_a(Zoom::BadRequest)
+          expect(error.message).to eq('Group does not belong to your account.')
+          expect(error.code).to eq(1010)
+        }
       end
     end
   end

spec/lib/zoom/actions/groups/list_spec.rb

@@ -38,9 +38,11 @@
       end
 
       it 'raises Zoom::Error exception with text' do
-        expect { zc.groups_list }.to raise_error(Zoom::Error, {
-          base: 'A group with this 999999 does not exist.'
-        }.to_s)
+        expect { zc.groups_list }.to raise_error { |error|
+          expect(error).to be_a(Zoom::NotFound)
+          expect(error.message).to eq('A group with this 999999 does not exist.')
+          expect(error.code).to eq(4130)
+        }
       end
     end
   end

spec/lib/zoom/actions/im/chat/channels/get_spec.rb

@@ -38,9 +38,11 @@
       end
 
       it 'raises Zoom::Error exception' do
-        expect { zc.get_chat_channels(args) }.to raise_error(Zoom::BadRequest, {
-          base: "Unauthorized request. You do not have permission to access this user’s channel information."
-        }.to_s)
+        expect { zc.get_chat_channels(args) }.to raise_error { |error|
+          expect(error).to be_a(Zoom::BadRequest)
+          expect(error.message).to eq("Unauthorized request. You do not have permission to access this user's channel information.")
+          expect(error.code).to eq(200)
+        }
       end
     end
 
@@ -55,9 +57,11 @@
       end
 
       it 'returns a hash 404' do
-        expect { zc.get_chat_channels(missed_channel_args) }.to raise_error(Zoom::Error, {
-          base: "Channel does not exist: #{missed_channel_args[:channel_id]}"
-        }.to_s)
+        expect { zc.get_chat_channels(missed_channel_args) }.to raise_error { |error|
+          expect(error).to be_a(Zoom::NotFound)
+          expect(error.message).to eq("Channel does not exist: #{missed_channel_args[:channel_id]}")
+          expect(error.code).to eq(4130)
+        }
       end
     end
   end

spec/lib/zoom/actions/im/chat/users/channels/get_spec.rb

@@ -36,9 +36,11 @@
       end
 
       it 'raises Zoom::Error exception with text' do
-        expect { zc.get_chat_user_channels(args) }.to raise_error(Zoom::Error, {
-          base: 'The next page token is either invalid or has expired.'
-        }.to_s)
+        expect { zc.get_chat_user_channels(args) }.to raise_error { |error|
+          expect(error).to be_a(Zoom::BadRequest)
+          expect(error.message).to eq('The next page token is either invalid or has expired.')
+          expect(error.code).to eq(300)
+        }
       end
     end
   end

spec/lib/zoom/actions/meeting/update_spec.rb

@@ -44,7 +44,11 @@
       it 'raises an error' do
         expect {
           zc.meeting_update(args.merge(meeting_id: 'invalid-meeting-id'))
-        }.to raise_error(Zoom::Error, { base: "Invalid meeting id." }.to_s)
+        }.to raise_error { |error|
+          expect(error).to be_a(Zoom::NotFound)
+          expect(error.message).to eq('Invalid meeting id.')
+          expect(error.code).to eq(300)
+        }
       end
     end
   end

spec/lib/zoom/actions/webinar/create_spec.rb

@@ -15,7 +15,8 @@
             stub_request(
               :post,
               zoom_url("/users/#{args[:host_id]}/webinars")
-            ).to_return(body: json_response('webinar', 'create'),
+            ).to_return(status: 201,
+                        body: json_response('webinar', 'create'),
                         headers: { 'Content-Type' => 'application/json' })
           end
 
@@ -45,7 +46,8 @@
             stub_request(
               :post,
               zoom_url("/users/#{args[:host_id]}/webinars")
-            ).to_return(body: json_response('error', 'validation'),
+            ).to_return(status: 400,
+                        body: json_response('error', 'validation'),
                         headers: { 'Content-Type' => 'application/json' })
           end
 

spec/lib/zoom/actions/webinar/list_spec.rb

@@ -12,7 +12,8 @@
         stub_request(
           :get,
           zoom_url("/users/#{args[:host_id]}/webinars")
-        ).to_return(body: json_response('webinar', 'list'),
+        ).to_return(status: 200,
+                    body: json_response('webinar', 'list'),
                     headers: { 'Content-Type' => 'application/json' })
       end
 
@@ -38,7 +39,8 @@
         stub_request(
           :get,
           zoom_url("/users/#{args[:host_id]}/webinars")
-        ).to_return(body: json_response('error', 'validation'),
+        ).to_return(status: 400,
+                    body: json_response('error', 'validation'),
                     headers: { 'Content-Type' => 'application/json' })
       end
 

spec/lib/zoom/actions/webinar/poll_get_spec.rb

@@ -12,7 +12,7 @@
         stub_request(
           :get,
           zoom_url("/webinars/#{args[:webinar_id]}/polls/#{args[:poll_id]}")
-        ).to_return(status: :ok,
+        ).to_return(status: 200,
                     body: json_response('webinar', 'poll_get'),
                     headers: { 'Content-Type' => 'application/json' })
       end
@@ -35,7 +35,8 @@
         stub_request(
           :get,
           zoom_url("/webinars/#{args[:webinar_id]}/polls/#{args[:poll_id]}")
-        ).to_return(body: json_response('error', 'validation'),
+        ).to_return(status: 400,
+                    body: json_response('error', 'validation'),
                     headers: { 'Content-Type' => 'application/json' })
       end
 

spec/lib/zoom/actions/webinar/polls_list_spec.rb

@@ -12,7 +12,7 @@
         stub_request(
           :get,
           zoom_url("/webinars/#{args[:webinar_id]}/polls")
-        ).to_return(status: :ok,
+        ).to_return(status: 200,
                     body: json_response('webinar', 'polls_list'),
                     headers: { 'Content-Type' => 'application/json' })
       end

spec/lib/zoom/utils_spec.rb

@@ -14,60 +14,77 @@ class Utils < Zoom::Utils; end
     end
   end
 
-  describe '#raise_if_error!' do
-    it 'raises Zoom::AuthenticationError if error is present and code = 124' do
-      response = { 'code' => 124, 'message' => 'Authentication error.' }
-      expect { Utils.raise_if_error!(response) }.to raise_error(Zoom::AuthenticationError)
+  describe '#raise_error' do
+    it 'raises Zoom::BadRequest if error is present and http code = 400' do
+      response = { 'code' => 123, 'message' => 'Bad request.' }
+      expect { Utils.raise_error(response, 400) }.to raise_error(Zoom::BadRequest)
     end
 
-    it 'raises Zoom::BadRequest if error is present and code = 400' do
-      response = { 'code' => 400, 'message' => 'Bas request.' }
-      expect { Utils.raise_if_error!(response) }.to raise_error(Zoom::BadRequest)
+    it 'raises Zoom::Unauthorized if error is present and http code = 401' do
+      response = { 'code' => 123, 'message' => 'Unauthorized.' }
+      expect { Utils.raise_error(response, 401) }.to raise_error(Zoom::Unauthorized)
     end
 
-    it 'raises Zoom::Unauthorized if error is present and code = 401' do
-      response = { 'code' => 401, 'message' => 'Unauthorized.' }
-      expect { Utils.raise_if_error!(response) }.to raise_error(Zoom::Unauthorized)
+    it 'raises Zoom::Forbidden if error is present and http code = 403' do
+      response = { 'code' => 123, 'message' => 'Forbidden.' }
+      expect { Utils.raise_error(response, 403) }.to raise_error(Zoom::Forbidden)
     end
 
-    it 'raises Zoom::Forbidden if error is present and code = 403' do
-      response = { 'code' => 403, 'message' => 'Forbidden.' }
-      expect { Utils.raise_if_error!(response) }.to raise_error(Zoom::Forbidden)
+    it 'raises Zoom::NotFound if error is present and http code = 404' do
+      response = { 'code' => 123, 'message' => 'NotFound.' }
+      expect { Utils.raise_error(response, 404) }.to raise_error(Zoom::NotFound)
     end
 
-    it 'raises Zoom::NotFound if error is present and code = 404' do
-      response = { 'code' => 404, 'message' => 'NotFound.' }
-      expect { Utils.raise_if_error!(response) }.to raise_error(Zoom::NotFound)
+    it 'raises Zoom::Conflict if error is present and http code = 409' do
+      response = { 'code' => 123, 'message' => 'Conflict.' }
+      expect { Utils.raise_error(response, 409) }.to raise_error(Zoom::Conflict)
     end
 
-    it 'raises Zoom::Conflict if error is present and code = 409' do
-      response = { 'code' => 409, 'message' => 'Conflict.' }
-      expect { Utils.raise_if_error!(response) }.to raise_error(Zoom::Conflict)
+    it 'raises Zoom::TooManyRequests if error is present and http code = 429' do
+      response = { 'code' => 123, 'message' => 'Too many requests.' }
+      expect { Utils.raise_error(response, 429) }.to raise_error(Zoom::TooManyRequests)
     end
 
-    it 'raises Zoom::TooManyRequests if error is present and code = 429' do
-      response = { 'code' => 429, 'message' => 'Too many requests.' }
-      expect { Utils.raise_if_error!(response) }.to raise_error(Zoom::TooManyRequests)
+    it 'raises Zoom::InternalServerError if error is present and http code = 500' do
+      response = { 'code' => 123, 'message' => 'Internal server error.' }
+      expect { Utils.raise_error(response, 500) }.to raise_error(Zoom::InternalServerError)
     end
 
-    it 'raises Zoom::InternalServerError if error is present and code = 500' do
-      response = { 'code' => 500, 'message' => 'Internal server error.' }
-      expect { Utils.raise_if_error!(response) }.to raise_error(Zoom::InternalServerError)
+    it 'raises Zoom::Error if http code is not in [400, 401, 403, 404, 429, 500]' do
+      response = { 'code' => 180, 'message' => 'lol error' }
+      expect { Utils.raise_error(response, 101) }.to raise_error(Zoom::Error)
     end
 
-    it 'does not raise Zoom::Error if error is not present' do
-      response = {}
-      expect { Utils.raise_if_error!(response) }.to_not raise_error
+    it 'raises Zoom::Error if http code is not in [400, 401, 403, 404, 429, 500]' do
+      response = { 'code' => 180, 'message' => 'lol error' }
+      expect { Utils.raise_error(response, 101) }.to raise_error(Zoom::Error)
     end
 
-    it 'does not raise Zoom::Error if response is not a Hash' do
-      response = 'xxx'
-      expect { Utils.raise_if_error!(response) }.to_not raise_error
+    it 'extracts error attributes from response' do
+      response = json_response('error', 'validation')
+      expect { Utils.raise_error(response, 400) }.to raise_error { |error|
+        expect(error.message).to eq(response['message'])
+        expect(error.code).to eq(response['code'])
+        expect(error.errors).to eq(response['errors'])
+      }
     end
+  end
 
-    it 'raises Zoom::Error if http code is not in [124, 400, 401, 403, 404, 429, 500]' do
-      response = { 'code' => 180, 'message' => 'lol error' }
-      expect { Utils.raise_if_error!(response, 400) }.to raise_error(Zoom::Error)
+  describe '#parse_response' do
+    it 'returns response if http response is successful' do
+      parsed_response = json_response('account', 'get')
+      response = double('Response', success?: true, parsed_response: parsed_response, code: 200)
+      expect(Utils.parse_response(response)).to eq(parsed_response)
+    end
+
+    it 'returns http status code if http response is successful and parsed response is nil' do
+      response = double('Response', success?: true, parsed_response: nil, code: 200)
+      expect(Utils.parse_response(response)).to eq(200)
+    end
+
+    it 'does not raise Zoom::Error if response is not a Hash' do
+      response = double('Response', success?: true, parsed_response: 'xxx', code: 200)
+      expect { Utils.parse_response(response) }.to_not raise_error
     end
   end