extract and refactor api errors

This commit is contained in:
simon lehericey 2020-12-03 16:52:58 +01:00
parent 46c355beb2
commit b187244a29
18 changed files with 89 additions and 61 deletions

View file

@ -16,7 +16,7 @@ class Champs::SiretController < ApplicationController
begin begin
etablissement = find_etablissement_with_siret etablissement = find_etablissement_with_siret
rescue ApiEntreprise::API::RequestFailed, ApiEntreprise::API::ServiceUnavailable rescue ApiEntreprise::API::Error::RequestFailed, ApiEntreprise::API::Error::ServiceUnavailable
return siret_error(:network_error) return siret_error(:network_error)
end end
if etablissement.nil? if etablissement.nil?

View file

@ -105,7 +105,7 @@ module Users
sanitized_siret = siret_model.siret sanitized_siret = siret_model.siret
begin begin
etablissement = ApiEntrepriseService.create_etablissement(@dossier, sanitized_siret, current_user.id) etablissement = ApiEntrepriseService.create_etablissement(@dossier, sanitized_siret, current_user.id)
rescue ApiEntreprise::API::RequestFailed, ApiEntreprise::API::BadGateway, ApiEntreprise::API::TimedOut rescue ApiEntreprise::API::Error::RequestFailed, ApiEntreprise::API::Error::BadGateway, ApiEntreprise::API::Error::TimedOut
return render_siret_error(t('errors.messages.siret_network_error')) return render_siret_error(t('errors.messages.siret_network_error'))
end end
if etablissement.nil? if etablissement.nil?

View file

@ -1,5 +1,5 @@
class ApiEntreprise::ExercicesJob < ApiEntreprise::Job class ApiEntreprise::ExercicesJob < ApiEntreprise::Job
rescue_from(ApiEntreprise::API::BadFormatRequest) do |exception| rescue_from(ApiEntreprise::API::Error::BadFormatRequest) do |exception|
end end
def perform(etablissement_id, procedure_id) def perform(etablissement_id, procedure_id)

View file

@ -8,24 +8,24 @@ class ApiEntreprise::Job < ApplicationJob
# - bdf: erreur interne # - bdf: erreur interne
# so we retry every day for 5 days # so we retry every day for 5 days
# same logic for ServiceUnavailable # same logic for ServiceUnavailable
retry_on ApiEntreprise::API::ServiceUnavailable, retry_on ApiEntreprise::API::Error::ServiceUnavailable,
ApiEntreprise::API::BadGateway, ApiEntreprise::API::Error::BadGateway,
wait: 1.day wait: 1.day
# We guess the backend is slow but not broken # We guess the backend is slow but not broken
# and the information we are looking for is available # and the information we are looking for is available
# so we retry few seconds later (exponentially to avoid overload) # so we retry few seconds later (exponentially to avoid overload)
retry_on ApiEntreprise::API::TimedOut, wait: :exponentially_longer retry_on ApiEntreprise::API::Error::TimedOut, wait: :exponentially_longer
# If by the time the job runs the Etablissement has been deleted # If by the time the job runs the Etablissement has been deleted
# (it can happen through EtablissementUpdateJob for instance), ignore the job # (it can happen through EtablissementUpdateJob for instance), ignore the job
discard_on ActiveRecord::RecordNotFound discard_on ActiveRecord::RecordNotFound
rescue_from(ApiEntreprise::API::ResourceNotFound) do |exception| rescue_from(ApiEntreprise::API::Error::ResourceNotFound) do |exception|
error(self, exception) error(self, exception)
end end
rescue_from(ApiEntreprise::API::BadFormatRequest) do |exception| rescue_from(ApiEntreprise::API::Error::BadFormatRequest) do |exception|
error(self, exception) error(self, exception)
end end

View file

@ -9,7 +9,7 @@ class ApiEntreprise::Adapter
def data_source def data_source
begin begin
@data_source ||= get_resource @data_source ||= get_resource
rescue ApiEntreprise::API::ResourceNotFound rescue ApiEntreprise::API::Error::ResourceNotFound
@data_source = nil @data_source = nil
end end
end end

View file

