Skip to content

Commit

Permalink
WORKING COMMIT Explicit logging of custom Lighthouse exceptions in up…
Browse files Browse the repository at this point in the history
…load provider

Documents submitted via our Lighthouse API client that fail to return a success response raise custom exceptions defined in the Lighthouse::ServiceException class. As such, the previous strategy of capturing and logging the Lighthouse API response directly in LighthouseSupplementalDocumentUploadProvider will never work - these exceptions will be raised by our exisitng Lighthouse API client before we have a chance to inspect the response here.

Updates the provider to rescue from any one of the possible API exceptions raised in this case, properly log the upload as a failure, and re-raise the exception to maintain the current behavior
  • Loading branch information
NB28VT committed Nov 14, 2024
1 parent 9899578 commit 7c57d37
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,14 @@ class LighthouseSupplementalDocumentUploadProvider

STATSD_PROVIDER_METRIC = 'lighthouse_supplemental_document_upload_provider'

# Custom exceptions for Lighthouse API non-200 responses
# thrown in lib/lighthouse/service_exception.rb
LIGHTHOUSE_RESPONSE_EXCEPTION_CLASSES = [
*Lighthouse::ServiceException::ERROR_MAP.values,
Common::Exceptions::Timeout,
Common::Exceptions::ServiceError
].freeze

# Maps VA's internal Document Types to the correct document_type attribute for a Lighthouse526DocumentUpload polling
# record. We need this to create a valid polling record
POLLING_DOCUMENT_TYPES = {
Expand Down Expand Up @@ -61,7 +69,14 @@ def validate_upload_document(lighthouse_document)
# @param file_body [String]
def submit_upload_document(lighthouse_document, file_body)
log_upload_attempt
api_response = BenefitsDocuments::Form526::UploadSupplementalDocumentService.call(file_body, lighthouse_document)

begin
api_response = BenefitsDocuments::Form526::UploadSupplementalDocumentService.call(file_body, lighthouse_document)
rescue *LIGHTHOUSE_RESPONSE_EXCEPTION_CLASSES => e
log_upload_failure(e)
raise e
end

handle_lighthouse_response(api_response)
end

Expand Down Expand Up @@ -114,20 +129,16 @@ def log_upload_success(lighthouse_request_id)
StatsD.increment("#{@statsd_metric_prefix}.#{STATSD_PROVIDER_METRIC}.#{STATSD_SUCCESS_METRIC}")
end

# For logging an error response from the Lighthouse Benefits Document API
#
# @param lighthouse_error_response [Hash] parsed JSON response from the Lighthouse API
# this will be an array of errors
def log_upload_failure(lighthouse_error_response)
def log_upload_failure(exception)
StatsD.increment("#{@statsd_metric_prefix}.#{STATSD_PROVIDER_METRIC}.#{STATSD_FAILED_METRIC}")

Rails.logger.error(
'LighthouseSupplementalDocumentUploadProvider upload failed',
{
**base_logging_info,
lighthouse_error_response:
error_info: exception.to_s
}
)

StatsD.increment("#{@statsd_metric_prefix}.#{STATSD_PROVIDER_METRIC}.#{STATSD_FAILED_METRIC}")
end

# Processes the response from Lighthouse and logs accordingly. If the upload is successful, creates
Expand All @@ -141,8 +152,6 @@ def handle_lighthouse_response(api_response)
lighthouse_request_id = response_body.dig('data', 'requestId')
create_lighthouse_polling_record(lighthouse_request_id)
log_upload_success(lighthouse_request_id)
else
log_upload_failure(response_body)
end
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,55 +207,50 @@
end
end

context 'when we get a non-200 response from Lighthouse' do
let(:error_response_body) do
# From vcr_cassettes/lighthouse/benefits_claims/documents/lighthouse_form_526_document_upload_400.yml
{
'errors' => [
{
'detail' => 'Something broke',
'status' => 400,
'title' => 'Bad Request',
'instance' => Faker::Internet.uuid
}
]
}
end

# See lib/lighthouse/service_exception.rb
context 'when a Lighthouse::ServiceException error is raised' do
before do
# Skip upload attempt logging
allow(provider).to receive(:log_upload_attempt)
end

allow(BenefitsDocuments::Form526::UploadSupplementalDocumentService).to receive(:call)
.with(file_body, lighthouse_document)
.and_return(faraday_response)
RSpec.shared_examples 'log Lighthouse response exception' do |exception_class|
it 'increments a StatsD failure metric, logs the error metadata and re-raises the error' do
error_info = { title: 'error title', detail: 'error message' }
exception = exception_class.new(error_info)

allow(faraday_response).to receive(:body).and_return(error_response_body)
end
allow(BenefitsDocuments::Form526::UploadSupplementalDocumentService).to receive(:call)
.and_raise(exception)

it 'logs to the Rails logger' do
expect(Rails.logger).to receive(:error).with(
'LighthouseSupplementalDocumentUploadProvider upload failed',
{
class: 'LighthouseSupplementalDocumentUploadProvider',
submitted_claim_id: submission.submitted_claim_id,
submission_id: submission.id,
user_uuid: submission.user_uuid,
va_document_type_code: va_document_type,
primary_form: 'Form526',
lighthouse_error_response: error_response_body
}
)
expect(StatsD).to receive(:increment).with(
'my_stats_metric_prefix.lighthouse_supplemental_document_upload_provider.upload_failure'
)
expect(Rails.logger).to receive(:error).with(
'LighthouseSupplementalDocumentUploadProvider upload failed',
{
class: 'LighthouseSupplementalDocumentUploadProvider',
submitted_claim_id: submission.submitted_claim_id,
submission_id: submission.id,
user_uuid: submission.user_uuid,
va_document_type_code: va_document_type,
primary_form: 'Form526',
error_info: error_info.to_s
}
)

provider.submit_upload_document(lighthouse_document, file_body)
expect { provider.submit_upload_document(lighthouse_document, file_body) }.to raise_error(exception)
end
end

it 'increments a StatsD metric' do
expect(StatsD).to receive(:increment).with(
'my_stats_metric_prefix.lighthouse_supplemental_document_upload_provider.upload_failure'
)
describe 'service exceptions' do
error_values = Lighthouse::ServiceException::ERROR_MAP.values

provider.submit_upload_document(lighthouse_document, file_body)
error_values.each do |exception|
it_behaves_like 'log Lighthouse response exception', exception
end

it_behaves_like 'log Lighthouse response exception', Common::Exceptions::Timeout
it_behaves_like 'log Lighthouse response exception', Common::Exceptions::ServiceError
end
end

Expand Down

0 comments on commit 7c57d37

Please sign in to comment.