[AzureBackup] Surface RequestFailedException on dual-backend vault list failure; tolerate DPP policy FormatException

servers/Azure.Mcp.Server/changelog-entries/azurebackup-fix-vault-list-and-policy-format-exception.yml

@@ -0,0 +1,5 @@
+changes:
+  - section: "Bugs Fixed"
+    description: "Fixed `azurebackup_vault_list` and `azurebackup_governance_find-unprotected` so that when both the RSV and DPP backends fail with `RequestFailedException`, a single `RequestFailedException` is surfaced (preserving the HTTP status and combining both error messages) instead of an `InvalidOperationException`. This ensures the failure is correctly classified as an Azure service error rather than an MCP-side bug."
+  - section: "Bugs Fixed"
+    description: "Fixed `azurebackup_policy_get` and `azurebackup_policy_list` (DPP vaults) failing with `FormatException` from `XmlConvert.ToTimeSpan` when policies contain ISO 8601 duration values that the Azure SDK cannot parse (e.g. fractional seconds, week designators in retention rules). `policy_list` now skips policies that the SDK cannot parse and returns the rest; `policy_get` falls back to listing the vault's policies and matching by name. Tracks the Azure SDK gap in `Azure.ResourceManager.DataProtectionBackup`."

tools/Azure.Mcp.Tools.AzureBackup/skills/azurebackup-telemetry-report/SKILL.md

@@ -164,7 +164,13 @@ Is success == true?
       Is ExceptionType FormatException / ArgumentNullException / ArgumentException / InvalidOperationException?
         └─ YES → "MCP Tool Bug"
         └─ NO →
-            Is StatusCode 400/403/404?
+            Is ExceptionType ValidationError?
+              └─ YES → "Customer (4xx)" (user omitted required options)
+            Is ExceptionType System.UnauthorizedAccessException?
+              └─ YES → "Customer (4xx)" (broken credentials / expired token)
+            Is ExceptionType Azure.Identity.CredentialUnavailableException?
+              └─ YES → "Customer (4xx)" (auth not configured)
+            Is StatusCode 400/401/403/404?
               └─ YES → "Customer (4xx)"
               └─ NO →
                   Is ExceptionType Azure.RequestFailedException with 5xx?

tools/Azure.Mcp.Tools.AzureBackup/skills/azurebackup-telemetry-report/references/kql-queries.md

@@ -25,8 +25,11 @@ getAzureMcpEvents_ToolCalls(ago(7d), now())
 | extend ErrorCategory = case(
     success == true, "Success",
     ExType in ("System.FormatException", "System.ArgumentNullException", "System.ArgumentException", "System.InvalidOperationException"), "McpToolBug",
+    ExType == "ValidationError", "Customer",
+    ExType == "System.UnauthorizedAccessException", "Customer",
     StatusCode >= 400 and StatusCode < 500, "Customer",
     ExType == "System.Collections.Generic.KeyNotFoundException", "Customer",
+    ExType == "Azure.Identity.CredentialUnavailableException", "Customer",
     ExType == "Azure.RequestFailedException" and StatusCode >= 500, "AzureService",
     ExType == "System.AggregateException", "AzureService",
     "Unknown")
@@ -63,8 +66,11 @@ getAzureMcpEvents_ToolCalls(TwoWeeksAgo, now())
 | extend ErrorCategory = case(
     success == true, "Success",
     ExType in ("System.FormatException", "System.ArgumentNullException", "System.ArgumentException", "System.InvalidOperationException"), "McpToolBug",
+    ExType == "ValidationError", "Customer",
+    ExType == "System.UnauthorizedAccessException", "Customer",
     StatusCode >= 400 and StatusCode < 500, "Customer",
     ExType == "System.Collections.Generic.KeyNotFoundException", "Customer",
+    ExType == "Azure.Identity.CredentialUnavailableException", "Customer",
     ExType == "Azure.RequestFailedException" and StatusCode >= 500, "AzureService",
     ExType == "System.AggregateException", "AzureService",
     "Unknown")
@@ -112,8 +118,11 @@ getAzureMcpEvents_ToolCalls(ago(7d), now())
 | extend StatusCode = toint(extract(@"StatusCode.*?(\d+)", 1, ExMsg))
 | extend ErrorCategory = case(
     ExType in ("System.FormatException", "System.ArgumentNullException", "System.ArgumentException", "System.InvalidOperationException"), "McpToolBug",
+    ExType == "ValidationError", "Customer",
+    ExType == "System.UnauthorizedAccessException", "Customer",
     StatusCode >= 400 and StatusCode < 500, "Customer",
     ExType == "System.Collections.Generic.KeyNotFoundException", "Customer",
+    ExType == "Azure.Identity.CredentialUnavailableException", "Customer",
     ExType == "Azure.RequestFailedException" and StatusCode >= 500, "AzureService",
     ExType == "System.AggregateException", "AzureService",
     "Unknown")

