Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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`."
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,17 @@ 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. Prefer the first non-zero status.
var status = rsvRfe.Status != 0 ? rsvRfe.Status : dppRfe.Status;
var errorCode = rsvRfe.ErrorCode ?? dppRfe.ErrorCode;
return new RequestFailedException(status, combinedMessage, errorCode, rsvRfe);
Comment thread
shrja-ms marked this conversation as resolved.
Outdated
}

return new InvalidOperationException(combinedMessage, rsvInner);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -441,9 +455,33 @@ 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);
try
{
policies.Add(MapToPolicyInfo(policy.Data));
while (true)
{
try
{
if (!await enumerator.MoveNextAsync())
{
break;
}

policies.Add(MapToPolicyInfo(enumerator.Current.Data));
}
catch (FormatException)
{
// The Azure SDK may throw FormatException when deserializing policies with
// non-standard ISO 8601 retention/duration fields (XmlConvert.ToTimeSpan
// limitation in DataProtectionBackupAbsoluteDeleteSetting). Return the
// policies collected so far rather than failing the entire list.
break;
}
}
Comment thread
shrja-ms marked this conversation as resolved.
}
finally
{
await enumerator.DisposeAsync();
}

return policies;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,24 +228,42 @@ 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]
Expand Down
Loading