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

Raise errors on failed API calls #654

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
10 changes: 6 additions & 4 deletions app/controllers/registration_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ def show
rescue Dynamoid::Errors::RecordNotFound
render json: {}, status: :not_found
rescue RegistrationError => e
render_error(e.http_status, e.error)
render_error(e.http_status, e.error, e.data)
end

def count
Expand Down Expand Up @@ -79,7 +79,7 @@ def validate_update_request
@user_id = params[:user_id]
@competition_id = params[:competition_id]

RegistrationChecker.update_registration_allowed!(params, CompetitionApi.find!(@competition_id), @current_user)
RegistrationChecker.update_registration_allowed!(params, CompetitionApi.find(@competition_id), @current_user)
rescue RegistrationError => e
render_error(e.http_status, e.error, e.data)
end
Expand All @@ -102,7 +102,7 @@ def bulk_update

def validate_bulk_update_request
@competition_id = params.require('competition_id')
RegistrationChecker.bulk_update_allowed!(params, CompetitionApi.find!(@competition_id), @current_user)
RegistrationChecker.bulk_update_allowed!(params, CompetitionApi.find(@competition_id), @current_user)
rescue BulkUpdateError => e
render_error(e.http_status, e.errors)
rescue NoMethodError
Expand Down Expand Up @@ -158,7 +158,7 @@ def payment_ticket

def validate_payment_ticket_request
competition_id = params[:competition_id]
@competition = CompetitionApi.find!(competition_id)
@competition = CompetitionApi.find(competition_id)
render_error(:forbidden, ErrorCodes::PAYMENT_NOT_ENABLED) unless @competition.using_wca_payment?

@registration = Registration.find("#{competition_id}-#{@current_user}")
Expand Down Expand Up @@ -223,6 +223,8 @@ def list_admin
Metrics.registration_dynamodb_errors_counter.increment
render json: { error: "Error getting registrations #{e}" },
status: :internal_server_error
rescue RegistrationError => e
render_error(e.http_status, e.error, e.data)
end

def import
Expand Down
44 changes: 12 additions & 32 deletions lib/competition_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,49 +8,29 @@ class CompetitionApi < WcaApi
def self.find(competition_id)
competition_json = fetch_competition(competition_id)
CompetitionInfo.new(competition_json)
rescue RegistrationError
nil
end

def self.url(competition_id)
"#{EnvConfig.WCA_HOST}/api/v0/competitions/#{competition_id}"
end

def self.find!(competition_id)
competition_json = fetch_competition(competition_id)
CompetitionInfo.new(competition_json)
end

def self.fetch_qualifications(competition_id)
Rails.cache.fetch("#{competition_id}/qualifications", expires_in: 5.minutes) do
response = HTTParty.get("#{url(competition_id)}/qualifications")
case response.code
when 200
@status = 200
response.parsed_response
when 404
Metrics.registration_competition_api_error_counter.increment
raise RegistrationError.new(404, ErrorCodes::COMPETITION_NOT_FOUND)
else
Metrics.registration_competition_api_error_counter.increment
raise RegistrationError.new(response.code.to_i, ErrorCodes::COMPETITION_API_5XX)
end
self.get_request("#{url(competition_id)}/qualifications")
rescue RegistrationError => e
if e.data[:http_code] == 404
raise RegistrationError.new(404, ErrorCodes::COMPETITION_NOT_FOUND)
else
raise e
end
end

private_class_method def self.fetch_competition(competition_id)
Rails.cache.fetch(competition_id, expires_in: 5.minutes) do
response = HTTParty.get(CompetitionApi.url(competition_id))
case response.code
when 200
response.parsed_response
when 404
Metrics.registration_competition_api_error_counter.increment
raise RegistrationError.new(404, ErrorCodes::COMPETITION_NOT_FOUND)
else
Metrics.registration_competition_api_error_counter.increment
raise RegistrationError.new(response.code.to_i, ErrorCodes::COMPETITION_API_5XX)
end
self.get_request(url(competition_id))
rescue RegistrationError => e
if e.data[:http_code] == 404
raise RegistrationError.new(404, ErrorCodes::COMPETITION_NOT_FOUND)
else
raise e
end
end
end
32 changes: 18 additions & 14 deletions lib/email_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,26 @@ def self.registration_email_path
end

