From 0452e643a4561af09245131026598a95b630697e Mon Sep 17 00:00:00 2001 From: Dylan Hall Date: Mon, 28 Aug 2023 16:59:17 -0400 Subject: [PATCH] FI-2035: Improve error handling for validator errors (#379) * FI-2035: improve error handling for validator errors * FI-2035: refactor resource_is_valid? and check for non-json responses * FI-2035: missed doco param --- lib/inferno/dsl/fhir_validation.rb | 63 +++++++++++++++++++----- lib/inferno/exceptions.rb | 10 ++++ spec/inferno/dsl/fhir_validation_spec.rb | 39 +++++++++++++++ 3 files changed, 101 insertions(+), 11 deletions(-) diff --git a/lib/inferno/dsl/fhir_validation.rb b/lib/inferno/dsl/fhir_validation.rb index 33aef90cd..7f1ae81c2 100644 --- a/lib/inferno/dsl/fhir_validation.rb +++ b/lib/inferno/dsl/fhir_validation.rb @@ -116,17 +116,26 @@ def exclude_message(&block) def resource_is_valid?(resource, profile_url, runnable) profile_url ||= FHIR::Definitions.resource_definition(resource.resourceType).url - outcome = FHIR::OperationOutcome.new(JSON.parse(validate(resource, profile_url))) + outcome, http_status = validate(resource, profile_url, runnable) - message_hashes = outcome.issue&.map { |issue| message_hash_from_issue(issue, resource) } || [] + message_hashes = message_hashes_from_outcome(outcome, resource, profile_url) - message_hashes.concat(additional_validation_messages(resource, profile_url)) + message_hashes + .each { |message_hash| runnable.add_message(message_hash[:type], message_hash[:message]) } - filter_messages(message_hashes) + unless http_status == 200 + raise Inferno::Exceptions::ErrorInValidatorException, + 'Error occurred in the validator. Review Messages tab or validator service logs for more information.' + end message_hashes - .each { |message_hash| runnable.add_message(message_hash[:type], message_hash[:message]) } .none? { |message_hash| message_hash[:type] == 'error' } + rescue Inferno::Exceptions::ErrorInValidatorException + raise + rescue StandardError => e + runnable.add_message('error', e.message) + raise Inferno::Exceptions::ErrorInValidatorException, + 'Error occurred in the validator. Review Messages tab or validator service logs for more information.' end # @private @@ -134,6 +143,17 @@ def filter_messages(message_hashes) message_hashes.reject! { |message| exclude_message.call(Entities::Message.new(message)) } if exclude_message end + # @private + def message_hashes_from_outcome(outcome, resource, profile_url) + message_hashes = outcome.issue&.map { |issue| message_hash_from_issue(issue, resource) } || [] + + message_hashes.concat(additional_validation_messages(resource, profile_url)) + + filter_messages(message_hashes) + + message_hashes + end + # @private def message_hash_from_issue(issue, resource) { @@ -171,12 +191,33 @@ def issue_message(issue, resource) # # @param resource [FHIR::Model] # @param profile_url [String] - # @return [String] the body of the validation response - def validate(resource, profile_url) - Faraday.new( - url, - params: { profile: profile_url } - ).post('validate', resource.source_contents).body + # @param runnable [Inferno::Entities::Test] + # @return [[Array(FHIR::OperationOutcome, Number)] the validation response and HTTP status code + def validate(resource, profile_url, runnable) + begin + response = Faraday.new( + url, + params: { profile: profile_url } + ).post('validate', resource.source_contents) + rescue StandardError => e + runnable.add_message('error', e.message) + raise Inferno::Exceptions::ErrorInValidatorException, "Unable to connect to validator at #{url}." + end + outcome = operation_outcome_from_validator_response(response.body, runnable) + + [outcome, response.status] + end + + # @private + def operation_outcome_from_validator_response(response, runnable) + if response.start_with? '{' + FHIR::OperationOutcome.new(JSON.parse(response)) + else + runnable.add_message('error', "Validator Response: #{response}") + raise Inferno::Exceptions::ErrorInValidatorException, + 'Validator response was an unexpected format. '\ + 'Review Messages tab or validator service logs for more information.' + end end end diff --git a/lib/inferno/exceptions.rb b/lib/inferno/exceptions.rb index dc3cc8e3b..bcd059594 100644 --- a/lib/inferno/exceptions.rb +++ b/lib/inferno/exceptions.rb @@ -39,6 +39,16 @@ def result end end + class ErrorInValidatorException < TestResultException + # This extends TestResultException instead of RuntimeError + # to bypass printing the stack trace in the UI. + # (The stack trace of this exception may not be useful, + # instead the message should point to where in the validator an error occurred) + def result + 'error' + end + end + class ParentNotLoadedException < RuntimeError def initialize(klass, id) super("No #{klass.name.demodulize} found with id '#{id}'") diff --git a/spec/inferno/dsl/fhir_validation_spec.rb b/spec/inferno/dsl/fhir_validation_spec.rb index 717edd159..0458e9d17 100644 --- a/spec/inferno/dsl/fhir_validation_spec.rb +++ b/spec/inferno/dsl/fhir_validation_spec.rb @@ -130,6 +130,45 @@ end end + context 'with error from validator' do + let(:error_outcome) do + { + resourceType: 'OperationOutcome', + issue: [ + { + severity: 'fatal', + code: 'structure', + diagnostics: 'Validator still warming up... Please wait', + details: { + text: 'Validator still warming up... Please wait' + } + } + ] + }.to_json + end + + it 'throws ErrorInValidatorException when validator not ready yet' do + stub_request(:post, "#{validation_url}/validate?profile=#{profile_url}") + .with(body: resource_string) + .to_return(status: 503, body: error_outcome) + + expect do + validator.resource_is_valid?(resource, profile_url, runnable) + end.to raise_error(Inferno::Exceptions::ErrorInValidatorException) + expect(runnable.messages.first[:message]).to include('Validator still warming up... Please wait') + end + + it 'throws ErrorInValidatorException for non-JSON response' do + stub_request(:post, "#{validation_url}/validate?profile=#{profile_url}") + .with(body: resource_string) + .to_return(status: 500, body: 'Internal Server Error') + + expect do + validator.resource_is_valid?(resource, profile_url, runnable) + end.to raise_error(Inferno::Exceptions::ErrorInValidatorException) + end + end + it 'posts the resource with primitive extensions intact' do stub_request(:post, "#{validation_url}/validate?profile=#{profile_url}") .with(body: resource_string)