@ -12,24 +12,6 @@ class ApiEntreprise::API
TIMEOUT = 20 TIMEOUT = 20
class ResourceNotFound < StandardError
end
class RequestFailed < StandardError
end
class BadFormatRequest < StandardError
end
class BadGateway < StandardError
end
class ServiceUnavailable < StandardError
end
class TimedOut < StandardError
end
def self.entreprise(siren, procedure_id) def self.entreprise(siren, procedure_id)
call_with_siret(ENTREPRISE_RESOURCE_NAME, siren, procedure_id) call_with_siret(ENTREPRISE_RESOURCE_NAME, siren, procedure_id)
end end
@ -84,7 +66,7 @@ class ApiEntreprise::API
if response.success? if response.success?
JSON.parse(response.body, symbolize_names: true) JSON.parse(response.body, symbolize_names: true)
else else
raise RequestFailed, "HTTP Error Code: #{response.code} for #{url}\nheaders: #{response.headers}\nbody: #{response.body}" raise RequestFailed.new(response)
end end
end end
@ -100,23 +82,17 @@ class ApiEntreprise::API
if response.success? if response.success?
JSON.parse(response.body, symbolize_names: true) JSON.parse(response.body, symbolize_names: true)
elsif response.code&.between?(401, 499) elsif response.code&.between?(401, 499)
raise ResourceNotFound, "url: #{url}" raise Error::ResourceNotFound.new(response)
elsif response.code == 400 elsif response.code == 400
raise BadFormatRequest, "url: #{url}" raise Error::BadFormatRequest.new(response)
elsif response.code == 502 elsif response.code == 502
raise BadGateway, "url: #{url}" raise Error::BadGateway.new(response)
elsif response.code == 503 elsif response.code == 503
raise ServiceUnavailable, "url: #{url}" raise Error::ServiceUnavailable.new(response)
elsif response.timed_out? elsif response.timed_out?
raise TimedOut, "url: #{url}" raise Error::TimedOut.new(response)
else else
raise RequestFailed, raise Error::RequestFailed.new(response)
<<~TEXT
HTTP Error Code: #{response.code} for #{url}
headers: #{response.headers}
body: #{response.body}
curl message: #{response.return_message}
TEXT
end end
end end

View file

@ -0,0 +1,18 @@
class ApiEntreprise::API::Error < ::StandardError
def initialize(response)
# use uri to avoid sending token
uri = URI.parse(response.effective_url)
msg = <<~TEXT
url: #{uri.host}#{uri.path}
HTTP error code: #{response.code}
body: #{CGI.escape(response.body)}
curl message: #{response.return_message}
total time: #{response.total_time}
connect time: #{response.connect_time}
response headers: #{response.headers}
TEXT
super(msg)
end
end

View file

@ -0,0 +1,4 @@
class ApiEntreprise::API::Error
class BadFormatRequest < ApiEntreprise::API::Error
end
end

View file

@ -0,0 +1,4 @@
class ApiEntreprise::API::Error
class BadGateway < ApiEntreprise::API::Error
end
end

View file

@ -0,0 +1,4 @@
class ApiEntreprise::API::Error
class RequestFailed < ApiEntreprise::API::Error
end
end

View file

@ -0,0 +1,4 @@
class ApiEntreprise::API::Error
class ResourceNotFound < ApiEntreprise::API::Error
end
end

View file

@ -0,0 +1,4 @@
class ApiEntreprise::API::Error
class ServiceUnavailable < ApiEntreprise::API::Error
end
end

View file

@ -0,0 +1,4 @@
class ApiEntreprise::API::Error
class TimedOut < ApiEntreprise::API::Error
end
end

View file

@ -5,7 +5,7 @@ class ApiEntrepriseService
# #
# Returns nil if the SIRET is unknown # Returns nil if the SIRET is unknown
# #
# Raises a ApiEntreprise::API::RequestFailed exception on transient errors # Raises a ApiEntreprise::API::Error::RequestFailed exception on transient errors
# (timeout, 5XX HTTP error code, etc.) # (timeout, 5XX HTTP error code, etc.)
def self.create_etablissement(dossier_or_champ, siret, user_id = nil) def self.create_etablissement(dossier_or_champ, siret, user_id = nil)
etablissement_params = ApiEntreprise::EtablissementAdapter.new(siret, dossier_or_champ.procedure.id).to_params etablissement_params = ApiEntreprise::EtablissementAdapter.new(siret, dossier_or_champ.procedure.id).to_params

View file

@ -31,13 +31,23 @@ RSpec.describe ApiEntreprise::Job, type: :job do
class ErrorJob < ApiEntreprise::Job class ErrorJob < ApiEntreprise::Job
def perform(error) def perform(error)
response = OpenStruct.new(
effective_url: 'http://host.com/path',
code: '666',
body: 'body',
return_message: 'return_message',
total_time: 10,
connect_time: 20,
headers: 'headers'
)
case error case error
when :service_unavaible when :service_unavaible
raise ApiEntreprise::API::ServiceUnavailable raise ApiEntreprise::API::Error::ServiceUnavailable.new(response)
when :bad_gateway when :bad_gateway
raise ApiEntreprise::API::BadGateway raise ApiEntreprise::API::Error::BadGateway.new(response)
when :timed_out when :timed_out
raise ApiEntreprise::API::TimedOut raise ApiEntreprise::API::Error::TimedOut.new(response)
else else
raise StandardError raise StandardError
end end

View file

