diff --git a/Gemfile b/Gemfile index 2573fc1..2833a20 100644 --- a/Gemfile +++ b/Gemfile @@ -8,5 +8,5 @@ git_source(:github) { |repo_name| "https://github.com/#{repo_name}" } gemspec group :test do - gem 'faker', git: 'https://github.com/stympy/faker.git', branch: 'master' + gem 'faker' end diff --git a/lib/onesignal/client.rb b/lib/onesignal/client.rb index d5b2861..f0b0fa5 100644 --- a/lib/onesignal/client.rb +++ b/lib/onesignal/client.rb @@ -5,6 +5,8 @@ module OneSignal class Client class ApiError < RuntimeError; end + class ClientError < ApiError; end + class ServerError < ApiError; end def initialize app_id, api_key, api_url @app_id = app_id @@ -46,9 +48,9 @@ def delete_player player_id end def csv_export extra_fields: nil, last_active_since: nil, segment_name: nil - post "players/csv_export?app_id=#{@app_id}", - extra_fields: extra_fields, - last_active_since: last_active_since&.to_i&.to_s, + post "players/csv_export?app_id=#{@app_id}", + extra_fields: extra_fields, + last_active_since: last_active_since&.to_i&.to_s, segment_name: segment_name end @@ -91,8 +93,17 @@ def get url end def handle_errors res - errors = JSON.parse(res.body).fetch 'errors', [] - raise ApiError, (errors.first || "Error code #{res.status}") if res.status > 399 || errors.any? + json = begin + JSON.parse(res.body) + rescue JSON::ParserError, TypeError + {} + end + errors = json.fetch('errors', []) + if res.status > 499 + raise ServerError, errors.first || "Error code #{res.status}" + elsif errors.any? || res.status > 399 + raise ClientError, errors.first || "Error code #{res.status}" + end res end diff --git a/spec/factories/segments.rb b/spec/factories/segments.rb index d461415..566f45e 100644 --- a/spec/factories/segments.rb +++ b/spec/factories/segments.rb @@ -1,7 +1,9 @@ # frozen_string_literal: true FactoryBot.define do - factory :segment do - initialize_with { new(name: Faker::Movies::StarWars.character) } + factory :segment, class: OneSignal::Segment do + name { Faker::Movies::StarWars.character } + + initialize_with { new(attributes) } end end diff --git a/spec/onesignal/client_spec.rb b/spec/onesignal/client_spec.rb index 254ef8f..43f920e 100644 --- a/spec/onesignal/client_spec.rb +++ b/spec/onesignal/client_spec.rb @@ -18,25 +18,43 @@ }.not_to raise_error end + it 'raises an error if the response does not have body' do + res = double :res, body: nil, status: 204 + expect { + expect(subject.send :handle_errors, res) + }.not_to raise_error + end + it 'raises an error if the response code is greater than 399' do res = double :res, body: '{ "errors": ["Internal Server Error"] }', status: 500 expect { subject.send :handle_errors, res - }.to raise_error Client::ApiError, 'Internal Server Error' + }.to raise_error Client::ServerError, 'Internal Server Error' end it 'raises an error if the response code is greater than 399 with default error message' do res = double :res, body: '{}', status: 401 expect { subject.send :handle_errors, res - }.to raise_error Client::ApiError, 'Error code 401' + }.to raise_error Client::ClientError, 'Error code 401' + end + + it 'raises an error if the response is a html' do + body = ''\ + '502 Server Error

Error: Server Error

'\ + '

The server encountered a temporary error and could not complete your request.'\ + '

Please try again in 30 seconds.

' + res = double :res, body: body, status: 502 + expect { + subject.send :handle_errors, res + }.to raise_error Client::ServerError, 'Error code 502' end it 'raises an error if the body contains errors' do res = double :res, body: '{ "errors": ["Internal Server Error"] }', status: 200 expect { subject.send :handle_errors, res - }.to raise_error Client::ApiError, 'Internal Server Error' + }.to raise_error Client::ClientError, 'Internal Server Error' end end