From 7927af22f9644536d30b19651a2a3c8c5d8a0284 Mon Sep 17 00:00:00 2001 From: Dylan Hall Date: Thu, 28 Sep 2023 09:12:36 -0400 Subject: [PATCH] FI-2166: improve validator error handling without changing API (#397) * FI-2166: improve validator error handling without changing validate() API * FI-2166 add newline to error message * move comment --- lib/inferno/dsl/fhir_validation.rb | 57 +++++++++++++++++++++--- lib/inferno/exceptions.rb | 13 ++++++ spec/inferno/dsl/fhir_validation_spec.rb | 39 ++++++++++++++++ 3 files changed, 103 insertions(+), 6 deletions(-) diff --git a/lib/inferno/dsl/fhir_validation.rb b/lib/inferno/dsl/fhir_validation.rb index 33aef90cd..5a8436980 100644 --- a/lib/inferno/dsl/fhir_validation.rb +++ b/lib/inferno/dsl/fhir_validation.rb @@ -116,17 +116,34 @@ 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))) + begin + response = call_validator(resource, profile_url) + rescue StandardError => e + # This could be a complete failure to connect (validator isn't running) + # or a timeout (validator took too long to respond). + 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) - 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 response.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 +151,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) { @@ -173,10 +201,27 @@ def issue_message(issue, resource) # @param profile_url [String] # @return [String] the body of the validation response def validate(resource, profile_url) + call_validator(resource, profile_url).body + end + + # @private + def call_validator(resource, profile_url) Faraday.new( url, params: { profile: profile_url } - ).post('validate', resource.source_contents).body + ).post('validate', resource.source_contents) + 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:\n#{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..47e90dc41 100644 --- a/lib/inferno/exceptions.rb +++ b/lib/inferno/exceptions.rb @@ -39,6 +39,19 @@ def result end end + # ErrorInValidatorException is used when an exception occurred in + # calling the validator service, for example a connection timeout + # or an unexpected response format. + # Note: This class extends TestResultException instead of RuntimeError + # to bypass printing the stack trace in the UI, since + # the stack trace of this exception is not likely be useful. + # Instead the message should point to where in the validator an error occurred. + class ErrorInValidatorException < TestResultException + 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)