Skip to content

Return 404 instead of 500 for unknown resource type for Sql#5513

Merged
mikaelweave merged 2 commits intomainfrom
agent/fix-resource-type-404
Apr 29, 2026
Merged

Return 404 instead of 500 for unknown resource type for Sql#5513
mikaelweave merged 2 commits intomainfrom
agent/fix-resource-type-404

Conversation

@mikaelweave
Copy link
Copy Markdown
Contributor

@mikaelweave mikaelweave commented Apr 21, 2026

Description

SqlServerFhirModel.GetResourceTypeId() performed a case-sensitive dictionary lookup and let
KeyNotFoundException bubble up, producing 500 Internal Server Error for any unknown or
mis-cased resource type that reached the SQL data layer (for example, a case-mismatched
DELETE /patient/{id} that slipped past routing).

Per FHIR, an unknown resource type should produce 404 Not Found, not 500.

Reproduction

  1. Start the server.
  2. Issue DELETE https://localhost:44348/patient/123?hardDelete=true (lowercase).
  3. Current behavior: 500.
  4. Expected behavior: 404.

Note: a complementary fix in a separate PR makes the routing layer return 404 for
mis-cased resource type segments before they reach the SQL layer. This PR is the
defense-in-depth fix at the data layer for any path that still reaches it
(e.g. internal callers, future code paths).

Fix

Changed GetResourceTypeId() to use TryGetValue() and throw ResourceNotFoundException
(already mapped to 404 by the exception filter) instead of letting KeyNotFoundException
surface as 500.

Dictionary comparers for resource type lookups remain StringComparer.Ordinal — the FHIR
spec requires case-sensitive resource type matching.

Now this request will return the below as 404 vs a 500

{
  "resourceType": "OperationOutcome",
  "id": "f84d47cb-357b-43f8-9efb-941fd7f512f8",
  "meta": {
    "lastUpdated": "2026-04-29T21:07:03.188648+00:00"
  },
  "issue": [
    {
      "severity": "error",
      "code": "not-found",
      "diagnostics": "Resource type 'patient' is not a known resource type."
    }
  ]
}

Impact

  • Fixes incorrect 500 responses for unknown / case-mismatched resource types reaching
    the SQL data layer.
  • All other callers of GetResourceTypeId() now get a clean 404 instead of an unhandled 500.
  • CosmosDB path is not affected by this bug.

Related Work Items

AB#190027

Testing

  • Existing PR coverage for the SQL GetResourceTypeId() path.

Risks and Follow-ups

  • Routing-layer fix for mis-cased resource type segments is shipped in a separate PR.

@mikaelweave mikaelweave requested a review from a team as a code owner April 21, 2026 16:09
…e types

Bug: DELETE requests with unknown or case-mismatched resource types threw
KeyNotFoundException from GetResourceTypeId() via direct dictionary access.
The exception filter had no handler for KeyNotFoundException, resulting in
HTTP 500 instead of 404.

Fix: Change GetResourceTypeId() to use TryGetValue and throw
ResourceNotFoundException (already mapped to 404 by the exception filter)
instead of letting KeyNotFoundException bubble up as 500.

Dictionary comparers remain StringComparer.Ordinal — the FHIR spec requires
case-sensitive resource type matching, so 'patient' correctly returns 404.

AB#190027
Validates that GetResourceTypeId returns the correct ID for known types
and throws ResourceNotFoundException for unknown types (e.g., lowercase
'patient'), covering the fix for AB#190027.

AB#190027
@mikaelweave mikaelweave force-pushed the agent/fix-resource-type-404 branch from 763cb3c to 51f3a2b Compare April 21, 2026 16:38
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 21, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@5bb9494). Learn more about missing BASE report.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #5513   +/-   ##
=======================================
  Coverage        ?   77.61%           
=======================================
  Files           ?      983           
  Lines           ?    37746           
  Branches        ?     6050           
=======================================
  Hits            ?    29296           
  Misses          ?     7081           
  Partials        ?     1369           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mikaelweave mikaelweave marked this pull request as draft April 28, 2026 15:47
@mikaelweave mikaelweave marked this pull request as ready for review April 29, 2026 05:52
@mikaelweave mikaelweave added this to the FY26\Q4\2Wk\2Wk22 milestone Apr 29, 2026
@mikaelweave mikaelweave added No-PaaS-breaking-change Azure API for FHIR Label denotes that the issue or PR is relevant to the Azure API for FHIR Azure Healthcare APIs Label denotes that the issue or PR is relevant to the FHIR service in the Azure Healthcare APIs Bug Bug bug bug. Schema Version unchanged No-ADR ADR not needed labels Apr 29, 2026
@mikaelweave mikaelweave changed the title Fix GetResourceTypeId throwing 500 instead of 404 for unknown resource types Deletes Return 405 / 500 Incorrectly Apr 29, 2026
@mikaelweave mikaelweave force-pushed the agent/fix-resource-type-404 branch from 0818472 to 51f3a2b Compare April 29, 2026 19:16
@mikaelweave mikaelweave changed the title Deletes Return 405 / 500 Incorrectly Hard Deletes 500 Incorrectly Apr 29, 2026
@mikaelweave mikaelweave changed the title Hard Deletes 500 Incorrectly Return 404 instead of 500 for unknown resource type in SqlServerFhirModel.GetResourceTypeId Apr 29, 2026
@mikaelweave mikaelweave changed the title Return 404 instead of 500 for unknown resource type in SqlServerFhirModel.GetResourceTypeId Return 404 instead of 500 for unknown resource type for Sql Apr 29, 2026
@mikaelweave mikaelweave merged commit 998ab70 into main Apr 29, 2026
70 of 82 checks passed
@mikaelweave mikaelweave deleted the agent/fix-resource-type-404 branch April 29, 2026 22:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Azure API for FHIR Label denotes that the issue or PR is relevant to the Azure API for FHIR Azure Healthcare APIs Label denotes that the issue or PR is relevant to the FHIR service in the Azure Healthcare APIs Bug Bug bug bug. No-ADR ADR not needed No-PaaS-breaking-change Schema Version unchanged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants