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

@@ -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
 

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

@@ -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

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

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

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