Skip to content

Fix azureterraform_azapi_get to return 400 BadRequest for invalid resource type or API version#2657

Merged
vcolin7 merged 3 commits into
mainfrom
error-msg-invest
Jun 16, 2026
Merged

Fix azureterraform_azapi_get to return 400 BadRequest for invalid resource type or API version#2657
vcolin7 merged 3 commits into
mainfrom
error-msg-invest

Conversation

@liuwuliuyun

@liuwuliuyun liuwuliuyun commented May 19, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

When azureterraform_azapi_get is called with an invalid resource type or API version, the tool previously returned 500 InternalServerError. This caused AI agents to interpret the failure as a server-side problem rather than a correctable input error, preventing auto-correction.

This PR fixes the issue by:

  • AzApiDocsService: Wraps the schema lookup in a try/catch that normalizes any non-InvalidDataException from the BicepSchema layer (e.g., the generic Exception thrown when a resource type isn't found and --api-version is omitted) into InvalidDataException, ensuring consistent exception surfacing.
  • AzApiDocsGetCommand: Overrides GetStatusCode to map InvalidDataException → 400 BadRequest, scoping the change strictly to this tool without touching shared core infrastructure.

With these changes, agents receive 400 BadRequest with an actionable message (e.g., "Resource type X does not have an apiVersion "Y". Available versions are: ...") and can retry with corrected input.

GitHub issue number?

N/A

Pre-merge Checklist

  • Required for All PRs
    • Read contribution guidelines
    • PR title clearly describes the change
    • Commit history is clean with descriptive messages (cleanup guide)
    • Added comprehensive tests for new/modified functionality
    • Created a changelog entry if the change falls among the following: new feature, bug fix, UI/UX update, breaking change, or updated dependencies. Follow the changelog entry guide
  • For MCP tool changes:
    • One tool per PR: This PR adds or modifies only one MCP tool for faster review cycles
    • Updated servers/Azure.Mcp.Server/README.md and/or servers/Fabric.Mcp.Server/README.md documentation
    • Validate README.md changes running the script ./eng/scripts/Process-PackageReadMe.ps1. See Package README
    • For new or modified tool descriptions, ran ToolDescriptionEvaluator and obtained a score of 0.4 or more and a top 3 ranking for all related test prompts
    • For tools with new names, including new tools or renamed tools, update consolidated-tools.json
    • For renamed tools, follow the Tool Rename Checklist and tag the PR with the breaking-change label
    • For new tools associated with Azure services or publicly available tools/APIs/products, add URL to documentation in the PR description
  • Extra steps for Azure MCP Server tool changes:
    • Updated command list in servers/Azure.Mcp.Server/docs/azmcp-commands.md
    • Ran ./eng/scripts/Update-AzCommandsMetadata.ps1 to update tool metadata in azmcp-commands.md (required for CI)
    • Updated test prompts in servers/Azure.Mcp.Server/docs/e2eTestPrompts.md
    • 👉 For Community (non-Microsoft team member) PRs:
      • Security review: Reviewed code for security vulnerabilities, malicious code, or suspicious activities before running tests (crypto mining, spam, data exfiltration, etc.)
      • Manual tests run: added comment /azp run mcp - pullrequest - live to run Live Test Pipeline

@liuwuliuyun liuwuliuyun marked this pull request as ready for review May 20, 2026 00:32
@liuwuliuyun liuwuliuyun requested review from a team as code owners May 20, 2026 00:32
@liuwuliuyun liuwuliuyun requested review from RickWinter, alzimmermsft, anannya03, conniey, Copilot, hallipr, jongio and srnagar and removed request for Copilot May 20, 2026 00:32
@vcolin7 vcolin7 requested a review from Copilot June 8, 2026 22:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the azureterraform_azapi_get tool’s error handling so invalid --resource-type / --api-version inputs return 400 BadRequest (instead of 500 InternalServerError), enabling callers (including AI agents) to treat the failure as a correctable input issue.

Changes:

  • Normalize schema lookup failures in AzApiDocsService to surface input-related failures as InvalidDataException.
  • Map InvalidDataException to 400 BadRequest in AzApiDocsGetCommand.
  • Update unit test expectation and add an Azure MCP Server changelog entry documenting the behavior change.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
tools/Azure.Mcp.Tools.AzureTerraform/tests/Azure.Mcp.Tools.AzureTerraform.Tests/AzApiDocsGetCommandTests.cs Updates the failure-path test to expect 400 BadRequest when the docs service throws an InvalidDataException.
tools/Azure.Mcp.Tools.AzureTerraform/src/Services/AzApiDocsService.cs Wraps schema lookup exceptions to normalize certain upstream failures into InvalidDataException.
tools/Azure.Mcp.Tools.AzureTerraform/src/Commands/AzApiDocsGetCommand.cs Overrides status code mapping so InvalidDataException returns 400 BadRequest.
servers/Azure.Mcp.Server/changelog-entries/yunliu1-azapi-get-bad-request.yaml Adds a changelog entry describing the corrected status code behavior for invalid inputs.

Comment thread tools/Azure.Mcp.Tools.AzureTerraform/src/Services/AzApiDocsService.cs Outdated

@vcolin7 vcolin7 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Left a comment on how to handle the different kinds of exception thrown by ResourceVisitor :)

…to throw `InvalidDataException` for unknown resource types (#2840)

Change `GetResourceApiVersions.ResourceVisitor` in the BicepSchema tools to throw `InvalidDataException` for unknown resource types

`GetResourceApiVersions` threw a bare `Exception` (and an unhandled `KeyNotFoundException` when the provider existed but the resource type did not), which `AzApiDocsService` normalized via a broad catch-all that also misclassified real server-side faults as `400 BadRequest`. Throw `InvalidDataException` at the source instead, consistent with `LoadSingleResource`, and drop the catch-all so unexpected exceptions surface as `500`.
@vcolin7 vcolin7 merged commit 057dab2 into main Jun 16, 2026
17 checks passed
@vcolin7 vcolin7 deleted the error-msg-invest branch June 16, 2026 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants