diff --git a/modules/claims_api/app/controllers/claims_api/v1/claims_controller.rb b/modules/claims_api/app/controllers/claims_api/v1/claims_controller.rb index 51680a6d099..94a35c9c06e 100644 --- a/modules/claims_api/app/controllers/claims_api/v1/claims_controller.rb +++ b/modules/claims_api/app/controllers/claims_api/v1/claims_controller.rb @@ -63,9 +63,11 @@ def fetch_errored(claim) end def format_evss_errors(errors) - errors.map do |error| - formatted = error['key'] ? error['key'].gsub('.', '/') : error['key'] - { status: 422, detail: "#{error['severity']} #{error['detail'] || error['text']}".squish, source: formatted } + errors.map do |err| + error = err.deep_symbolize_keys + # Some old saved error messages saved key is an integer, so need to call .to_s before .gsub + formatted = error[:key] ? error[:key].to_s.gsub('.', '/') : error[:key] + { status: 422, detail: "#{error[:severity]} #{error[:detail] || error[:text]}".squish, source: formatted } end end end diff --git a/modules/claims_api/app/controllers/claims_api/v1/forms/disability_compensation_controller.rb b/modules/claims_api/app/controllers/claims_api/v1/forms/disability_compensation_controller.rb index f146e3087ec..3266c7a6b83 100644 --- a/modules/claims_api/app/controllers/claims_api/v1/forms/disability_compensation_controller.rb +++ b/modules/claims_api/app/controllers/claims_api/v1/forms/disability_compensation_controller.rb @@ -177,8 +177,7 @@ def validate_form_526 track_526_validation_errors(error_details) raise ::Common::Exceptions::UnprocessableEntity.new(errors: format_526_errors(error_details)) rescue ::Common::Exceptions::BackendServiceException => e - error_details = e&.original_body&.[](:messages) - raise ::Common::Exceptions::UnprocessableEntity.new(errors: format_526_errors(error_details)) + raise ::Common::Exceptions::UnprocessableEntity.new(errors: format_526_errors(e.original_body)) rescue ::Common::Exceptions::GatewayTimeout, ::Timeout::Error, ::Faraday::TimeoutError, @@ -252,7 +251,9 @@ def valid_526_response def format_526_errors(errors) errors.map do |error| - { status: 422, detail: "#{error['key']} #{error['detail']}", source: error['key'] } + e = error.deep_symbolize_keys + details = e[:text].presence || e[:detail] + { status: 422, detail: "#{e[:key]}, #{details}", source: e[:key] } end end diff --git a/modules/claims_api/app/sidekiq/claims_api/claim_establisher.rb b/modules/claims_api/app/sidekiq/claims_api/claim_establisher.rb index fcecf966879..fdfbb41763d 100644 --- a/modules/claims_api/app/sidekiq/claims_api/claim_establisher.rb +++ b/modules/claims_api/app/sidekiq/claims_api/claim_establisher.rb @@ -42,14 +42,14 @@ def perform(auto_claim_id) # rubocop:disable Metrics/MethodLength queue_flash_updater(auto_claim.flashes, auto_claim_id) queue_special_issues_updater(auto_claim.special_issues, auto_claim) - rescue ::Common::Exceptions::ServiceError => e + rescue ::Common::Exceptions::BackendServiceException => e auto_claim.status = ClaimsApi::AutoEstablishedClaim::ERRORED - auto_claim.evss_response = e.errors || e.detailed_message + auto_claim.evss_response = get_error_message(e) # e.original_body auto_claim.form_data = orig_form_data auto_claim.save rescue => e auto_claim.status = ClaimsApi::AutoEstablishedClaim::ERRORED - auto_claim.evss_response = (e.errors.presence || e.detailed_message) + auto_claim.evss_response = get_error_message(e) # (e.errors.presence || e.detailed_message) auto_claim.form_data = orig_form_data auto_claim.save raise e diff --git a/modules/claims_api/app/sidekiq/claims_api/service_base.rb b/modules/claims_api/app/sidekiq/claims_api/service_base.rb index 18effe692f6..fe6087095b6 100644 --- a/modules/claims_api/app/sidekiq/claims_api/service_base.rb +++ b/modules/claims_api/app/sidekiq/claims_api/service_base.rb @@ -53,6 +53,10 @@ def get_error_message(error) error.original_body elsif error.respond_to? :message error.message + elsif error.respond_to? :errors + error.errors + elsif error.respond_to? :detailed_message + error.detailed_message else error end diff --git a/modules/claims_api/lib/custom_error.rb b/modules/claims_api/lib/custom_error.rb index cefb0227595..3a489ba41fa 100644 --- a/modules/claims_api/lib/custom_error.rb +++ b/modules/claims_api/lib/custom_error.rb @@ -2,90 +2,57 @@ module ClaimsApi class CustomError - def initialize(error, claim, method) + def initialize(error) @error = error - @claim = claim - @method = method end - def build_error # rubocop:disable Metrics/MethodLength - get_status - get_code - get_source - if @error.is_a?(Faraday::ConnectionFailed) || @error.is_a?(Faraday::ParsingError) || - @error.is_a?(Faraday::NilStatusError) || @error.is_a?(Faraday::TimeoutError) || - @error.is_a?(::Common::Exceptions::BackendServiceException) || - @error.is_a?(::Common::Exceptions::ExternalServerInternalServerError) || - @error.is_a?(::Common::Exceptions::BadGateway) || @error.is_a?(Faraday::SSLError) || - @error.is_a?(Faraday::ServerError) - errors = { errors: [{ 'title' => 'Service Exception', - 'key' => @source, - 'detail' => 'A re-tryable error has occurred, original_error: ' \ - "#{@error}.", status: @status, code: @code }] } - log_outcome_for_claims_api(errors) - raise ::Common::Exceptions::ServiceError, errors + def build_error + case @error + when Faraday::ParsingError + raise_backend_exception(@error.class, @error, 'EVSS502') + when ::Common::Exceptions::BackendServiceException + raise ::Common::Exceptions::Forbidden if @error&.original_status == 403 - elsif @error.is_a?(StandardError) || @error.is_a?(Faraday::BadRequestError) || - @error.is_a?(Faraday::ConflictError) || @error.is_a?(Faraday::ForbiddenError) || - @error.is_a?(Faraday::ProxyAuthError) || @error.is_a?(Faraday::ResourceNotFound) || - @error.is_a?(Faraday::UnauthorizedError) || @error.is_a?(Faraday::UnprocessableEntityError) || - @error.is_a?(Faraday::ClientError) - - errors = { errors: [{ 'title' => 'Client error', - 'key' => @source, - 'detail' => 'A client exception has occurred, job will not be re-tried. ' \ - "original_error: #{@error}.", status: '400', code: '400' }] } - log_outcome_for_claims_api(errors) - raise ::Common::Exceptions::BadRequest, errors + raise_backend_exception(@error.class, @error, 'EVSS400') if @error&.original_status == 400 + raise ::Common::Exceptions::Unauthorized if @error&.original_status == 401 else - errors = { errors: [{ 'title' => 'Unknown error', - 'key' => @source, - 'detail' => 'An unknown error has occurred, and the custom_error file may ' \ - "need to be modified. original_error: #{@error}.", status: @status, - code: @code }] } - log_outcome_for_claims_api(errors) - raise ::Common::Exceptions::ServiceError, errors + raise @error end end private - def log_outcome_for_claims_api(errors) - ClaimsApi::Logger.log('526_docker_container', - detail: "claims_api-526-#{@method}, errors: #{errors}", claim: @claim&.id) - end + def raise_backend_exception(source, error, key = 'EVSS') + error_details = get_error_details(error) - def get_status - @status = if @error.respond_to?(:status) - @error.status - elsif @error.respond_to?(:status_code) - @error.status_code - else - '500' - end + raise ::Common::Exceptions::BackendServiceException.new( + key, + { source: }, + error&.original_status, + error_details + ) end - def get_code - @code = if @error.respond_to?(:va900?) - 'VA900' - elsif @error.respond_to?(:status_code) - @error.status_code - else - @status - end + def get_error_details(error) + all_errors = [] + error.original_body[:messages].each do |err| + symbolized_error = err.deep_symbolize_keys + all_errors << collect_errors(symbolized_error) + end + all_errors end - def get_source - if (@error.respond_to?(:key) && @error.key.present?) || - (@error.respond_to?(:backtrace) && @error.backtrace.present?) - matches = if @error.backtrace.nil? - @error.key[0].match(/vets-api(\S*) (.*)/) - else - @error.backtrace[0].match(/vets-api(\S*) (.*)/) - end - spliters = matches[0].split(':') - @source = spliters[0] - end + def collect_errors(symbolized_error) + severity = symbolized_error[:severity] || nil + detail = symbolized_error[:detail] || nil + text = symbolized_error[:text] || nil + key = symbolized_error[:key] || nil + { + key:, + severity:, + detail:, + text: + }.compact! end end end diff --git a/modules/claims_api/lib/evss_service/base.rb b/modules/claims_api/lib/evss_service/base.rb index fe6417eab2a..e37bc8e89b5 100644 --- a/modules/claims_api/lib/evss_service/base.rb +++ b/modules/claims_api/lib/evss_service/base.rb @@ -27,7 +27,7 @@ def submit(claim, data) log_outcome_for_claims_api('submit', 'success', resp, claim) resp # return is for v1 Sidekiq worker rescue => e - error_handler(e, claim, 'submit') + error_handler(e) end end @@ -43,7 +43,7 @@ def validate(claim, data) detail = e.respond_to?(:original_body) ? e.original_body : e log_outcome_for_claims_api('validate', 'error', detail, claim) - error_handler(e, claim, 'validate') + error_handler(e) end end @@ -85,25 +85,17 @@ def access_token @auth_token ||= ClaimsApi::V2::BenefitsDocuments::Service.new.get_auth_token end - # v1/disability_compenstaion_controller expects different values then the docker container provides - def format_docker_container_error_for_v1(errors) - errors.each do |err| - # need to add a :detail key v1 looks for in it's error reporting, get :text key from docker container - err.merge!(detail: err[:text]).stringify_keys! - end - end - def log_outcome_for_claims_api(action, status, response, claim) ClaimsApi::Logger.log('526_docker_container', detail: "EVSS DOCKER CONTAINER #{action} #{status}: #{response}", claim: claim&.id) end - def custom_error(error, claim, method) - ClaimsApi::CustomError.new(error, claim, method) + def custom_error(error) + ClaimsApi::CustomError.new(error) end - def error_handler(error, claim, method) - custom_error(error, claim, method).build_error + def error_handler(error) + custom_error(error).build_error end end end diff --git a/modules/claims_api/spec/controllers/v1/disability_compensation_controller_spec.rb b/modules/claims_api/spec/controllers/v1/disability_compensation_controller_spec.rb new file mode 100644 index 00000000000..2a6d6e8ae40 --- /dev/null +++ b/modules/claims_api/spec/controllers/v1/disability_compensation_controller_spec.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +# spec/controllers/disability_compensation_controller_spec.rb + +require 'rails_helper' + +RSpec.describe ClaimsApi::V1::Forms::DisabilityCompensationController, type: :controller do + describe '#format_526_errors' do + it 'formats errors correctly' do + error = [ + { + key: 'header.va_eauth_birlsfilenumber.Invalid', + severity: 'ERROR', + text: 'Size must be between 8 and 9' + } + ] + + formatted_error = subject.send(:format_526_errors, error) + + expect(formatted_error).to match_array([ + { status: 422, detail: "#{error[0][:key]}, #{error[0][:text]}", + source: error[0][:key] } + ]) + end + end +end diff --git a/modules/claims_api/spec/requests/v1/claims_request_spec.rb b/modules/claims_api/spec/requests/v1/claims_request_spec.rb index cd0395aee41..433e24fca21 100644 --- a/modules/claims_api/spec/requests/v1/claims_request_spec.rb +++ b/modules/claims_api/spec/requests/v1/claims_request_spec.rb @@ -354,4 +354,44 @@ end end end + + # possible to have errors saved in production that were saved with this wrapper + # so need to make sure they do not break the formatter, even though the + # key of 400 will still show as the source, it will return the claim instead of saying 'not found' + context 'when a claim has an evss_response message with a key that is an integer' do + let(:err_message) do + [{ + 'key' => 400, + 'severity' => 'FATAL', + 'text' => + { 'messages' => + [{ + 'key' => 'form526.submit.establishClaim.serviceError', + 'severity' => 'FATAL', + 'text' => 'Claim not established. System error with BGS. GUID: 00797c5d-89d4-4da6-aab7-24b4ad0e4a4f' + }] } + }] + end + + it 'shows correct error message despite the key being an integer' do + mock_acg(scopes) do |auth_header| + create(:auto_established_claim, + source: 'abraham lincoln', + auth_headers: auth_header, + evss_id: 600_118_851, + id: 'd5536c5c-0465-4038-a368-1a9d9daf65c9', + status: 'errored', + evss_response: err_message) + VCR.use_cassette('bgs/claims/claim') do + headers = request_headers.merge(auth_header) + get('/services/claims/v1/claims/d5536c5c-0465-4038-a368-1a9d9daf65c9', params: nil, headers:) + expect(response.status).not_to eq(404) + body = JSON.parse(response.body) + expect(body['errors'][0]['detail']).not_to eq('Claim not found') + expect(body['errors'][0]['source']).to eq('400') + expect(body['errors'][0]['detail']).to include('Claim not established') + end + end + end + end end diff --git a/modules/claims_api/spec/sidekiq/claim_custom_error_spec.rb b/modules/claims_api/spec/sidekiq/claim_custom_error_spec.rb index 65fb42793bb..4e492530017 100644 --- a/modules/claims_api/spec/sidekiq/claim_custom_error_spec.rb +++ b/modules/claims_api/spec/sidekiq/claim_custom_error_spec.rb @@ -28,78 +28,62 @@ end describe 'errors are funneled as service errors and set to raise and not re-try' do - context 'claim_establisher sends a backend exception' do - let(:message) { OpenStruct.new(status: 500, detail: nil, code: 'VA900', source: '') } - let(:backend_error) { Common::Exceptions::BackendServiceException.new(backtrace.backtrace, message) } - let(:backend_error_submit) { ClaimsApi::CustomError.new(backend_error, claim, 'submit') } - - it 'handles it as a service error' do - backend_error_submit.build_error - backend_error_submit.send(:build_error) - rescue => e - expect(e.errors[0]['detail']).to include 'BackendServiceException' - end - end - - context 'claim_establisher sends a Faraday ConnectionFailed' do - let(:faraday_error) { Faraday::ConnectionFailed.new(backtrace) } - let(:faraday_error_submit) { ClaimsApi::CustomError.new(faraday_error, claim, 'validate') } - - it 'handles the faraday error correctly' do - faraday_error_submit.build_error - faraday_error_submit.send(:build_error) - rescue => e - expect(e.errors[0]['detail']).to include 're-tryable' - end - end - - context 'claim_establisher sends a Faraday::ServerError' do - let(:faraday_error) { Faraday::ServerError.new(backtrace) } - let(:faraday_error_submit) { ClaimsApi::CustomError.new(faraday_error, claim, 'validate') } - - it 'handles the faraday error correctly' do - faraday_error_submit.build_error - faraday_error_submit.send(:build_error) - rescue => e - expect(e.errors[0]['detail']).to include 're-tryable' + context 'no longer wraps the error and sets the key as an integer' do + error_original_body = { + messages: [ + { + 'key' => 'form526.submit.establishClaim.serviceError', + 'severity' => 'FATAL', + 'text' => 'Claim not established. System error with BGS. GUID: 00797c5d-89d4-4da6-aab7-24b4ad0e4a4f' + } + ] + } + + let(:backend_error) do + Common::Exceptions::BackendServiceException.new( + backtrace.backtrace, + {}, + 400, + error_original_body + ) end - end - end - describe 'errors are funneled as re-tryable' do - context 'claim_establisher sends a ActiveRecord::RecordInvalid' do - let(:active_record_error) { ActiveRecord::RecordInvalid.new(claim) } - let(:active_record_error_submit) { ClaimsApi::CustomError.new(active_record_error, claim, 'submit') } + let(:backend_error_submit) { ClaimsApi::CustomError.new(backend_error) } - it 'handles it as a client exception' do - active_record_error_submit.build_error - active_record_error_submit.send(:build_error) + it 'correctly set the key as the string value from the error message' do + backend_error_submit.build_error rescue => e - expect(e.errors[0]['detail']).to include 'client exception' + expect(e.key).to be_a(String) + expect(e.key).not_to be_an_instance_of(Integer) end end - context 'claim_establisher sends a Faraday::BadRequestError' do - let(:bad_request_error) { Faraday::BadRequestError.new(claim) } - let(:bad_request_error_submit) { ClaimsApi::CustomError.new(bad_request_error, claim, 'submit') } - - it 'handles it as a client exception' do - bad_request_error_submit.build_error - bad_request_error_submit.send(:build_error) - rescue => e - expect(e.errors[0]['detail']).to include 'client exception' + context 'the BRLS file number is the wrong size' do + error_original_body = { + messages: [ + { + 'key' => 'header.va_eauth_birlsfilenumber.Invalid', + 'severity' => 'ERROR', + 'text' => 'Size must be between 8 and 9' + } + ] + } + + let(:backend_error) do + Common::Exceptions::BackendServiceException.new( + backtrace.backtrace, + { status: 400, detail: nil, code: 'VA900', source: nil }, + 400, + error_original_body + ) end - end - context 'claim_establisher sends a Faraday::UnprocessableEntityError' do - let(:unprocessable_error) { Faraday::UnprocessableEntityError.new(claim) } - let(:unprocessable_error_submit) { ClaimsApi::CustomError.new(unprocessable_error, claim, 'submit') } + let(:backend_error_submit) { ClaimsApi::CustomError.new(backend_error) } - it 'handles it as a client exception' do - unprocessable_error_submit.build_error - unprocessable_error_submit.send(:build_error) + it 'sets the evss_response to the original body error message' do + backend_error_submit.build_error rescue => e - expect(e.errors[0]['detail']).to include 'client exception' + expect(e.original_body[0]).to eq(error_original_body[:messages][0].deep_symbolize_keys) end end end diff --git a/modules/claims_api/spec/sidekiq/claim_establisher_spec.rb b/modules/claims_api/spec/sidekiq/claim_establisher_spec.rb index b17a0956063..c38b1ec22cd 100644 --- a/modules/claims_api/spec/sidekiq/claim_establisher_spec.rb +++ b/modules/claims_api/spec/sidekiq/claim_establisher_spec.rb @@ -57,18 +57,17 @@ let(:errors) do [{ 'title' => 'Operation failed', 'detail' => 'Operation failed', 'code' => 'VA900', 'status' => '400' }] end - let(:custom_error_submit) { ClaimsApi::CustomError.new(backend_error, claim, 'submit') } - let(:backend_error) { Common::Exceptions::BackendServiceException } it 'sets the status of the claim to an error if it raises an Common::Exceptions::BackendServiceException error' do evss_service_stub = instance_double('ClaimsApi::EVSSService::Base') allow(ClaimsApi::EVSSService::Base).to receive(:new) { evss_service_stub } - allow(evss_service_stub).to receive(:submit).and_raise( - ::Common::Exceptions::BackendServiceException - ) - expect do - subject.new.perform(claim.id) - end.to raise_error(Common::Exceptions::BackendServiceException) + allow(evss_service_stub).to receive(:submit).and_raise(Common::Exceptions::BackendServiceException.new( + 'EVSS400', + 'source', + '400', + errors + )) + subject.new.perform(claim.id) claim.reload expect(claim.evss_id).to be_nil expect(claim.evss_response).to eq(errors)