def self.send_update_email(competition_id, user_id, status, current_user)
HTTParty.post(EmailApi.registration_email_path, headers: { WCA_API_HEADER => self.wca_token }, body: {
competition_id: competition_id,
user_id: user_id,
registration_status: status,
registration_action: 'update',
current_user: current_user,
})
self.post_request(
EmailApi.registration_email_path,
{ competition_id: competition_id,
user_id: user_id,
registration_status: status,
registration_action: 'update',
current_user: current_user }.to_json,
)
end

def self.send_creation_email(competition_id, user_id)
HTTParty.post(EmailApi.registration_email_path, headers: { WCA_API_HEADER => self.wca_token }, body: {
competition_id: competition_id,
user_id: user_id,
registration_status: 'pending',
registration_action: 'create',
current_user: user_id,
})
self.post_request(
EmailApi.registration_email_path,
{
competition_id: competition_id,
user_id: user_id,
registration_status: 'pending',
registration_action: 'create',
current_user: user_id,
}.to_json,
)
end
end
3 changes: 2 additions & 1 deletion lib/error_codes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@ module ErrorCodes
EXPIRED_TOKEN = -2
MISSING_AUTHENTICATION = -3

# Competition Errors
# API Errors
COMPETITION_NOT_FOUND = -1000
COMPETITION_API_5XX = -1001
MONOLITH_API_ERROR = -1002

# User Errors
USER_CANNOT_COMPETE = -2001
Expand Down
11 changes: 4 additions & 7 deletions lib/payment_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,10 @@

class PaymentApi < WcaApi
def self.get_ticket(attendee_id, amount, currency_code, current_user)
response = HTTParty.post(payment_init_path,
body: { 'attendee_id' => attendee_id, 'amount' => amount, 'currency_code' => currency_code, 'current_user' => current_user }.to_json,
headers: { WCA_API_HEADER => self.wca_token,
'Content-Type' => 'application/json' })
unless response.ok?
raise 'Error from the payments service'
end
response = self.post_request(
payment_init_path,
{ 'attendee_id' => attendee_id, 'amount' => amount, 'currency_code' => currency_code, 'current_user' => current_user }.to_json,
)
[response['client_secret'], response['id']]
end

Expand Down
6 changes: 3 additions & 3 deletions lib/user_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,15 @@ def self.competitor_qualifications_path(user_id, date = nil)
end

def self.get_permissions(user_id)
HTTParty.get(permissions_path(user_id), headers: { WCA_API_HEADER => self.wca_token })
self.get_request(permissions_path(user_id))
end

def self.get_user_info_pii(user_ids)
HTTParty.post(UserApi.competitor_info_path, headers: { WCA_API_HEADER => self.wca_token }, body: { ids: user_ids.to_a })
self.post_request(UserApi.competitor_info_path, { ids: user_ids.to_a })
end

def self.qualifications(user_id, date = nil)
HTTParty.get(UserApi.competitor_qualifications_path(user_id, date), headers: { WCA_API_HEADER => self.wca_token })
self.get_request(UserApi.competitor_qualifications_path(user_id, date))
end

def self.can_compete?(user_id, competition_start_date)
Expand Down
29 changes: 29 additions & 0 deletions lib/wca_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,33 @@ def self.wca_token
end
end
end

def self.get_request(url)
Rails.cache.fetch(url, expires_in: 5.minutes) do
response = HTTParty.get(url, headers: { WCA_API_HEADER => self.wca_token })

if response.success?
response
else
raise RegistrationError.new(
500,
ErrorCodes::MONOLITH_API_ERROR,
{ url: "GET: #{url}", http_code: response.code, body: response.parsed_response },
)
end
end
end

def self.post_request(url, body)
response = HTTParty.post(url, headers: { WCA_API_HEADER => self.wca_token, 'Content-Type' => 'application/json' }, body: body)
if response.success?
response
else
raise RegistrationError.new(
500,
ErrorCodes::MONOLITH_API_ERROR,
{ url: "POST: #{url}", http_code: response.code, body: response.parsed_response },
)
end
end
end
70 changes: 70 additions & 0 deletions spec/controllers/registration_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -235,4 +235,74 @@
expect(response.code).to eq('400')
end
end

