Skip to content

Fix AVM module-not-found returning HTTP 422 instead of 400#2676

Open
liuwuliuyun wants to merge 1 commit into
mainfrom
fix/avm-module-not-found-status-code
Open

Fix AVM module-not-found returning HTTP 422 instead of 400#2676
liuwuliuyun wants to merge 1 commit into
mainfrom
fix/avm-module-not-found-status-code

Conversation

@liuwuliuyun
Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes AvmDocsService.GetVersionsAsync and GetDocumentationAsync throwing InvalidOperationException when a module name is not found, which BaseCommand.GetStatusCode mapped to HTTP 422 (Unprocessable Entity). Since an invalid user-supplied module name is bad input rather than a configuration error, the exception type is changed to ArgumentException, which maps to HTTP 400 (Bad Request).

Changes

  • AvmDocsService.cs: Changed both "module not found" throws from InvalidOperationException to ArgumentException with proper nameof(moduleName) parameter
  • AvmVersionListCommandTests.cs / AvmDocumentationGetCommandTests.cs: Updated test mocks to throw ArgumentException to match the new service behavior

GitHub issue number

N/A — discovered during troubleshooting of HTTP 422 System.InvalidOperationException at AvmDocsService.GetVersionsAsync

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 (not applicable — no tool description or behavior changes visible to users)
    • Validate README.md changes running the script ./eng/scripts/Process-PackageReadMe.ps1 (not applicable)
    • For new or modified tool descriptions, ran ToolDescriptionEvaluator (not applicable — no tool description changes)
    • For tools with new names, update consolidated-tools.json (not applicable)
    • For renamed tools, follow the Tool Rename Checklist (not applicable)
    • For new tools associated with Azure services, add URL to documentation in the PR description (not applicable)

AvmDocsService threw InvalidOperationException when a module name was
not found, which BaseCommand mapped to 422 Unprocessable Entity. An
invalid user-supplied module name is bad input, not a configuration
error, so ArgumentException (mapped to 400 Bad Request) is the correct
exception type.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 adjusts Azure Terraform AVM documentation/version lookup to treat an unknown module name as invalid user input by throwing ArgumentException (HTTP 400) instead of InvalidOperationException (HTTP 422), and updates unit tests and server changelog accordingly.

Changes:

  • Changed AVM “module not found” exceptions in AvmDocsService from InvalidOperationException to ArgumentException(nameof(moduleName)) to map to HTTP 400.
  • Updated AVM command unit tests’ service mocks to throw ArgumentException to match the new service behavior.
  • Added a changelog entry describing the HTTP status code fix.

Invoking Livetests

Copilot submitted PRs are not trustworthy by default. Users with write access 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.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
tools/Azure.Mcp.Tools.AzureTerraform/src/Services/AvmDocsService.cs Reclassifies “module not found” as bad input via ArgumentException to produce HTTP 400.
tools/Azure.Mcp.Tools.AzureTerraform/tests/Azure.Mcp.Tools.AzureTerraform.Tests/AvmVersionListCommandTests.cs Updates mocked exception type for the versions command test path.
tools/Azure.Mcp.Tools.AzureTerraform/tests/Azure.Mcp.Tools.AzureTerraform.Tests/AvmDocumentationGetCommandTests.cs Updates mocked exception type for the documentation command test path.
servers/Azure.Mcp.Server/changelog-entries/yunliu1-fix-avm-module-not-found-status.yaml Documents the bug fix in the server changelog pipeline.
Comments suppressed due to low confidence (2)

tools/Azure.Mcp.Tools.AzureTerraform/tests/Azure.Mcp.Tools.AzureTerraform.Tests/AvmVersionListCommandTests.cs:62

  • This test is meant to validate the new HTTP mapping for a missing module, but it only asserts the response is not OK. Please assert the specific expected status code (BadRequest/400) when the service throws an ArgumentException so the regression to 422 can't slip back in.
            .ThrowsAsync(new ArgumentException("Module not found", "moduleName"));

        var response = await ExecuteCommandAsync("--module-name", "nonexistent-module");

        Assert.NotEqual(HttpStatusCode.OK, response.Status);

tools/Azure.Mcp.Tools.AzureTerraform/tests/Azure.Mcp.Tools.AzureTerraform.Tests/AvmDocumentationGetCommandTests.cs:65

  • This test currently only checks that the response isn't OK, which doesn't validate the intended behavior change (ArgumentException -> HTTP 400). Please assert the specific expected status code (BadRequest/400) when GetDocumentationAsync throws ArgumentException.
            .ThrowsAsync(new ArgumentException("Module not found", "moduleName"));

        var response = await ExecuteCommandAsync("--module-name", "nonexistent", "--module-version", "1.0.0");

        Assert.NotEqual(HttpStatusCode.OK, response.Status);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Untriaged

Development

Successfully merging this pull request may close these issues.

2 participants