@ -17,8 +17,8 @@ describe ApiEntreprise::API do
let(:status) { 502 } let(:status) { 502 }
let(:body) { File.read('spec/fixtures/files/api_entreprise/entreprises_unavailable.json') } let(:body) { File.read('spec/fixtures/files/api_entreprise/entreprises_unavailable.json') }
it 'raises ApiEntreprise::API::RequestFailed' do it 'raises ApiEntreprise::API::Error::RequestFailed' do
expect { subject }.to raise_error(ApiEntreprise::API::BadGateway) expect { subject }.to raise_error(ApiEntreprise::API::Error::BadGateway)
end end
end end
@ -27,8 +27,8 @@ describe ApiEntreprise::API do
let(:status) { 404 } let(:status) { 404 }
let(:body) { File.read('spec/fixtures/files/api_entreprise/entreprises_not_found.json') } let(:body) { File.read('spec/fixtures/files/api_entreprise/entreprises_not_found.json') }
it 'raises ApiEntreprise::API::ResourceNotFound' do it 'raises ApiEntreprise::API::Error::ResourceNotFound' do
expect { subject }.to raise_error(ApiEntreprise::API::ResourceNotFound) expect { subject }.to raise_error(ApiEntreprise::API::Error::ResourceNotFound)
end end
end end
@ -37,8 +37,8 @@ describe ApiEntreprise::API do
let(:status) { 400 } let(:status) { 400 }
let(:body) { File.read('spec/fixtures/files/api_entreprise/entreprises_not_found.json') } let(:body) { File.read('spec/fixtures/files/api_entreprise/entreprises_not_found.json') }
it 'raises ApiEntreprise::API::BadFormatRequest' do it 'raises ApiEntreprise::API::Error::BadFormatRequest' do
expect { subject }.to raise_error(ApiEntreprise::API::BadFormatRequest) expect { subject }.to raise_error(ApiEntreprise::API::Error::BadFormatRequest)
end end
end end
@ -47,8 +47,8 @@ describe ApiEntreprise::API do
let(:status) { 403 } let(:status) { 403 }
let(:body) { File.read('spec/fixtures/files/api_entreprise/entreprises_private.json') } let(:body) { File.read('spec/fixtures/files/api_entreprise/entreprises_private.json') }
it 'raises ApiEntreprise::API::ResourceNotFound' do it 'raises ApiEntreprise::API::Error::ResourceNotFound' do
expect { subject }.to raise_error(ApiEntreprise::API::ResourceNotFound) expect { subject }.to raise_error(ApiEntreprise::API::Error::ResourceNotFound)
end end
end end
@ -97,8 +97,8 @@ describe ApiEntreprise::API do
let(:status) { 404 } let(:status) { 404 }
let(:body) { '' } let(:body) { '' }
it 'raises ApiEntreprise::API::ResourceNotFound' do it 'raises ApiEntreprise::API::Error::ResourceNotFound' do
expect { subject }.to raise_error(ApiEntreprise::API::ResourceNotFound) expect { subject }.to raise_error(ApiEntreprise::API::Error::ResourceNotFound)
end end
end end
@ -127,8 +127,8 @@ describe ApiEntreprise::API do
let(:status) { 404 } let(:status) { 404 }
let(:body) { '' } let(:body) { '' }
it 'raises ApiEntreprise::API::ResourceNotFound' do it 'raises ApiEntreprise::API::Error::ResourceNotFound' do
expect { subject }.to raise_error(ApiEntreprise::API::ResourceNotFound) expect { subject }.to raise_error(ApiEntreprise::API::Error::ResourceNotFound)
end end
end end
@ -159,8 +159,8 @@ describe ApiEntreprise::API do
let(:status) { 404 } let(:status) { 404 }
let(:body) { '' } let(:body) { '' }
it 'raises ApiEntreprise::API::ResourceNotFound' do it 'raises ApiEntreprise::API::Error::ResourceNotFound' do
expect { subject }.to raise_error(ApiEntreprise::API::ResourceNotFound) expect { subject }.to raise_error(ApiEntreprise::API::Error::ResourceNotFound)
end end
end end

View file

@ -84,7 +84,7 @@ describe ApiEntreprise::EntrepriseAdapter do
let(:status) { 500 } let(:status) { 500 }
it 'raises an exception' do it 'raises an exception' do
expect { subject }.to raise_error(ApiEntreprise::API::RequestFailed) expect { subject }.to raise_error(ApiEntreprise::API::Error::RequestFailed)
end end
end end
end end

View file

@ -38,8 +38,8 @@ describe ApiEntrepriseService do
let(:etablissements_status) { 504 } let(:etablissements_status) { 504 }
let(:etablissements_body) { '' } let(:etablissements_body) { '' }
it 'should raise ApiEntreprise::API::RequestFailed' do it 'should raise ApiEntreprise::API::Error::RequestFailed' do
expect { subject }.to raise_error(ApiEntreprise::API::RequestFailed) expect { subject }.to raise_error(ApiEntreprise::API::Error::RequestFailed)
end end
end end