Skip to content

Improve error message for export operations in batch/transaction bundles #5475

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tranhoangtu-it wants to merge 2 commits into
base: main
Choose a base branch
from
+28 −1
Open

Improve error message for export operations in batch/transaction bundles #5475

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
33512ea
fix: improve error message for export operations in bundles (#1459)
tranhoangtu-it Mar 30, 2026
7f863d9
fix: address Copilot review — check path only, use NotSupported issue…
tranhoangtu-it Mar 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion src/Microsoft.Health.Fhir.Core/Resources.Designer.cs
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/Microsoft.Health.Fhir.Core/Resources.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -625,6 +625,9 @@
<data name="ExportOperationCompleted" xml:space="preserve">
<value>Export operation has already completed</value>
</data>
<data name="ExportOperationNotSupportedInBundle" xml:space="preserve">
<value>Export operations ($export) are not supported within batch or transaction bundles. Please submit export requests as standalone operations.</value>
</data>
<data name="ConditionalRequestTooCostly" xml:space="preserve">
<value>The operation was stopped due to the underlying request being too resource intensive. If possible, try narrowing or changing the criteria.</value>
</data>
Expand Down
8 changes: 8 additions & 0 deletions src/Microsoft.Health.Fhir.Shared.Api/Features/Resources/Bundle/BundleHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -539,6 +539,14 @@ private async Task GenerateRequest(EntryComponent entry, int order, Cancellation

var requestUrl = entry.Request?.Url;

// Export operations are not supported within bundles (batch or transaction).
// Return a clear error instead of the cryptic Accept header error that occurs
// when the internal routing fails to set required export headers.
if (requestUrl != null && requestUrl.Contains("$export", StringComparison.OrdinalIgnoreCase))
{
throw new RequestNotValidException(Core.Resources.ExportOperationNotSupportedInBundle);
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The $export detection uses requestUrl.Contains("$export", ...), which will also match query-string values (e.g., Patient?name=$export) and can falsely reject non-export requests. Consider parsing requestUrl as a Uri and checking only the path segments for a segment equal to $export (after unescaping), rather than searching the full URL string.

Suggested change
if (requestUrl != null && requestUrl.Contains("$export", StringComparison.OrdinalIgnoreCase))
{
throw new RequestNotValidException(Core.Resources.ExportOperationNotSupportedInBundle);
if (!string.IsNullOrEmpty(requestUrl))
{
Uri exportDetectionUri = new Uri(LocalHost, requestUrl);
string exportDetectionPath = exportDetectionUri.GetComponents(UriComponents.Path, UriFormat.Unescaped);
bool hasExportPathSegment = exportDetectionPath
.Split(new[] { '/' }, StringSplitOptions.RemoveEmptyEntries)
.Any(segment => segment.Equals("$export", StringComparison.OrdinalIgnoreCase));
if (hasExportPathSegment)
{
throw new RequestNotValidException(Core.Resources.ExportOperationNotSupportedInBundle);
}

Copilot uses AI. Check for mistakes.
}
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This throws RequestNotValidException with the default issue type (invalid). Since the message explicitly says the operation is not supported (and the linked issue asks for Not Supported), pass OperationOutcomeConstants.IssueType.NotSupported (or use an exception type that sets it) so OperationOutcome.issue.code matches the semantics.

Copilot uses AI. Check for mistakes.
Comment on lines +542 to +555
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are existing xUnit tests for bundle handling under src/Microsoft.Health.Fhir.Shared.Api.UnitTests/Features/Resources/Bundle/, but this new $export-in-bundle guard is not covered. Add a unit test that submits a batch/transaction bundle containing an entry with a $export URL and asserts the thrown exception/OperationOutcome contains the new resource message (and the intended issue type).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

@LTA-Thinking LTA-Thinking Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tranhoangtu-it could you add a unit test for this in BundleHandlerTests.cs?

Comment on lines +542 to +555
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is an existing unit test (BundleHandlerTests.GivenABundleWithAnExportPost_WhenProcessed_ThenItIsProcessedCorrectly) that currently expects /$export inside a batch bundle to succeed. With this new guard, that test will fail and should be updated (and replaced with assertions that a bundle containing $export returns the new ExportOperationNotSupportedInBundle message and not-supported issue type).

Copilot generated this review using guidance from repository custom instructions.

// For resources within a transaction, we need to resolve any intra-bundle references and potentially persist any internally assigned ids
HTTPVerb requestMethod = entry.Request.Method.Value;

Expand Down
Loading