Fix transaction bundle returning HTTP 408 instead of 4xx for validation errors#5582
Open
rbans96 wants to merge 1 commit into
Open
Fix transaction bundle returning HTTP 408 instead of 4xx for validation errors#5582rbans96 wants to merge 1 commit into
rbans96 wants to merge 1 commit into
Conversation
rbans96
added
Azure Healthcare APIs
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5582 +/- ##
==========================================
- Coverage 77.64% 77.33% -0.31%
==========================================
Files 993 996 +3
Lines 36438 36535 +97
Branches 5517 5539 +22
==========================================
- Hits 28291 28255 -36
- Misses 6789 6911 +122
- Partials 1358 1369 +11 🚀 New features to boost your workflow:
|
fhibf
reviewed
May 22, 2026
| // by a previous entry's failure in parallel execution, not by the client disconnecting. | ||
| // Returning 408 for a validation error (e.g., 400) misleads callers into retrying | ||
| // a permanently failing request. | ||
| bool isClientError = statusCode >= HttpStatusCode.BadRequest |
Contributor
There was a problem hiding this comment.
As discussed offline, the suggestion is to use a flag to detect when a FhirTransactionFailedException happened due a customer caused issue. And then when detecting if the error was caused by a client cancellation, then mark it as a customer issue.
That should remove the need to compare the precedence between BadRequest, InternalServerError and RequestTimeout.
fhibf
reviewed
May 22, 2026
| // Even though the cancellation token is triggered, the 4xx status from the entry | ||
| // takes priority over 408 to avoid misleading callers into retrying permanently failing requests. | ||
| FhirTransactionFailedException fhirTfe = await Assert.ThrowsAsync<FhirTransactionFailedException>(async () => await _bundleHandler.Handle(bundleRequest, cancellationToken)); | ||
| Assert.True(fhirTfe.ResponseStatusCode == System.Net.HttpStatusCode.NotFound); |
Contributor
There was a problem hiding this comment.
I was not expecting this operation to return HTTP404. If that's the case, then we need to change this test to proper submit an Observation that would be ingested if the cancellation did not happen.
rbans96
force-pushed
the
personal/ribans/fix-transaction-408-status-code
branch
from
bc3b76c to
c9eb4cc
Compare
fhibf
reviewed
May 26, 2026
| operationOutcome, | ||
| isCancelled: true)); | ||
|
|
||
| Assert.Equal(HttpStatusCode.RequestTimeout, tce.ResponseStatusCode); |
Contributor
There was a problem hiding this comment.
I had the impression that the results should be different than Timeout.
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
3 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.
Transaction bundle returns HTTP 408 instead of 400 when entry fails validation during parallel execution. In
TransactionExceptionHandler.ThrowTransactionException, whenisCancelledis true, the code unconditionally throwsFhirTransactionCancelledException(hardcoded to 408), ignoring the actual entry error status. During parallel bundle execution, when one entry fails validation, the catch handler cancels the linkedCancellationTokenSource. Other entries with validation errors that complete after the token is cancelled seeIsBundleCancelledByClient() == trueand get misclassified as client cancellations (408) instead of validation failures (400). Sinceawait Task.WhenAllpropagates the first faulted task by array position (not chronological order), the 408 is returned to the caller. Callers correctly treat 408 as transient/retryable per HTTP semantics, causing infinite retry loops and massive request amplification (6-12x) for permanently failing bundles.Fix: When an entry has a concrete client error (4xx, excluding 408), that status code takes priority over the cancellation signal. HTTP 408 is now reserved for genuine timeout/cancellation scenarios only. This aligns with the FHIR specification: 'return an HTTP 400 or 500 type response' for failed transactions.
Addresses : https://microsofthealth.visualstudio.com/Health/_workitems/edit/192974