Fixes the $expand to return 404 when ValueSet not found#5580
Open
rbans96 wants to merge 7 commits into
Open
Conversation
rbans96
added
Bug
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…e may be null' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5580 +/- ##
==========================================
+ Coverage 77.27% 77.37% +0.09%
==========================================
Files 997 997
Lines 36559 36564 +5
Branches 5549 5549
==========================================
+ Hits 28251 28291 +40
+ Misses 6935 6898 -37
- Partials 1373 1375 +2 🚀 New features to boost your workflow:
|
feordin
reviewed
May 26, 2026
Throw ResourceNotFoundException instead of returning OperationOutcome
when Firely surfaces an unknown ValueSet (HttpStatusCode.NotFound).
The existing OperationOutcomeExceptionFilter maps it to 404.
Added catch (ResourceNotFoundException) { throw; } in the handler to
prevent double Error-level logging. Only Warning is now logged.
FHIR spec only requires error OperationOutcome with non-2xx status;
404 is consistent with Firely NotFound semantics.
rbans96
changed the title
rbans96
changed the title
feordin
previously approved these changes
Jun 2, 2026
abiisnn
previously approved these changes
Jun 3, 2026
Contributor
abiisnn
left a comment
abiisnn
left a comment
There was a problem hiding this comment.
Locally tested!
Before:
After:
…d-log-level-fix # Conflicts: # .github/copilot-instructions.md
abiisnn
approved these changes
Jun 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
FirelyTerminologyServiceProxy.ExpandAsync() logs all exceptions at LogError, including benign "valueset is unknown" scenarios where HTTP 200 is still returned. This triggers FHIR002 monitor and creates unnecessary ICMs.
Also returns 404 status code instead of 200
The FHIR spec (ValueSet $expand) requires an error OperationOutcome with a non-2xx status but does not mandate a specific code. Since Firely surfaces this as HttpStatusCode.NotFound, 404 is the most semantically consistent choice. The existing OperationOutcomeExceptionFilterAttribute already maps ResourceNotFoundException → 404.
Fix: Catch FhirOperationException with "is unknown" separately and log at Warning instead of Error. Added unit tests for both paths.
Return 404 if valueset not found
Related issues
Addresses - https://microsofthealth.visualstudio.com/Health/_workitems/edit/192757
Testing
Added unit tests
FHIR Team Checklist
Semver Change (docs)
Patch|Skip|Feature|Breaking (reason)