Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump up simple_command #42

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -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
36 changes: 31 additions & 5 deletions lib/onesignal/client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,18 @@
module OneSignal
class Client
class ApiError < RuntimeError; end
class ClientError < ApiError; end
class ApiRateLimitError < ClientError; end
class InvalidExternalUserIdsError < ClientError; end
class InvalidPlayerIdsError < ClientError; end
class TagsLimitError < ClientError; end
class ServerError < ApiError; end

ERROR_MESSAGE_MAPPING = {
'API rate limit exceeded' => ApiRateLimitError,
'invalid_external_user_ids' => InvalidExternalUserIdsError,
'invalid_player_ids' => InvalidPlayerIdsError
}.freeze

def initialize app_id, api_key, api_url
@app_id = app_id
Expand Down Expand Up @@ -46,9 +58,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

Expand Down Expand Up @@ -91,8 +103,22 @@ 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?
error = errors.detect { |key, _v| ERROR_MESSAGE_MAPPING.keys.include?(key) }
raise ERROR_MESSAGE_MAPPING[error[0]].new(error[1]) if error && error.is_a?(Array)
raise ERROR_MESSAGE_MAPPING[error].new(error) if error
raise ClientError, errors.first
elsif res.status > 399
raise ClientError, errors.first || "Error code #{res.status} #{res.body}"
end

res
end
Expand Down
2 changes: 1 addition & 1 deletion onesignal-ruby.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -44,5 +44,5 @@ Gem::Specification.new do |spec|

spec.add_runtime_dependency 'activesupport', '>= 5.0.0', '< 8'
spec.add_runtime_dependency 'faraday', '>= 1', '< 3'
spec.add_runtime_dependency 'simple_command', '~> 0', '>= 0.0.9'
spec.add_runtime_dependency 'simple_command', '>= 0.0.9'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
spec.add_runtime_dependency 'simple_command', '>= 0.0.9'
spec.add_runtime_dependency 'simple_command', '>= 0.0.9', '< 1'

end
6 changes: 4 additions & 2 deletions spec/factories/segments.rb
Original file line number Diff line number Diff line change
@@ -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
71 changes: 67 additions & 4 deletions spec/onesignal/client_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,25 +18,88 @@
}.not_to raise_error
end

it 'raises an error if the response code is greater than 399' do
it 'does not raise 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 InvalidExternalUserIdsError if the response contains errors' do
body = '{
"errors": {
"invalid_external_user_ids": [
"786956"
]
}
}'
res = double :res, body: body, status: 200
expect {
subject.send :handle_errors, res
}.to raise_error Client::InvalidExternalUserIdsError, '["786956"]'
end

it 'raises an InvalidPlayerIdsError if the response contains errors' do
body = '{
"errors": {
"invalid_player_ids" : ["6392d91a-b206-4b7b-a620-cd68e32c3a76"]
}
}'
res = double :res, body: body, status: 200
expect {
subject.send :handle_errors, res
}.to raise_error Client::InvalidPlayerIdsError, '["6392d91a-b206-4b7b-a620-cd68e32c3a76"]'
end

it 'raises an ClientError if the response contains errors' do
body = '{
"errors": [
"Message Notifications must have English language content"
]
}'
res = double :res, body: body, status: 200
expect {
subject.send :handle_errors, res
}.to raise_error Client::ClientError, 'Message Notifications must have English language content'
end

it 'raises an error if the response code is greater than 499' 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 ApiRateLimitError if the response contains errors' do
res = double :res, body: '{ "errors": ["API rate limit exceeded"] }', status: 429
expect {
subject.send :handle_errors, res
}.to raise_error Client::ApiRateLimitError, 'API rate limit exceeded'
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 = '<html><head><meta http-equiv="content-type" content="text/html;charset=utf-8">'\
'<title>502 Server Error</title></head><body text=#000000 bgcolor=#ffffff><h1>Error: Server Error</h1>'\
'<h2>The server encountered a temporary error and could not complete your request.'\
'<p>Please try again in 30 seconds.</h2><h2></h2></body></html>'
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

Expand Down