Fix Cosmos' QueryItems silently parsing non-success responses as data#2836
Merged
Conversation
QueryItems read the query stream iterator's ResponseMessage and parsed its content without checking the HTTP status. Throttled (429), auth-failure (403), or unavailable (503) responses were silently added to the result set as data items. Guard the response with EnsureSuccessStatusCode() before parsing, matching the existing convention in ListDatabases/ListContainers, so a non-success response throws a CosmosException instead of being parsed as data. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a data-correctness bug in the Cosmos tool where CosmosService.QueryItems could silently parse non-success ResponseMessage bodies (e.g., 403/429/503 error payloads) as if they were query results. The change aligns QueryItems with the existing stream-response handling pattern already used by other CosmosService methods.
Changes:
- Add
response.EnsureSuccessStatusCode()immediately afterReadNextAsyncinCosmosService.QueryItemsto ensure non-success pages throw instead of being parsed as data. - Add a regression test asserting
QueryItemsthrows on non-success responses. - Add an Azure.Mcp.Server changelog entry documenting the fix.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tools/Azure.Mcp.Tools.Cosmos/src/Services/CosmosService.cs | Validates stream query page HTTP status before parsing JSON to prevent error payloads being treated as data. |
| tools/Azure.Mcp.Tools.Cosmos/tests/Azure.Mcp.Tools.Cosmos.Tests/CosmosServiceTests.cs | Adds regression coverage ensuring QueryItems throws CosmosException on non-success responses. |
| servers/Azure.Mcp.Server/changelog-entries/1780624204134.yaml | Documents the Cosmos QueryItems non-success parsing bug fix in the server changelog entry format. |
QueryItems silently parsing non-success responses as data
alzimmermsft
approved these changes
Jun 9, 2026
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
g2vinay
reviewed
Jun 9, 2026
g2vinay
approved these changes
Jun 9, 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
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.
What does this PR do?
CosmosService.QueryItemsread each page from the query stream iterator (Container.GetItemQueryStreamIterator, aFeedIterator) and parsedResponseMessage.Contentwithout first checking the HTTP status. The stream iterator surfaces non-success responses (throttled429, auth-failure403, unavailable503) as a non-successResponseMessagerather than throwing, so the error JSON body was silently cloned into the result set and returned to the caller as if it were data.The fix adds a
response.EnsureSuccessStatusCode();guard immediately afterReadNextAsyncand beforeJsonDocument.Parse, so a non-success page throws aCosmosExceptioninstead of being parsed as data. This matches the existing convention already used byListDatabasesandListContainersin the same file, which guard their stream responses the same way.GitHub issue number?
Fixes #432
Pre-merge Checklist
Required for All PRs
For MCP tool changes:
servers/Azure.Mcp.Server/README.mdand/orservers/Fabric.Mcp.Server/README.mddocumentationREADME.mdchanges running the script./eng/scripts/Process-PackageReadMe.ps1. See Package READMEToolDescriptionEvaluatorand obtained a score of0.4or more and a top 3 ranking for all related test promptsconsolidated-tools.jsonbreaking-changelabelExtra steps for Azure MCP Server tool changes:
servers/Azure.Mcp.Server/docs/azmcp-commands.md./eng/scripts/Update-AzCommandsMetadata.ps1to update tool metadata inazmcp-commands.md(required for CI)servers/Azure.Mcp.Server/docs/e2eTestPrompts.mdInvoking Livetests
Copilot submitted PRs are not trustworthy by default. Users with
writeaccess to the repo need to validate the contents of this PR before leaving a comment with the text/azp run mcp - pullrequest - live. This will trigger the necessary livetest workflows to complete required validation.