From 532cac8a3dadead34559791a16716d21ecc24811 Mon Sep 17 00:00:00 2001 From: Duncan Date: Sun, 25 Aug 2024 11:26:31 +0200 Subject: [PATCH 01/11] better errors on api failures --- app/controllers/registration_controller.rb | 4 +- lib/error_codes.rb | 3 +- lib/user_api.rb | 6 +- lib/wca_api.rb | 20 ++++++ .../registration_controller_spec.rb | 71 +++++++++++++++++++ spec/support/stub_helper.rb | 13 ++++ 6 files changed, 112 insertions(+), 5 deletions(-) diff --git a/app/controllers/registration_controller.rb b/app/controllers/registration_controller.rb index 1589129d..5cec31d5 100644 --- a/app/controllers/registration_controller.rb +++ b/app/controllers/registration_controller.rb @@ -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 @@ -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 diff --git a/lib/error_codes.rb b/lib/error_codes.rb index 4ac1ccb6..bf4025a9 100644 --- a/lib/error_codes.rb +++ b/lib/error_codes.rb @@ -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 diff --git a/lib/user_api.rb b/lib/user_api.rb index 9fa75813..281d9e59 100644 --- a/lib/user_api.rb +++ b/lib/user_api.rb @@ -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) diff --git a/lib/wca_api.rb b/lib/wca_api.rb index 8c0a8763..e638dc76 100644 --- a/lib/wca_api.rb +++ b/lib/wca_api.rb @@ -21,4 +21,24 @@ def self.wca_token end end end + + def self.get_request(url) + response = HTTParty.get(url, headers: { WCA_API_HEADER => self.wca_token }) + if response.code == 200 + response + else + Metrics.registration_competition_api_error_counter.increment + raise RegistrationError.new(:service_unavailable, ErrorCodes::MONOLITH_API_ERROR, { http_code: response.code, body: response.parsed_response }) + end + end + + def self.post_request(url, body) + response = HTTParty.post(url, headers: { WCA_API_HEADER => self.wca_token }, body: body) + if response.code == 200 + response + else + Metrics.registration_competition_api_error_counter.increment + raise RegistrationError.new(:service_unavailable, ErrorCodes::MONOLITH_API_ERROR, { http_code: response.code, body: response.parsed_response }) + end + end end diff --git a/spec/controllers/registration_controller_spec.rb b/spec/controllers/registration_controller_spec.rb index 64056e39..da313dad 100644 --- a/spec/controllers/registration_controller_spec.rb +++ b/spec/controllers/registration_controller_spec.rb @@ -235,4 +235,75 @@ 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('503') + 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('503') + 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' }, + ) + + registration = 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('503') + end + end + + end end diff --git a/spec/support/stub_helper.rb b/spec/support/stub_helper.rb index c6f46bce..1ed3a559 100644 --- a/spec/support/stub_helper.rb +++ b/spec/support/stub_helper.rb @@ -25,3 +25,16 @@ 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| + uri = URI(request.uri) + params = URI.decode_www_form(uri.query || '').to_h + date = params['date'] + payload_date = qualification_data_date || date + + { status: 502 } + end +end From 2ff7ac8c417f2d5fb0d81ed36c60512b5eee5678 Mon Sep 17 00:00:00 2001 From: Duncan Date: Sun, 25 Aug 2024 11:27:21 +0200 Subject: [PATCH 02/11] raise error on comp api failure --- app/controllers/registration_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/registration_controller.rb b/app/controllers/registration_controller.rb index 5cec31d5..60aacf2a 100644 --- a/app/controllers/registration_controller.rb +++ b/app/controllers/registration_controller.rb @@ -61,7 +61,7 @@ def create def validate_create_request @competition_id = registration_params[:competition_id] @user_id = registration_params[:user_id] - RegistrationChecker.create_registration_allowed!(registration_params, CompetitionApi.find(@competition_id), @current_user) + RegistrationChecker.create_registration_allowed!(registration_params, CompetitionApi.find!(@competition_id), @current_user) rescue RegistrationError => e render_error(e.http_status, e.error, e.data) end From 7fc30296dbe258add29f63d19dc94fd0cc804da7 Mon Sep 17 00:00:00 2001 From: Duncan Date: Mon, 26 Aug 2024 06:00:15 +0200 Subject: [PATCH 03/11] rubocop --- spec/controllers/registration_controller_spec.rb | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/spec/controllers/registration_controller_spec.rb b/spec/controllers/registration_controller_spec.rb index da313dad..fc8939ff 100644 --- a/spec/controllers/registration_controller_spec.rb +++ b/spec/controllers/registration_controller_spec.rb @@ -272,8 +272,8 @@ 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(: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) @@ -282,7 +282,7 @@ expect(response.code).to eq('503') end - + it 'UserApi::get_user_info_pii' do @competition = FactoryBot.build(:competition) stub_json(CompetitionApi.url(@competition['id']), 200, @competition.except('qualifications')) @@ -304,6 +304,5 @@ expect(response.code).to eq('503') end end - end end From a8e4bd2b826093d3c14b58bb4caa27eb6c821968 Mon Sep 17 00:00:00 2001 From: Duncan Date: Mon, 26 Aug 2024 10:05:56 +0200 Subject: [PATCH 04/11] use httparty succes & rubocop --- lib/wca_api.rb | 4 ++-- spec/controllers/registration_controller_spec.rb | 2 +- spec/support/stub_helper.rb | 5 ----- 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/lib/wca_api.rb b/lib/wca_api.rb index e638dc76..b8e7448c 100644 --- a/lib/wca_api.rb +++ b/lib/wca_api.rb @@ -24,7 +24,7 @@ def self.wca_token def self.get_request(url) response = HTTParty.get(url, headers: { WCA_API_HEADER => self.wca_token }) - if response.code == 200 + if response.success? response else Metrics.registration_competition_api_error_counter.increment @@ -34,7 +34,7 @@ def self.get_request(url) def self.post_request(url, body) response = HTTParty.post(url, headers: { WCA_API_HEADER => self.wca_token }, body: body) - if response.code == 200 + if response.success? response else Metrics.registration_competition_api_error_counter.increment diff --git a/spec/controllers/registration_controller_spec.rb b/spec/controllers/registration_controller_spec.rb index fc8939ff..c2573bba 100644 --- a/spec/controllers/registration_controller_spec.rb +++ b/spec/controllers/registration_controller_spec.rb @@ -294,7 +294,7 @@ headers: { 'Content-Type' => 'application/json' }, ) - registration = FactoryBot.create(:registration, registration_status: 'pending') + FactoryBot.create(:registration, registration_status: 'pending') stub_request(:post, UserApi.competitor_info_path).to_return(status: 502) diff --git a/spec/support/stub_helper.rb b/spec/support/stub_helper.rb index 1ed3a559..45cd7906 100644 --- a/spec/support/stub_helper.rb +++ b/spec/support/stub_helper.rb @@ -30,11 +30,6 @@ 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| - uri = URI(request.uri) - params = URI.decode_www_form(uri.query || '').to_h - date = params['date'] - payload_date = qualification_data_date || date - { status: 502 } end end From c7e7cf1dc03d1cdbbb7b20ff2e0a2964108040d1 Mon Sep 17 00:00:00 2001 From: Duncan Date: Wed, 28 Aug 2024 08:02:00 +0200 Subject: [PATCH 05/11] updated to use caching and all api classes use wca_api methods --- app/controllers/registration_controller.rb | 8 ++-- lib/competition_api.rb | 44 ++++++---------------- lib/email_api.rb | 33 +++++++++------- lib/payment_api.rb | 11 ++---- lib/wca_api.rb | 16 ++++---- 5 files changed, 48 insertions(+), 64 deletions(-) diff --git a/app/controllers/registration_controller.rb b/app/controllers/registration_controller.rb index 60aacf2a..1bd40bf0 100644 --- a/app/controllers/registration_controller.rb +++ b/app/controllers/registration_controller.rb @@ -61,7 +61,7 @@ def create def validate_create_request @competition_id = registration_params[:competition_id] @user_id = registration_params[:user_id] - RegistrationChecker.create_registration_allowed!(registration_params, CompetitionApi.find!(@competition_id), @current_user) + RegistrationChecker.create_registration_allowed!(registration_params, CompetitionApi.find(@competition_id), @current_user) rescue RegistrationError => e render_error(e.http_status, e.error, e.data) end @@ -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 @@ -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 @@ -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}") diff --git a/lib/competition_api.rb b/lib/competition_api.rb index 3bf9d9f3..82201d37 100644 --- a/lib/competition_api.rb +++ b/lib/competition_api.rb @@ -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 diff --git a/lib/email_api.rb b/lib/email_api.rb index 50fa8090..bda933a4 100644 --- a/lib/email_api.rb +++ b/lib/email_api.rb @@ -9,22 +9,27 @@ 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 diff --git a/lib/payment_api.rb b/lib/payment_api.rb index ab6d416d..f40ce890 100644 --- a/lib/payment_api.rb +++ b/lib/payment_api.rb @@ -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 diff --git a/lib/wca_api.rb b/lib/wca_api.rb index b8e7448c..71f48ecc 100644 --- a/lib/wca_api.rb +++ b/lib/wca_api.rb @@ -23,17 +23,19 @@ def self.wca_token end def self.get_request(url) - response = HTTParty.get(url, headers: { WCA_API_HEADER => self.wca_token }) - if response.success? - response - else - Metrics.registration_competition_api_error_counter.increment - raise RegistrationError.new(:service_unavailable, ErrorCodes::MONOLITH_API_ERROR, { http_code: response.code, body: response.parsed_response }) + 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(:service_unavailable, ErrorCodes::MONOLITH_API_ERROR, { 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 }, body: body) + response = HTTParty.post(url, headers: { WCA_API_HEADER => self.wca_token, 'Content-Type' => 'application/json' }, body: body) if response.success? response else From 4cb7785597b445f7c3497058f40c8b82b47718c7 Mon Sep 17 00:00:00 2001 From: Duncan Date: Wed, 28 Aug 2024 08:04:32 +0200 Subject: [PATCH 06/11] rubocop --- lib/competition_api.rb | 8 ++++---- lib/email_api.rb | 3 +-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/competition_api.rb b/lib/competition_api.rb index 82201d37..9d8dc848 100644 --- a/lib/competition_api.rb +++ b/lib/competition_api.rb @@ -17,9 +17,9 @@ def self.url(competition_id) def self.fetch_qualifications(competition_id) self.get_request("#{url(competition_id)}/qualifications") rescue RegistrationError => e - if e.data[:http_code] = 404 + if (e.data[:http_code] = 404) raise RegistrationError.new(404, ErrorCodes::COMPETITION_NOT_FOUND) - else + else raise e end end @@ -27,9 +27,9 @@ def self.fetch_qualifications(competition_id) private_class_method def self.fetch_competition(competition_id) self.get_request(url(competition_id)) rescue RegistrationError => e - if e.data[:http_code] = 404 + if (e.data[:http_code] = 404) raise RegistrationError.new(404, ErrorCodes::COMPETITION_NOT_FOUND) - else + else raise e end end diff --git a/lib/email_api.rb b/lib/email_api.rb index bda933a4..d4b45192 100644 --- a/lib/email_api.rb +++ b/lib/email_api.rb @@ -15,8 +15,7 @@ def self.send_update_email(competition_id, user_id, status, current_user) user_id: user_id, registration_status: status, registration_action: 'update', - current_user: current_user, - }.to_json, + current_user: current_user }.to_json, ) end From f738790ede783b09b6b79dff0f9df653037d6c58 Mon Sep 17 00:00:00 2001 From: Duncan Date: Wed, 28 Aug 2024 10:24:31 +0200 Subject: [PATCH 07/11] added proper 404 check and URL that raised the error --- lib/competition_api.rb | 4 ++-- lib/wca_api.rb | 13 ++++++++++--- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/lib/competition_api.rb b/lib/competition_api.rb index 9d8dc848..e759a020 100644 --- a/lib/competition_api.rb +++ b/lib/competition_api.rb @@ -17,7 +17,7 @@ def self.url(competition_id) def self.fetch_qualifications(competition_id) self.get_request("#{url(competition_id)}/qualifications") rescue RegistrationError => e - if (e.data[:http_code] = 404) + if (e.data[:http_code] == 404) raise RegistrationError.new(404, ErrorCodes::COMPETITION_NOT_FOUND) else raise e @@ -27,7 +27,7 @@ def self.fetch_qualifications(competition_id) private_class_method def self.fetch_competition(competition_id) self.get_request(url(competition_id)) rescue RegistrationError => e - if (e.data[:http_code] = 404) + if (e.data[:http_code] == 404) raise RegistrationError.new(404, ErrorCodes::COMPETITION_NOT_FOUND) else raise e diff --git a/lib/wca_api.rb b/lib/wca_api.rb index 71f48ecc..5fe1f35b 100644 --- a/lib/wca_api.rb +++ b/lib/wca_api.rb @@ -29,7 +29,11 @@ def self.get_request(url) if response.success? response else - raise RegistrationError.new(:service_unavailable, ErrorCodes::MONOLITH_API_ERROR, { http_code: response.code, body: response.parsed_response }) + raise RegistrationError.new( + :service_unavailable, + ErrorCodes::MONOLITH_API_ERROR, + { url: "GET: #{url}", http_code: response.code, body: response.parsed_response }, + ) end end end @@ -39,8 +43,11 @@ def self.post_request(url, body) if response.success? response else - Metrics.registration_competition_api_error_counter.increment - raise RegistrationError.new(:service_unavailable, ErrorCodes::MONOLITH_API_ERROR, { http_code: response.code, body: response.parsed_response }) + raise RegistrationError.new( + :service_unavailable, + ErrorCodes::MONOLITH_API_ERROR, + { url: "POST: #{url}, http_code: response.code, body: response.parsed_response }, + ) end end end From 9754385e2ea082c1558afd6b9443e3c4e0540dd4 Mon Sep 17 00:00:00 2001 From: Duncan Date: Fri, 30 Aug 2024 07:14:05 +0200 Subject: [PATCH 08/11] fixed 404 error and change to internal server error not 503 --- lib/wca_api.rb | 6 +++--- spec/lib/competition_api_spec.rb | 28 ++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) create mode 100644 spec/lib/competition_api_spec.rb diff --git a/lib/wca_api.rb b/lib/wca_api.rb index 5fe1f35b..b6353da1 100644 --- a/lib/wca_api.rb +++ b/lib/wca_api.rb @@ -30,7 +30,7 @@ def self.get_request(url) response else raise RegistrationError.new( - :service_unavailable, + 500, ErrorCodes::MONOLITH_API_ERROR, { url: "GET: #{url}", http_code: response.code, body: response.parsed_response }, ) @@ -44,9 +44,9 @@ def self.post_request(url, body) response else raise RegistrationError.new( - :service_unavailable, + 500, ErrorCodes::MONOLITH_API_ERROR, - { url: "POST: #{url}, http_code: response.code, body: response.parsed_response }, + { url: "POST: #{url}", http_code: response.code, body: response.parsed_response }, ) end end diff --git a/spec/lib/competition_api_spec.rb b/spec/lib/competition_api_spec.rb new file mode 100644 index 00000000..8569ba59 --- /dev/null +++ b/spec/lib/competition_api_spec.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe CompetitionApi, :tag 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 From 48205d85ccd1795a4e96a222728ab20cf64f81aa Mon Sep 17 00:00:00 2001 From: Duncan Date: Fri, 30 Aug 2024 08:47:34 +0200 Subject: [PATCH 09/11] rubocop --- lib/competition_api.rb | 4 ++-- spec/lib/competition_api_spec.rb | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/competition_api.rb b/lib/competition_api.rb index e759a020..ea87ad04 100644 --- a/lib/competition_api.rb +++ b/lib/competition_api.rb @@ -17,7 +17,7 @@ def self.url(competition_id) def self.fetch_qualifications(competition_id) self.get_request("#{url(competition_id)}/qualifications") rescue RegistrationError => e - if (e.data[:http_code] == 404) + if e.data[:http_code] == 404 raise RegistrationError.new(404, ErrorCodes::COMPETITION_NOT_FOUND) else raise e @@ -27,7 +27,7 @@ def self.fetch_qualifications(competition_id) private_class_method def self.fetch_competition(competition_id) self.get_request(url(competition_id)) rescue RegistrationError => e - if (e.data[:http_code] == 404) + if e.data[:http_code] == 404 raise RegistrationError.new(404, ErrorCodes::COMPETITION_NOT_FOUND) else raise e diff --git a/spec/lib/competition_api_spec.rb b/spec/lib/competition_api_spec.rb index 8569ba59..aba0073a 100644 --- a/spec/lib/competition_api_spec.rb +++ b/spec/lib/competition_api_spec.rb @@ -24,5 +24,4 @@ expect(error.error).to eq(ErrorCodes::MONOLITH_API_ERROR) end end - end From f6b28b573c7a5e4e765d46b245e68a6dc5d9219b Mon Sep 17 00:00:00 2001 From: Duncan Date: Fri, 30 Aug 2024 15:17:32 +0200 Subject: [PATCH 10/11] removed test tag --- spec/lib/competition_api_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/lib/competition_api_spec.rb b/spec/lib/competition_api_spec.rb index aba0073a..ef6a6802 100644 --- a/spec/lib/competition_api_spec.rb +++ b/spec/lib/competition_api_spec.rb @@ -2,7 +2,7 @@ require 'rails_helper' -describe CompetitionApi, :tag do +describe CompetitionApi do it 'raises RegistrationError when 404 is returned' do stub_request(:get, CompetitionApi.url('test')).to_return(status: 404) From 0148d75f28991eeb83e060b36f6dd1b005c50824 Mon Sep 17 00:00:00 2001 From: Duncan Date: Fri, 30 Aug 2024 16:10:53 +0200 Subject: [PATCH 11/11] expect 500 not 503 errors --- spec/controllers/registration_controller_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/controllers/registration_controller_spec.rb b/spec/controllers/registration_controller_spec.rb index c2573bba..8926c014 100644 --- a/spec/controllers/registration_controller_spec.rb +++ b/spec/controllers/registration_controller_spec.rb @@ -257,7 +257,7 @@ request.headers['Authorization'] = @registration_request['jwt_token'] post :create, params: @registration_request, as: :json - expect(response.code).to eq('503') + expect(response.code).to eq('500') end it 'UserApi::qualifications' do @@ -280,7 +280,7 @@ request.headers['Authorization'] = @registration_request['jwt_token'] post :create, params: @registration_request, as: :json - expect(response.code).to eq('503') + expect(response.code).to eq('500') end it 'UserApi::get_user_info_pii' do @@ -301,7 +301,7 @@ request.headers['Authorization'] = fetch_jwt_token(@requesting_user_id) get :list_admin, params: { competition_id: @competition['id'] } - expect(response.code).to eq('503') + expect(response.code).to eq('500') end end end