From bc7a87559a886a4a47da32fe6153b9f6f2374dae Mon Sep 17 00:00:00 2001 From: Annie Tran Date: Fri, 20 Sep 2024 14:18:18 -0500 Subject: [PATCH 1/2] log error body if Lighthouse returns 422 --- lib/decision_review_v1/service.rb | 1 + spec/requests/v1/supplemental_claims_spec.rb | 14 ++++++++++++++ 2 files changed, 15 insertions(+) diff --git a/lib/decision_review_v1/service.rb b/lib/decision_review_v1/service.rb index 28c38a648ba..0563e67828e 100644 --- a/lib/decision_review_v1/service.rb +++ b/lib/decision_review_v1/service.rb @@ -416,6 +416,7 @@ def log_error_details(error:, message: nil) error_class: error.class, error: } + info[:error_body] = error.body if error.instance_of?(Common::Client::Errors::ClientError) && error.status == 422 ::Rails.logger.info(info) end diff --git a/spec/requests/v1/supplemental_claims_spec.rb b/spec/requests/v1/supplemental_claims_spec.rb index 09cb880dce6..515e3e5710e 100644 --- a/spec/requests/v1/supplemental_claims_spec.rb +++ b/spec/requests/v1/supplemental_claims_spec.rb @@ -41,6 +41,17 @@ '{:source=>"Common::Client::Errors::ClientError raised in DecisionReviewV1::Service", :code=>"DR_422"}' end + let(:response_error_body) do + { + 'errors' => [{ 'title' => 'Missing required fields', + 'detail' => 'One or more expected fields were not found', + 'code' => '145', + 'source' => { 'pointer' => '/data/attributes' }, + 'status' => '422', + 'meta' => { 'missing_fields' => ['form5103Acknowledged'] } }] + } + end + before { sign_in_as(user) } describe '#create' do @@ -94,6 +105,9 @@ def personal_information_logs backtrace: anything ) expect(Rails.logger).to receive(:error).with(extra_error_log_message, anything) + allow(Rails.logger).to receive(:info) + expect(Rails.logger).to receive(:info).with({ message: nil, error_class: anything, error: anything, + error_body: response_error_body }) allow(StatsD).to receive(:increment) expect(StatsD).to receive(:increment).with('decision_review.form_995.overall_claim_submission.failure') From 6208115b4d3c1d4e1e81a8fe7fa21a8ee9284060 Mon Sep 17 00:00:00 2001 From: Annie Tran Date: Fri, 20 Sep 2024 16:59:16 -0500 Subject: [PATCH 2/2] only log 422 body for lighthouse responses --- .../appeals/supplemental_claim_services.rb | 8 ++-- lib/decision_review_v1/service.rb | 17 +++++--- spec/requests/v1/higher_level_reviews_spec.rb | 13 +++++- .../v1/notice_of_disagreements_spec.rb | 43 +++++++++++++------ spec/requests/v1/supplemental_claims_spec.rb | 5 +-- 5 files changed, 57 insertions(+), 29 deletions(-) diff --git a/lib/decision_review_v1/appeals/supplemental_claim_services.rb b/lib/decision_review_v1/appeals/supplemental_claim_services.rb index 74213d1060d..6dba51c47d1 100644 --- a/lib/decision_review_v1/appeals/supplemental_claim_services.rb +++ b/lib/decision_review_v1/appeals/supplemental_claim_services.rb @@ -45,7 +45,7 @@ def create_supplemental_claim(request_body:, user:) response, bm = run_and_benchmark_if_enabled do perform :post, 'supplemental_claims', request_body, headers rescue => e - log_formatted(**common_log_params.merge(is_success: false, response_error: e)) + log_formatted(**common_log_params.merge(error_log_params(e))) raise e end log_formatted(**common_log_params.merge(is_success: true, status_code: response.status, body: '[Redacted]')) @@ -101,7 +101,7 @@ def get_supplemental_claim_contestable_issues(user:, benefit_type:) rescue => e # We can freely log Lighthouse's error responses because they do not include PII or PHI. # See https://developer.va.gov/explore/api/decision-reviews/docs?version=v1. - log_formatted(**common_log_params.merge(is_success: false, response_error: e)) + log_formatted(**common_log_params.merge(error_log_params(e))) raise e end raise_schema_error_unless_200_status response.status @@ -141,7 +141,7 @@ def get_supplemental_claim_upload_url(sc_uuid:, file_number:, user_uuid: nil, ap rescue => e # We can freely log Lighthouse's error responses because they do not include PII or PHI. # See https://developer.va.gov/explore/api/decision-reviews/docs?version=v2 - log_formatted(**common_log_params.merge(is_success: false, response_error: e)) + log_formatted(**common_log_params.merge(error_log_params(e))) raise e end end @@ -189,7 +189,7 @@ def put_supplemental_claim_upload(upload_url:, file_upload:, metadata_string:, u rescue => e # We can freely log Lighthouse's error responses because they do not include PII or PHI. # See https://developer.va.gov/explore/api/decision-reviews/docs?version=v2 - log_formatted(**common_log_params.merge(is_success: false, response_error: e)) + log_formatted(**common_log_params.merge(error_log_params(e))) raise e end ensure diff --git a/lib/decision_review_v1/service.rb b/lib/decision_review_v1/service.rb index 0563e67828e..88aeee6bd84 100644 --- a/lib/decision_review_v1/service.rb +++ b/lib/decision_review_v1/service.rb @@ -43,7 +43,7 @@ def create_higher_level_review(request_body:, user:) response = perform :post, 'higher_level_reviews', request_body, headers log_formatted(**common_log_params.merge(is_success: true, status_code: response.status, body: '[Redacted]')) rescue => e - log_formatted(**common_log_params.merge(is_success: false, response_error: e)) + log_formatted(**common_log_params.merge(error_log_params(e))) raise e end raise_schema_error_unless_200_status response.status @@ -88,7 +88,7 @@ def get_higher_level_review_contestable_issues(user:, benefit_type:) rescue => e # We can freely log Lighthouse's error responses because they do not include PII or PHI. # See https://developer.va.gov/explore/api/decision-reviews/docs?version=v1. - log_formatted(**common_log_params.merge(is_success: false, response_error: e)) + log_formatted(**common_log_params.merge(error_log_params(e))) raise e end raise_schema_error_unless_200_status response.status @@ -153,7 +153,7 @@ def create_notice_of_disagreement(request_body:, user:) # rubocop:disable Metric response = perform :post, 'notice_of_disagreements', request_body, headers log_formatted(**common_log_params.merge(is_success: true, status_code: response.status, body: '[Redacted]')) rescue => e - log_formatted(**common_log_params.merge(is_success: false, response_error: e)) + log_formatted(**common_log_params.merge(error_log_params(e))) raise e end raise_schema_error_unless_200_status response.status @@ -230,7 +230,7 @@ def get_notice_of_disagreement_upload_url(nod_uuid:, file_number:, user_uuid: ni rescue => e # We can freely log Lighthouse's error responses because they do not include PII or PHI. # See https://developer.va.gov/explore/api/decision-reviews/docs?version=v2 - log_formatted(**common_log_params.merge(is_success: false, response_error: e)) + log_formatted(**common_log_params.merge(error_log_params(e))) raise e end end @@ -279,7 +279,7 @@ def put_notice_of_disagreement_upload(upload_url:, file_upload:, metadata_string rescue => e # We can freely log Lighthouse's error responses because they do not include PII or PHI. # See https://developer.va.gov/explore/api/decision-reviews/docs?version=v2 - log_formatted(**common_log_params.merge(is_success: false, response_error: e)) + log_formatted(**common_log_params.merge(error_log_params(e))) raise e end ensure @@ -416,10 +416,15 @@ def log_error_details(error:, message: nil) error_class: error.class, error: } - info[:error_body] = error.body if error.instance_of?(Common::Client::Errors::ClientError) && error.status == 422 ::Rails.logger.info(info) end + def error_log_params(error) + log_params = { is_success: false, response_error: error } + log_params[:body] = error.body if error.try(:status) == 422 + log_params + end + def handle_error(error:, message: nil) save_and_log_error(error:, message:) source_hash = { source: "#{error.class} raised in #{self.class}" } diff --git a/spec/requests/v1/higher_level_reviews_spec.rb b/spec/requests/v1/higher_level_reviews_spec.rb index acc85dfde78..38a151f3292 100644 --- a/spec/requests/v1/higher_level_reviews_spec.rb +++ b/spec/requests/v1/higher_level_reviews_spec.rb @@ -32,11 +32,22 @@ is_success: false, http: { status_code: 422, - body: anything + body: response_error_body } } end + let(:response_error_body) do + { + 'errors' => [{ 'title' => 'Missing required fields', + 'detail' => 'One or more expected fields were not found', + 'code' => '145', + 'source' => { 'pointer' => '/' }, + 'status' => '422', + 'meta' => { 'missing_fields' => %w[data included] } }] + } + end + let(:extra_error_log_message) do 'BackendServiceException: {:source=>"Common::Client::Errors::ClientError raised in DecisionReviewV1::Service", ' \ ':code=>"DR_422"}' diff --git a/spec/requests/v1/notice_of_disagreements_spec.rb b/spec/requests/v1/notice_of_disagreements_spec.rb index 4cd23a7fc72..47cb00254bb 100644 --- a/spec/requests/v1/notice_of_disagreements_spec.rb +++ b/spec/requests/v1/notice_of_disagreements_spec.rb @@ -14,6 +14,34 @@ end let(:headers) { { 'CONTENT_TYPE' => 'application/json' } } + let(:error_log_args) do + { + message: 'Overall claim submission failure!', + user_uuid: user.uuid, + action: 'Overall claim submission', + form_id: '10182', + upstream_system: nil, + downstream_system: 'Lighthouse', + is_success: false, + http: { + status_code: 422, + body: response_error_body + }, + version_number: 'v2' + } + end + + let(:response_error_body) do + { + 'errors' => [{ 'title' => 'Missing required fields', + 'detail' => 'One or more expected fields were not found', + 'code' => '145', + 'source' => { 'pointer' => '/data/attributes' }, + 'status' => '422', + 'meta' => { 'missing_fields' => ['boardReviewOption'] } }] + } + end + before { sign_in_as(user) } describe '#create' do @@ -86,20 +114,7 @@ def personal_information_logs it 'adds to the PersonalInformationLog when an exception is thrown and logs to StatsD and logger' do VCR.use_cassette('decision_review/NOD-CREATE-RESPONSE-422_V1') do allow(Rails.logger).to receive(:error) - expect(Rails.logger).to receive(:error).with({ - message: 'Overall claim submission failure!', - user_uuid: user.uuid, - action: 'Overall claim submission', - form_id: '10182', - upstream_system: nil, - downstream_system: 'Lighthouse', - is_success: false, - http: { - status_code: 422, - body: anything - }, - version_number: 'v2' - }) + expect(Rails.logger).to receive(:error).with(error_log_args) expect(Rails.logger).to receive(:error).with( message: "Exception occurred while submitting Notice Of Disagreement: #{extra_error_log_message}", backtrace: anything diff --git a/spec/requests/v1/supplemental_claims_spec.rb b/spec/requests/v1/supplemental_claims_spec.rb index 515e3e5710e..015eb762364 100644 --- a/spec/requests/v1/supplemental_claims_spec.rb +++ b/spec/requests/v1/supplemental_claims_spec.rb @@ -32,7 +32,7 @@ is_success: false, http: { status_code: 422, - body: anything + body: response_error_body } } end @@ -105,9 +105,6 @@ def personal_information_logs backtrace: anything ) expect(Rails.logger).to receive(:error).with(extra_error_log_message, anything) - allow(Rails.logger).to receive(:info) - expect(Rails.logger).to receive(:info).with({ message: nil, error_class: anything, error: anything, - error_body: response_error_body }) allow(StatsD).to receive(:increment) expect(StatsD).to receive(:increment).with('decision_review.form_995.overall_claim_submission.failure')