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

@@ -116,24 +116,52 @@ 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
         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
 

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}'")

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: '<html><body>Internal Server Error</body></html>')
+
+        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)