tools/Azure.Mcp.Tools.AzureBackup/src/Commands/ProtectedItem/ProtectedItemGetCommand.cs

@@ -117,6 +117,12 @@ public override async Task<CommandResponse> ExecuteAsync(CommandContext context,
         KeyNotFoundException => "Protected item not found. Verify the item name and vault.",
         RequestFailedException reqEx when reqEx.Status == (int)HttpStatusCode.NotFound =>
             "Protected item not found. Verify the item name and vault.",
+        RequestFailedException reqEx when reqEx.ErrorCode == "BMSUserErrorContainerNameIncorrectFormat" =>
+            $"Container name format is incorrect. Use the fully qualified container name from 'azurebackup protecteditem get' (e.g., 'IaasVMContainer;iaasvmcontainerv2;resourceGroup;vmName'). Details: {reqEx.Message}",
+        RequestFailedException reqEx when reqEx.ErrorCode == "BMSUserErrorProtectedItemNameIncorrectFormat" =>
+            $"Protected item name format is incorrect. Use the fully qualified name from 'azurebackup protecteditem get' (e.g., 'VM;iaasvmcontainerv2;resourceGroup;vmName'). Details: {reqEx.Message}",
+        RequestFailedException reqEx when reqEx.Status == (int)HttpStatusCode.Forbidden =>
+            $"Authorization failed. Ensure the caller has the Backup Reader or Backup Operator role on the vault. Details: {reqEx.Message}",
         RequestFailedException reqEx => reqEx.Message,
         _ => base.GetErrorMessage(ex)
     };

tools/Azure.Mcp.Tools.AzureBackup/src/Commands/RecoveryPoint/RecoveryPointGetCommand.cs

