Skip to content

Commit a99b36b

Browse files
authored
Make Azure Backup recovery point listing and policy update resilient to SDK deserialization failures (#3861)
1 parent d20b3d9 commit a99b36b

4 files changed

Lines changed: 299 additions & 6 deletions

File tree

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
changes:
2+
- section: Bugs Fixed
3+
description: >-
4+
Made Azure Backup recovery point listing resilient to malformed Azure Files recovery points. The
5+
RecoveryServicesBackup SDK could throw a UriFormatException while deserializing a single file-share
6+
recovery point, failing the entire 'azurebackup recoverypoint get' call; the service now skips the
7+
offending item and returns the remaining valid recovery points.
8+
- section: Bugs Fixed
9+
description: >-
10+
Replaced the opaque NullReferenceException surfaced by 'azurebackup policy update' with an actionable
11+
error. The SDK can fail to materialize the updated policy from the service response for some IaaS VM
12+
policy shapes even though the change was submitted; the tool now returns a clear message directing the
13+
user to verify the policy with 'azurebackup policy get'.

‎tools/Azure.Mcp.Tools.AzureBackup/src/Services/RsvBackupOperations.cs‎

Lines changed: 88 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -510,13 +510,69 @@ public async Task<List<RecoveryPointInfo>> ListRecoveryPointsAsync(
510510
var itemResource = armClient.GetBackupProtectedItemResource(itemId);
511511
var collection = itemResource.GetBackupRecoveryPoints();
512512

513-
var points = new List<RecoveryPointInfo>();
514-
await foreach (var rp in collection.GetAllAsync(cancellationToken: cancellationToken))
513+
// The RecoveryServicesBackup SDK can throw UriFormatException/FormatException while
514+
// deserializing individual Azure Files (FileShareRecoveryPoint) recovery points whose
515+
// payload contains a malformed URI or duration field. Enumerate defensively so a single
516+
// malformed recovery point does not fail the entire list.
517+
return await EnumerateTolerantAsync(
518+
collection.GetAllAsync(cancellationToken: cancellationToken),
519+
static rp => MapToRecoveryPointInfo(rp.Data),
520+
cancellationToken);
521+
}
522+
523+
/// <summary>
524+
/// Enumerates an async pageable, projecting each element with <paramref name="selector"/> while
525+
/// tolerating Azure SDK deserialization failures. The RecoveryServicesBackup SDK can throw
526+
/// <see cref="FormatException"/> (including its <see cref="UriFormatException"/> subtype) when a
527+
/// single item's payload contains a malformed duration or URI - for example an Azure Files
528+
/// <c>FileShareRecoveryPoint</c>. Offending items are skipped so the remaining valid items are
529+
/// still returned. If the enumerator becomes unusable (a page-level failure where
530+
/// <see cref="IAsyncEnumerator{T}.MoveNextAsync"/> keeps throwing), enumeration stops after
531+
/// <paramref name="maxConsecutiveFailures"/> consecutive errors to avoid an infinite loop.
532+
/// </summary>
533+
internal static async Task<List<TResult>> EnumerateTolerantAsync<TSource, TResult>(
534+
IAsyncEnumerable<TSource> source,
535+
Func<TSource, TResult> selector,
536+
CancellationToken cancellationToken,
537+
int maxConsecutiveFailures = 3)
538+
{
539+
ArgumentNullException.ThrowIfNull(source);
540+
ArgumentNullException.ThrowIfNull(selector);
541+
542+
var results = new List<TResult>();
543+
var enumerator = source.GetAsyncEnumerator(cancellationToken);
544+
var consecutiveFailures = 0;
545+
try
546+
{
547+
while (true)
548+
{
549+
try
550+
{
551+
if (!await enumerator.MoveNextAsync())
552+
{
553+
break;
554+
}
555+
556+
results.Add(selector(enumerator.Current));
557+
consecutiveFailures = 0;
558+
}
559+
catch (FormatException)
560+
{
561+
// UriFormatException derives from FormatException; both indicate a single item
562+
// (or page) the SDK cannot deserialize. Skip and continue.
563+
if (++consecutiveFailures >= maxConsecutiveFailures)
564+
{
565+
break;
566+
}
567+
}
568+
}
569+
}
570+
finally
515571
{
516-
points.Add(MapToRecoveryPointInfo(rp.Data));
572+
await enumerator.DisposeAsync();
517573
}
518574

519-
return points;
575+
return results;
520576
}
521577

522578
/// <summary>
@@ -792,12 +848,38 @@ public async Task<OperationResult> UpdatePolicyAsync(
792848
UpdatePolicyScheduleAndRetention(policyProperties, newScheduleTime, newRetentionDays);
793849
}
794850

795-
var operation = await policyCollection.CreateOrUpdateAsync(WaitUntil.Started, policyName, policyData, cancellationToken);
796-
await WaitForLroCompletionAsync(operation, cancellationToken);
851+
// The RecoveryServicesBackup SDK can throw a NullReferenceException from the
852+
// BackupProtectionPolicyResource constructor while materializing the CreateOrUpdate response
853+
// for certain IaaS VM policy shapes (the returned policy data omits the resource id). The PUT
854+
// itself is accepted by the service, so translate the opaque failure into an actionable error
855+
// that points the caller at a verification step instead of surfacing a bare 500.
856+
try
857+
{
858+
var operation = await policyCollection.CreateOrUpdateAsync(WaitUntil.Started, policyName, policyData, cancellationToken);
859+
await WaitForLroCompletionAsync(operation, cancellationToken);
860+
}
861+
catch (NullReferenceException)
862+
{
863+
throw CreatePolicyMaterializationError(policyName!, vaultName);
864+
}
797865

798866
return new OperationResult("Succeeded", null, $"Policy '{policyName}' updated in vault '{vaultName}'.");
799867
}
800868

