FI-2035: Improve error handling for validator errors #379
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #379 +/- ##
==========================================
- Coverage 76.96% 76.95% -0.01%
==========================================
Files 213 213
Lines 10573 10594 +21
Branches 972 976 +4
==========================================
+ Hits 8137 8153 +16
- Misses 1867 1872 +5
Partials 569 569
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
lib/inferno/dsl/fhir_validation.rb
Outdated
|
|
||
| # The validator should return OperationOutcome both on successful validation and on error, | ||
| # so parse the OO before checking the HTTP status code. | ||
| outcome = FHIR::OperationOutcome.new(JSON.parse(validation_response.body)) |
There was a problem hiding this comment.
We should specifically handle the case of non-json responses and display those in the message without the json parse error.
There was a problem hiding this comment.
Updated to check for this specifically: 
Unfortunately this all put the resource_is_valid? function over rubocop's cyclomatic complexity limit and I had to refactor a bit. Feel free to say the refactorings aren't clear and I can spend a little more time on it
dehall
force-pushed
the
fi-2035-validator-errors
branch
from
a35f63e
to
9dd41ee
Compare
| if response.start_with? '{' | ||
| FHIR::OperationOutcome.new(JSON.parse(response)) | ||
| else | ||
| runnable.add_message('error', "Validator Response: #{response}") |
There was a problem hiding this comment.
Could use a newline here:
dehall
force-pushed
the
fi-2035-validator-errors
branch
from
9dd41ee
to
65be52c
Compare
This reverts commit 0452e64.
Summary
This PR improves error handling when there is a problem in the validator, in two ways:
Problems reported as OperationOutcome with an HTTP status other than 200 will raise an
ErrorInValidatorExceptionwhich renders in the inferno UI as a purple error, and more detail, when given in the OO, will be in the Messages tab:(note - this message is introduced in Fi 1977 slow fhir-validator-wrapper#64 )
(note 2 - a future PR will introduce a better error handler in the validator, which will render errors as OOs. still TBD)
When the response from the validator can't be parsed as an OperationOutcome, for example when the validator returns an HTML page that just says "HTTP 500", that will be wrapped in an
ErrorInValidatorException, and more detail if possible will be in the Messages tab. The overall message will indicate an issue with the validator and be a purple error, but not lead off with the stack trace of the Ruby code where things went off track because that's isn't useful:Please feel free to suggest alternate text. Also I suspect the logic of the modified function could be cleaner so I'm going to keep thinking about how to clean that up.
Testing Guidance
Testing this requires a test kit that actually validates resources via the validator service. I'm not sure what the "easiest" way to do that is, but I loaded the US Core 3.1.1 test kit into the
dev_suitesfolders and it wasn't tough to get running. (There are some things that need to be pruned like references to TLS tests but those lines can be deleted/commented out)Testing the various possibilities also requires an instance of the validator that you can easily control.
inferno-framework/fhir-validator-wrapper#64 introduces an OperationOutcome that is returned while the app is starting up. You may also want to manually add something like
throw new RuntimeException();in a spot of your choosing in the validator to simulate an error there.