API-34957 v1 error formatter update

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

modules/claims_api/app/controllers/claims_api/v1/forms/disability_compensation_controller.rb

@@ -252,7 +252,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
 

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

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

modules/claims_api/lib/custom_error.rb

@@ -8,84 +8,53 @@ def initialize(error, claim, method)
       @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
-
-      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
+    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
+
+        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

modules/claims_api/lib/evss_service/base.rb

@@ -85,14 +85,6 @@ 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)

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

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'
+    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
-
-    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'
-      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, claim, 'submit') }
 
-      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, claim, 'submit') }
 
-      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

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)