869+
/// <summary>
870+
/// Builds the error surfaced when the Azure SDK cannot materialize the updated policy from the
871+
/// service response (a known <see cref="NullReferenceException"/> in
872+
/// <c>BackupProtectionPolicyResource</c>'s constructor for some IaaS VM policy shapes). The update
873+
/// request is accepted server-side, so the message is explicit that the change may have applied
874+
/// and directs the caller to verify the current policy state.
875+
/// </summary>
876+
internal static RequestFailedException CreatePolicyMaterializationError(string policyName, string vaultName) =>
877+
new(
878+
(int)HttpStatusCode.InternalServerError,
879+
$"The update for policy '{policyName}' in vault '{vaultName}' was submitted, but the Azure SDK could not " +
880+
"deserialize the updated policy from the service response (a known SDK limitation for some IaaS VM policy shapes). " +
881+
"The change may have been applied - verify the current policy with 'azurebackup policy get'.");
882+
801883
/// <summary>
802884
/// Applies the caller-supplied IaasVM extended flags on top of the existing policy in place.
803885
/// Semantics: TimeZone is overlaid when supplied. Schedule (SimpleSchedulePolicy) is replaced when
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT License.
3+
4+
using Azure.Mcp.Tools.AzureBackup.Services;
5+
using Xunit;
6+
7+
namespace Azure.Mcp.Tools.AzureBackup.Tests.Services;
8+
9+
/// <summary>
10+
/// Unit tests for the internal <see cref="RsvBackupOperations.EnumerateTolerantAsync{TSource, TResult}"/>
11+
/// helper that backs defensive recovery-point enumeration. The RecoveryServicesBackup SDK can throw
12+
/// <see cref="UriFormatException"/>/<see cref="FormatException"/> while deserializing a single
13+
/// Azure Files (<c>FileShareRecoveryPoint</c>) item; the helper must skip the offending item and
14+
/// return the remaining valid items rather than failing the entire list.
15+
/// </summary>
16+
public class RsvBackupOperationsEnumerateTolerantTests
17+
{
18+
/// <summary>
19+
/// An <see cref="IAsyncEnumerable{T}"/> whose per-item factories can throw to simulate the SDK
20+
/// failing to deserialize an individual element during <see cref="IAsyncEnumerator{T}.MoveNextAsync"/>.
21+
/// The enumerator advances its index before invoking the factory, so a throwing item still lets
22+
/// the next <c>MoveNextAsync</c> yield the following element - matching the SDK's item-level
23+
/// deserialization failure behavior.
24+
/// </summary>
25+
private sealed class FaultyAsyncEnumerable(IReadOnlyList<Func<int>> factories) : IAsyncEnumerable<int>
26+
{
27+
public bool Disposed { get; private set; }
28+
29+
public IAsyncEnumerator<int> GetAsyncEnumerator(CancellationToken cancellationToken = default)
30+
=> new Enumerator(this, factories);
31+
32+
private sealed class Enumerator(FaultyAsyncEnumerable owner, IReadOnlyList<Func<int>> factories) : IAsyncEnumerator<int>
33+
{
34+
private int _index = -1;
35+
36+
public int Current { get; private set; }
37+
38+
public ValueTask<bool> MoveNextAsync()
39+
{
40+
_index++;
41+
if (_index >= factories.Count)
42+
{
43+
return ValueTask.FromResult(false);
44+
}
45+
46+
Current = factories[_index]();
47+
return ValueTask.FromResult(true);
48+
}
49+
50+
public ValueTask DisposeAsync()
51+
{
52+
owner.Disposed = true;
53+
return ValueTask.CompletedTask;
54+
}
55+
}
56+
}
57+
58+
private static Func<int> Good(int value) => () => value;
59+
private static Func<int> ThrowsFormat() => () => throw new FormatException("bad duration");
60+
private static Func<int> ThrowsUri() => () => throw new UriFormatException("bad uri");
61+
62+
[Fact]
63+
public async Task EnumerateTolerantAsync_AllValid_ReturnsAllItems()
64+
{
65+
var source = new FaultyAsyncEnumerable([Good(1), Good(2), Good(3)]);
66+
67+
var result = await RsvBackupOperations.EnumerateTolerantAsync(source, static x => x, CancellationToken.None);
68+
69+
Assert.Equal([1, 2, 3], result);
70+
Assert.True(source.Disposed);
71+
}
72+
73+
[Fact]
74+
public async Task EnumerateTolerantAsync_SkipsFormatExceptionItem_ReturnsValidItems()
75+
{
76+
var source = new FaultyAsyncEnumerable([Good(1), ThrowsFormat(), Good(3)]);
77+
78+
var result = await RsvBackupOperations.EnumerateTolerantAsync(source, static x => x, CancellationToken.None);
79+
80+
Assert.Equal([1, 3], result);
81+
}
82+
83+
[Fact]
84+
public async Task EnumerateTolerantAsync_SkipsUriFormatExceptionItem_ReturnsValidItems()
85+
{
86+
// UriFormatException derives from FormatException - the NEW-5 Azure Files case.
87+
var source = new FaultyAsyncEnumerable([Good(10), ThrowsUri(), Good(20)]);
88+
89+
var result = await RsvBackupOperations.EnumerateTolerantAsync(source, static x => x, CancellationToken.None);
90+
91+
Assert.Equal([10, 20], result);
92+
}
93+
94+
[Fact]
95+
public async Task EnumerateTolerantAsync_SelectorThrowsFormatException_SkipsThatItem()
96+
{
97+
var source = new FaultyAsyncEnumerable([Good(1), Good(2), Good(3)]);
98+
99+
var result = await RsvBackupOperations.EnumerateTolerantAsync(
100+
source,
101+
static x => x == 2 ? throw new FormatException("bad projection") : x,
102+
CancellationToken.None);
103+
104+
Assert.Equal([1, 3], result);
105+
}
106+
107+
[Fact]
108+
public async Task EnumerateTolerantAsync_StopsAfterMaxConsecutiveFailures()
109+
{
110+
// first valid item, then three consecutive failures hit the cap and enumeration stops
111+
// before the trailing valid item is ever read.
112+
var source = new FaultyAsyncEnumerable(
113+
[Good(1), ThrowsFormat(), ThrowsFormat(), ThrowsFormat(), Good(99)]);
114+
115+
var result = await RsvBackupOperations.EnumerateTolerantAsync(
116+
source, static x => x, CancellationToken.None, maxConsecutiveFailures: 3);
117+
118+
Assert.Equal([1], result);
119+
}
120+
121+
[Fact]
122+
public async Task EnumerateTolerantAsync_ConsecutiveFailureCountResetsAfterSuccess()
123+
{
124+
// failures separated by successes never reach the cap, so every valid item is returned.
125+
var source = new FaultyAsyncEnumerable(
126+
[ThrowsFormat(), Good(1), ThrowsFormat(), Good(2), ThrowsFormat(), Good(3)]);
127+
128+
var result = await RsvBackupOperations.EnumerateTolerantAsync(
129+
source, static x => x, CancellationToken.None, maxConsecutiveFailures: 3);
130+
131+
Assert.Equal([1, 2, 3], result);
132+
}
133+
134+
[Fact]
135+
public async Task EnumerateTolerantAsync_EmptySource_ReturnsEmpty()
136+
{
137+
var source = new FaultyAsyncEnumerable([]);
138+
139+
var result = await RsvBackupOperations.EnumerateTolerantAsync(source, static x => x, CancellationToken.None);
140+
141+
Assert.Empty(result);
142+
Assert.True(source.Disposed);
143+
}
144+
145+
[Fact]
146+
public async Task EnumerateTolerantAsync_NullSource_Throws()
147+
{
148+
await Assert.ThrowsAsync<ArgumentNullException>(() =>
149+
RsvBackupOperations.EnumerateTolerantAsync<int, int>(null!, static x => x, CancellationToken.None));
150+
}
151+
152+
[Fact]
153+
public async Task EnumerateTolerantAsync_NullSelector_Throws()
154+
{
155+
var source = new FaultyAsyncEnumerable([Good(1)]);
156+
157+
await Assert.ThrowsAsync<ArgumentNullException>(() =>
158+
RsvBackupOperations.EnumerateTolerantAsync<int, int>(source, null!, CancellationToken.None));
159+
}
160+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT License.
3+
4+
using System.Net;
5+
using Azure.Mcp.Tools.AzureBackup.Services;
6+
using Xunit;
7+
8+
namespace Azure.Mcp.Tools.AzureBackup.Tests.Services;
9+
10+
/// <summary>
11+
/// Unit tests for the internal <see cref="RsvBackupOperations.CreatePolicyMaterializationError"/>
12+
/// helper. When updating an RSV protection policy, the RecoveryServicesBackup SDK can throw a
13+
/// <see cref="NullReferenceException"/> from the <c>BackupProtectionPolicyResource</c> constructor
14+
/// while materializing the CreateOrUpdate response for some IaaS VM policy shapes. The service
15+
/// translates that opaque failure into an actionable <see cref="RequestFailedException"/>; these
16+
/// tests pin the status code and the user-facing message contract.
17+
/// </summary>
18+
public class RsvBackupOperationsPolicyUpdateErrorTests
19+
{
20+
[Fact]
21+
public void CreatePolicyMaterializationError_UsesInternalServerErrorStatus()
22+
{
23+
var ex = RsvBackupOperations.CreatePolicyMaterializationError("daily-policy", "my-vault");
24+
25+
Assert.Equal((int)HttpStatusCode.InternalServerError, ex.Status);
26+
}
27+
28+
[Fact]
29+
public void CreatePolicyMaterializationError_MessageIncludesPolicyAndVaultAndVerificationStep()
30+
{
31+
var ex = RsvBackupOperations.CreatePolicyMaterializationError("daily-policy", "my-vault");
32+
33+
Assert.Contains("daily-policy", ex.Message);
34+
Assert.Contains("my-vault", ex.Message);
35+
// Directs the caller to confirm whether the submitted change applied.
36+
Assert.Contains("policy get", ex.Message);
37+
}
38+
}

0 commit comments

Comments
 (0)