describe '#API errors' do
describe 'raise Registration error when monolith API fails:' do
it 'UserApi::get_permissions' do
@registration_request = FactoryBot.build(:registration_request)
stub_request(:get, UserApi.permissions_path(@registration_request['user_id'])).to_return(
status: 502,
headers: { 'Content-Type' => 'application/json' },
)

@competition = FactoryBot.build(:competition)
stub_request(:get, CompetitionApi.url(@competition['id'])).to_return(
status: 200,
body: @competition.except('qualifications').to_json,
headers: { 'Content-Type' => 'application/json' },
)

stub_request(:post, EmailApi.registration_email_path).to_return(status: 200, body: { emails_sent: 1 }.to_json)

request.headers['Authorization'] = @registration_request['jwt_token']
post :create, params: @registration_request, as: :json

expect(response.code).to eq('500')
end

it 'UserApi::qualifications' do
@registration_request = FactoryBot.build(:registration_request)
stub_request(:get, UserApi.permissions_path(@registration_request['user_id'])).to_return(
status: 200,
body: FactoryBot.build(:permissions).to_json,
headers: { 'Content-Type' => 'application/json' },
)

@competition = FactoryBot.build(:competition, :has_qualifications)
stub_json(CompetitionApi.url(@competition['id']), 200, @competition.except('qualifications'))
stub_json(CompetitionApi.url("#{@competition['id']}/qualifications"), 200, @competition['qualifications'])

stub_request(:get, %r{#{Regexp.escape(EnvConfig.WCA_HOST)}/api/v0/results/\d+/qualification_data(\?date=\d{4}-\d{2}-\d{2})?})
.to_return(status: 502)

stub_request(:post, EmailApi.registration_email_path).to_return(status: 200, body: { emails_sent: 1 }.to_json)

request.headers['Authorization'] = @registration_request['jwt_token']
post :create, params: @registration_request, as: :json

expect(response.code).to eq('500')
end

it 'UserApi::get_user_info_pii' do
@competition = FactoryBot.build(:competition)
stub_json(CompetitionApi.url(@competition['id']), 200, @competition.except('qualifications'))

@requesting_user_id = 1400
stub_request(:get, UserApi.permissions_path(@requesting_user_id)).to_return(
status: 200,
body: FactoryBot.build(:permissions_response, organized_competitions: [@competition['id']]).to_json,
headers: { 'Content-Type' => 'application/json' },
)

FactoryBot.create(:registration, registration_status: 'pending')

stub_request(:post, UserApi.competitor_info_path).to_return(status: 502)

request.headers['Authorization'] = fetch_jwt_token(@requesting_user_id)
get :list_admin, params: { competition_id: @competition['id'] }

expect(response.code).to eq('500')
end
end
end
end
27 changes: 27 additions & 0 deletions spec/lib/competition_api_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# frozen_string_literal: true

require 'rails_helper'

describe CompetitionApi do
it 'raises RegistrationError when 404 is returned' do
stub_request(:get, CompetitionApi.url('test')).to_return(status: 404)

expect {
CompetitionApi.find('test')
}.to raise_error(RegistrationError) do |error|
expect(error.http_status).to eq(404)
expect(error.error).to eq(ErrorCodes::COMPETITION_NOT_FOUND)
end
end

it 'raises RegistrationError when 404 is returned' do
stub_request(:get, CompetitionApi.url('test')).to_return(status: 500)

expect {
CompetitionApi.find('test')
}.to raise_error(RegistrationError) do |error|
expect(error.http_status).to eq(500)
expect(error.error).to eq(ErrorCodes::MONOLITH_API_ERROR)
end
end
end
8 changes: 8 additions & 0 deletions spec/support/stub_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,11 @@ def stub_qualifications(payload = nil, qualification_data_date = nil)
end
end
end

def stub_qualification_error
url_regex = %r{#{Regexp.escape(EnvConfig.WCA_HOST)}/api/v0/results/\d+/qualification_data(\?date=\d{4}-\d{2}-\d{2})?}

stub_request(:get, url_regex).to_return do |request|
{ status: 502 }
end
end
Loading