@@ -116,6 +116,10 @@ public override async Task<CommandResponse> ExecuteAsync(CommandContext context,
     {
         RequestFailedException reqEx when reqEx.Status == (int)HttpStatusCode.NotFound =>
             "Recovery point not found. Verify the recovery point ID and protected item.",
+        RequestFailedException reqEx when reqEx.ErrorCode == "BMSUserErrorContainerNameIncorrectFormat" =>
+            $"Container name format is incorrect. Use the fully qualified container name from 'azurebackup protecteditem get' (e.g., 'IaasVMContainer;iaasvmcontainerv2;resourceGroup;vmName'). Details: {reqEx.Message}",
+        RequestFailedException reqEx when reqEx.Status == (int)HttpStatusCode.Forbidden =>
+            $"Authorization failed. Ensure the caller has the Backup Reader or Backup Operator role on the vault. Details: {reqEx.Message}",
         RequestFailedException reqEx => reqEx.Message,
         _ => base.GetErrorMessage(ex)
     };

tools/Azure.Mcp.Tools.AzureBackup/src/Services/AzureBackupService.cs

@@ -61,6 +61,18 @@ private static Exception BuildBothVaultListingsFailedException(
             $"RSV error: {rsvInner.GetType().Name}: {rsvInner.Message} " +
             $"DPP error: {dppInner.GetType().Name}: {dppInner.Message}";
 
+        if (rsvInner is RequestFailedException rsvRfe && dppInner is RequestFailedException dppRfe)
+        {
+            // NEW-5 fix: when both inners are RequestFailedException, return a
+            // RequestFailedException so the command-layer error mapper classifies the
+            // failure as an Azure service error (with the original HTTP status code)
+            // rather than as an MCP-side bug. Pick a single source for the
+            // (Status, ErrorCode) pair so they are guaranteed to come from the same
+            // exception - prefer the side that reports a non-zero HTTP status.
+            var primary = rsvRfe.Status != 0 ? rsvRfe : dppRfe;
+            return new RequestFailedException(primary.Status, combinedMessage, primary.ErrorCode, primary);
+        }
+
         return new InvalidOperationException(combinedMessage, rsvInner);
     }
 

tools/Azure.Mcp.Tools.AzureBackup/src/Services/DppBackupOperations.cs

@@ -421,9 +421,23 @@ public async Task<BackupPolicyInfo> GetPolicyAsync(
         var armClient = await CreateArmClientAsync(tenant, retryPolicy, cancellationToken: cancellationToken);
         var policyId = DataProtectionBackupPolicyResource.CreateResourceIdentifier(subscription, resourceGroup, vaultName, policyName);
         var policyResource = armClient.GetDataProtectionBackupPolicyResource(policyId);
-        var policy = await policyResource.GetAsync(cancellationToken);
 
-        return MapToPolicyInfo(policy.Value.Data);
+        try
+        {
+            var policy = await policyResource.GetAsync(cancellationToken);
+            return MapToPolicyInfo(policy.Value.Data);
+        }
+        catch (FormatException)
+        {
+            // The Azure SDK may throw FormatException when deserializing the policy's
+            // retention/duration fields (XmlConvert.ToTimeSpan limitation in
+            // DataProtectionBackupAbsoluteDeleteSetting). Fall back to listing all
+            // policies and matching by name to work around this SDK limitation.
+            var policies = await ListPoliciesAsync(vaultName, resourceGroup, subscription, tenant, retryPolicy, cancellationToken);
+            return policies.FirstOrDefault(p => p.Name == policyName)
+                ?? throw new InvalidOperationException(
+                    $"Policy '{policyName}' not found or cannot be parsed by the Azure SDK due to an unsupported retention/duration field.");
+        }
     }
 
     public async Task<List<BackupPolicyInfo>> ListPoliciesAsync(
@@ -441,9 +455,42 @@ public async Task<List<BackupPolicyInfo>> ListPoliciesAsync(
         var collection = vaultResource.GetDataProtectionBackupPolicies();
 
         var policies = new List<BackupPolicyInfo>();
-        await foreach (var policy in collection.GetAllAsync(cancellationToken))
+        var enumerator = collection.GetAllAsync(cancellationToken).GetAsyncEnumerator(cancellationToken);
+        // The Azure SDK may throw FormatException when deserializing policies with
+        // non-standard ISO 8601 retention/duration fields (XmlConvert.ToTimeSpan
+        // limitation in DataProtectionBackupAbsoluteDeleteSetting). When that happens
+        // we try to skip past the offending item and continue, so valid policies that
+        // appear after a bad one are still returned. If the SDK enumerator becomes
+        // unusable (typical for page-level deserialization failures), MoveNextAsync
+        // will keep throwing - cap consecutive failures so we cannot loop forever.
+        const int maxConsecutiveFailures = 3;
+        var consecutiveFailures = 0;
+        try
+        {
+            while (true)
+            {
+                try
+                {
+                    if (!await enumerator.MoveNextAsync())
+                    {
+                        break;
+                    }
+
+                    policies.Add(MapToPolicyInfo(enumerator.Current.Data));
+                    consecutiveFailures = 0;
+                }
+                catch (FormatException)
+                {
+                    if (++consecutiveFailures >= maxConsecutiveFailures)
+                    {
+                        break;
+                    }
+                }
+            }
+        }
+        finally
         {
-            policies.Add(MapToPolicyInfo(policy.Data));
+            await enumerator.DisposeAsync();
         }
 
         return policies;
@@ -542,6 +589,13 @@ public async Task<List<BackupJobInfo>> ListJobsAsync(
 
         var jobs = new List<BackupJobInfo>();
         var enumerator = collection.GetAllAsync(cancellationToken).GetAsyncEnumerator(cancellationToken);
+        // The Azure SDK may throw FormatException when deserializing jobs with
+        // non-standard ISO 8601 duration fields (XmlConvert.ToTimeSpan limitation).
+        // Try to skip past the offending item so valid jobs after a bad one are still
+        // returned; cap consecutive failures so a permanently-broken enumerator
+        // (typical for page-level deserialization failures) cannot loop forever.
+        const int maxConsecutiveFailures = 3;
+        var consecutiveFailures = 0;
         try
         {
             while (true)
@@ -554,13 +608,14 @@ public async Task<List<BackupJobInfo>> ListJobsAsync(
                     }
 
                     jobs.Add(MapToJobInfo(enumerator.Current.Data));
+                    consecutiveFailures = 0;
                 }
                 catch (FormatException)
                 {
-                    // The Azure SDK may throw FormatException when deserializing jobs with
-                    // non-standard ISO 8601 duration fields (XmlConvert.ToTimeSpan limitation).
-                    // Return the jobs collected so far rather than failing the entire list.
-                    break;
+                    if (++consecutiveFailures >= maxConsecutiveFailures)
+                    {
+                        break;
+                    }
                 }
             }
         }

tools/Azure.Mcp.Tools.AzureBackup/tests/Azure.Mcp.Tools.AzureBackup.Tests/Services/AzureBackupServiceTests.cs

@@ -228,24 +228,61 @@ public async Task ListVaultsAsync_DppFails_ReturnsRsvOnly()
     }
 
     [Fact]
-    public async Task ListVaultsAsync_BothFailWithSameStatus_ThrowsInvalidOperationWithBothMessages()
+    public async Task ListVaultsAsync_BothFailWithRequestFailedException_ThrowsRequestFailedException()
     {
-        // NEW-1: when both backends fail with the same HTTP status, the two services can
-        // carry meaningfully different ErrorCodes / messages, so neither side is dropped -
-        // both inner messages are folded into the resulting InvalidOperationException's
-        // message and the RSV exception is preserved as InnerException for diagnostics.
+        // NEW-1: both inner messages are preserved in the combined message.
+        // NEW-5: when both inners are RequestFailedException, the wrapper itself is a
+        // RequestFailedException so the command-layer error mapper classifies the failure
+        // as an Azure service error (with the original HTTP status code) rather than as
+        // an MCP-side bug.
         _rsvOps.ListVaultsAsync("22222222-2222-2222-2222-222222222222", null, null, Arg.Any<CancellationToken>())
             .ThrowsAsync(new RequestFailedException(403, "RSV error"));
         _dppOps.ListVaultsAsync("22222222-2222-2222-2222-222222222222", null, null, Arg.Any<CancellationToken>())
             .ThrowsAsync(new RequestFailedException(403, "DPP error"));
 
-        var ex = await Assert.ThrowsAsync<InvalidOperationException>(() =>
+        var ex = await Assert.ThrowsAsync<RequestFailedException>(() =>
             _service.ListVaultsAsync("22222222-2222-2222-2222-222222222222", null, null, null, null, CancellationToken.None));
 
+        Assert.Equal(403, ex.Status);
         Assert.Contains("RSV error", ex.Message);
         Assert.Contains("DPP error", ex.Message);
         Assert.IsType<RequestFailedException>(ex.InnerException);
-        Assert.Equal(403, ((RequestFailedException)ex.InnerException!).Status);
+    }
+
+    [Fact]
+    public async Task ListVaultsAsync_BothFailWithRequestFailedException_PrefersNonZeroStatus()
+    {
+        // NEW-5: when the RSV RequestFailedException carries a 0 status (e.g. transport-level
+        // failure with no HTTP status), prefer the DPP status so the user still sees a
+        // useful HTTP code.
+        _rsvOps.ListVaultsAsync("22222222-2222-2222-2222-222222222222", null, null, Arg.Any<CancellationToken>())
+            .ThrowsAsync(new RequestFailedException(0, "RSV transport error"));
+        _dppOps.ListVaultsAsync("22222222-2222-2222-2222-222222222222", null, null, Arg.Any<CancellationToken>())
+            .ThrowsAsync(new RequestFailedException(503, "DPP throttled"));
+
+        var ex = await Assert.ThrowsAsync<RequestFailedException>(() =>
+            _service.ListVaultsAsync("22222222-2222-2222-2222-222222222222", null, null, null, null, CancellationToken.None));
+
+        Assert.Equal(503, ex.Status);
+    }
+
+    [Fact]
+    public async Task ListVaultsAsync_BothFailWithRequestFailedException_StatusAndErrorCodePairedFromSameSource()
+    {
+        // NEW-5: Status and ErrorCode must come from the same exception so callers
+        // never see a mismatched (Status, ErrorCode) pair. Here RSV has Status=0 with
+        // its own ErrorCode; the wrapper should take both fields from DPP (the source
+        // of the non-zero Status).
+        _rsvOps.ListVaultsAsync("22222222-2222-2222-2222-222222222222", null, null, Arg.Any<CancellationToken>())
+            .ThrowsAsync(new RequestFailedException(0, "RSV transport error", "RsvOnlyCode", null));
+        _dppOps.ListVaultsAsync("22222222-2222-2222-2222-222222222222", null, null, Arg.Any<CancellationToken>())
+            .ThrowsAsync(new RequestFailedException(503, "DPP throttled", "DppThrottled", null));
+
+        var ex = await Assert.ThrowsAsync<RequestFailedException>(() =>
+            _service.ListVaultsAsync("22222222-2222-2222-2222-222222222222", null, null, null, null, CancellationToken.None));
+
+        Assert.Equal(503, ex.Status);
+        Assert.Equal("DppThrottled", ex.ErrorCode);
     }
 
     [Fact]