[AzureBackup] Surface RequestFailedException on dual-backend vault list failure; tolerate DPP policy FormatException#2805
Merged
shrja-ms merged 3 commits intoJun 9, 2026
Conversation
NEW-5: When azurebackup_vault_list and azurebackup_governance_find-unprotected have both RSV and DPP backends fail with RequestFailedException, surface a single RequestFailedException (preserving HTTP status and combining both messages) instead of an InvalidOperationException so the failure is correctly classified as an Azure service error rather than an MCP-side bug. NEW-6: Fix azurebackup_policy_get and azurebackup_policy_list (DPP vaults) failing with FormatException from XmlConvert.ToTimeSpan when policies contain ISO 8601 duration values the Azure SDK cannot parse. policy_list now skips unparseable policies; policy_get falls back to listing the vault's policies and matching by name.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR delivers two AzureBackup toolset reliability fixes driven by production telemetry: (1) preserve Azure service error classification when both vault-list backends fail with RequestFailedException, and (2) tolerate Azure SDK FormatException during DPP policy materialization so callers can still retrieve usable policy data.
Changes:
- Return a
RequestFailedException(instead ofInvalidOperationException) when both RSV and DPP vault listing operations fail withRequestFailedException, preserving an actionable HTTP status. - Add
FormatException-tolerant logic for DPPpolicy_get/policy_listby falling back to listing + matching and by custom-enumerating results. - Update unit tests for the revised vault-list exception behavior and add a changelog entry documenting both fixes.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tools/Azure.Mcp.Tools.AzureBackup/src/Services/AzureBackupService.cs | Adjusts dual-backend vault-list failure wrapping to surface RequestFailedException and preserve status/error details. |
| tools/Azure.Mcp.Tools.AzureBackup/src/Services/DppBackupOperations.cs | Adds FormatException tolerance for DPP policy get/list to avoid total failure on SDK duration parsing issues. |
| tools/Azure.Mcp.Tools.AzureBackup/tests/Azure.Mcp.Tools.AzureBackup.Tests/Services/AzureBackupServiceTests.cs | Updates/adds tests validating the new RequestFailedException wrapping behavior and non-zero status preference. |
| servers/Azure.Mcp.Server/changelog-entries/azurebackup-fix-vault-list-and-policy-format-exception.yml | Documents the two bug fixes in the server changelog entry format. |
…ot-break on per-item FormatException 1. AzureBackupService.BuildBothVaultListingsFailedException now picks one source exception (preferring non-zero HTTP status) and takes both Status and ErrorCode from it, so callers never see a mismatched (Status, ErrorCode) pair. 2. DppBackupOperations.ListPoliciesAsync and ListJobsAsync now continue past a single-item FormatException instead of breaking out of the enumerator. A small consecutive-failure cap (3) prevents infinite loops when the SDK enumerator is left in a permanently-broken state by a page-level deserialization failure. This also makes GetPolicyAsync's list-and-match fallback resilient to bad policies positioned before the requested one. Adds a NEW-5 test asserting the (Status, ErrorCode) pair is sourced consistently.
…rors; fix KQL classifier - RecoveryPointGetCommand: add BMSUserErrorContainerNameIncorrectFormat and 403 AuthorizationFailed error messages with guidance - ProtectedItemGetCommand: add BMSUserErrorContainerNameIncorrectFormat, BMSUserErrorProtectedItemNameIncorrectFormat, and 403 error messages - KQL classifier: add ValidationError, UnauthorizedAccessException, and CredentialUnavailableException to Customer bucket (reclassifies 26 Unknown hits) - SKILL.md: update error classification decision tree to match
alzimmermsft
reviewed
Jun 3, 2026
alzimmermsft
reviewed
Jun 3, 2026
alzimmermsft
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.
Summary
Fixes Issue 5, Issue 6 from weekly telemetry, plus customer 4xx UX improvements and KQL classifier update.
Issue 5 —
vault_getInvalidOperationException 422When both RSV and DPP backends fail in
ListVaultsAsync, theTask.WhenAllresult was surfacing as a rawInvalidOperationException(classified as MCP bug). NowAzureBackupService.ListVaultsAsyncunwraps theAggregateExceptionand re-throws aRequestFailedExceptionwith the original HTTP status code, so the error is correctly classified as an Azure Service error.Issue 6 —
policy_getFormatException 500Same Azure SDK
XmlConvert.ToTimeSpanfamily bug as Issue 2 (azure-sdk-for-net#59306) but in policy retention deserializer (DataProtectionBackupAbsoluteDeleteSetting). Applied the same try/catch workaround:DppBackupOperations.GetPolicyAsync: catchFormatExceptionand fall back to listing all policies + matching by nameDppBackupOperations.ListPoliciesAsync: tolerate per-itemFormatExceptionduring enumeration with a consecutive-failure cap (max 3)Customer 4xx UX — actionable error messages
RecoveryPointGetCommand: addedBMSUserErrorContainerNameIncorrectFormaterror message with guidance to use fully qualified container name, plus 403AuthorizationFailedmessage with RBAC role hintProtectedItemGetCommand: addedBMSUserErrorContainerNameIncorrectFormat,BMSUserErrorProtectedItemNameIncorrectFormat, and 403 error messagesKQL telemetry classifier fix
Updated all 3 classification blocks in kql-queries.md and the decision tree in SKILL.md:
ValidationError→ Customer (was Unknown — 19 hits/wk, user omitted required options)System.UnauthorizedAccessException→ Customer (was Unknown — 7 hits/wk, broken credentials)Azure.Identity.CredentialUnavailableException→ Customer (2 hits/wk, auth not configured)This reclassifies all 26 "Unknown" hits from last week into the correct Customer bucket.
Changes
AzureBackupService.csAggregateExceptionfrom dual-backend vault list; re-throwRequestFailedExceptionwith original statusGetPolicyAsyncFormatException fallback;ListPoliciesAsyncper-item FormatException tolerance with capValidationError,UnauthorizedAccessException,CredentialUnavailableException→ Customerchangelog-entries/*.ymlAzureBackupServiceTests.csInvoking 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.