Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DR-90265] Log error body to DataDog if Lighthouse returns 422 #18559

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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]'))
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
16 changes: 11 additions & 5 deletions lib/decision_review_v1/service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -419,6 +419,12 @@ def log_error_details(error:, message: nil)
::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}" }
Expand Down
13 changes: 12 additions & 1 deletion spec/requests/v1/higher_level_reviews_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"}'
Expand Down
43 changes: 29 additions & 14 deletions spec/requests/v1/notice_of_disagreements_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
13 changes: 12 additions & 1 deletion spec/requests/v1/supplemental_claims_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
is_success: false,
http: {
status_code: 422,
body: anything
body: response_error_body
}
}
end
Expand All @@ -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
Expand Down
Loading