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.

Sign up for GitHub

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

Handle 502 Server Error from OneSignal Servers #45

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

Handle 502 Server Error from OneSignal Servers #45

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8e4424c
Handle 502 Server Error from OneSignal Servers
yltsrc Mar 2, 2023
3d5c3b8
Use faker from rubygems
yltsrc Mar 2, 2023
b266c1b
Introduce ServerError and ClientError
yltsrc Mar 20, 2023
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
View file
Open in desktop
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
21 changes: 16 additions & 5 deletions lib/onesignal/client.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions spec/factories/segments.rb
View file
Open in desktop
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
24 changes: 21 additions & 3 deletions spec/onesignal/client_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = '<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