Fix AVM module-not-found returning HTTP 422 instead of 400#2676
Fix AVM module-not-found returning HTTP 422 instead of 400#2676liuwuliuyun wants to merge 1 commit into
Conversation
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.
There was a problem hiding this comment.
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
AvmDocsServicefromInvalidOperationExceptiontoArgumentException(nameof(moduleName))to map to HTTP 400. - Updated AVM command unit tests’ service mocks to throw
ArgumentExceptionto 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);
What does this PR do?
Fixes
AvmDocsService.GetVersionsAsyncandGetDocumentationAsyncthrowingInvalidOperationExceptionwhen a module name is not found, whichBaseCommand.GetStatusCodemapped 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 toArgumentException, which maps to HTTP 400 (Bad Request).Changes
AvmDocsService.cs: Changed both "module not found" throws fromInvalidOperationExceptiontoArgumentExceptionwith propernameof(moduleName)parameterAvmVersionListCommandTests.cs/AvmDocumentationGetCommandTests.cs: Updated test mocks to throwArgumentExceptionto match the new service behaviorGitHub issue number
N/A — discovered during troubleshooting of HTTP 422
System.InvalidOperationExceptionatAvmDocsService.GetVersionsAsyncPre-merge Checklist
Updated(not applicable — no tool description or behavior changes visible to users)servers/Azure.Mcp.Server/README.mdand/orservers/Fabric.Mcp.Server/README.mddocumentationValidate(not applicable)README.mdchanges running the script./eng/scripts/Process-PackageReadMe.ps1For new or modified tool descriptions, ran(not applicable — no tool description changes)ToolDescriptionEvaluatorFor tools with new names, update(not applicable)consolidated-tools.jsonFor 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)