Skip to content

Commit b50c5be

Browse files
authored
Forward x-ms-client-request-id and use AzPS exception types (#452)
- Forward x-ms-client-request-id header to acquirePolicyToken API call for log correlation (Celina's feedback) - Replace InvalidOperationException with AzPSInvalidOperationException and AzPSCloudException for proper Azure PowerShell error handling patterns - Add ErrorKind categorization (ServiceError, UserError) and desensitized messages for telemetry - Improve error messages with request URI context - Add E2E test for x-ms-client-request-id forwarding - Update exception assertions in existing tests
1 parent f38d84a commit b50c5be

2 files changed

Lines changed: 71 additions & 12 deletions

File tree

src/Common.Test/AcquirePolicyTokenHandlerTests.cs

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
// limitations under the License.
1313
// ----------------------------------------------------------------------------------
1414

15+
using Microsoft.Azure.Commands.Common;
16+
using Microsoft.Azure.Commands.Common.Exceptions;
1517
using Microsoft.WindowsAzure.Commands.Common;
1618
using Newtonsoft.Json;
1719
using Newtonsoft.Json.Linq;
@@ -841,6 +843,28 @@ public async Task E2E_TokenRequest_ForwardsAuthHeaders()
841843
Assert.True(capturedTokenRequest.Headers.Contains("x-ms-authorization-auxiliary"));
842844
}
843845

846+
// ----- Token request forwards x-ms-client-request-id for log correlation -----
847+
848+
[Fact]
849+
public async Task E2E_TokenRequest_ForwardsClientRequestId()
850+
{
851+
HttpRequestMessage capturedTokenRequest = null;
852+
var tokenClient = CreateMockTokenServer(req => capturedTokenRequest = req);
853+
var (handler, client) = CreateTestPipeline(tokenClient);
854+
855+
var request = new HttpRequestMessage(HttpMethod.Delete,
856+
$"{BaseUrl}/subscriptions/{TestSubId}/resourceGroups/rg/providers/Microsoft.Storage/storageAccounts/sa1?api-version=2024-01-01");
857+
request.Headers.Authorization = new System.Net.Http.Headers.AuthenticationHeaderValue("Bearer", "test-token");
858+
request.Headers.Add("x-ms-client-request-id", "correlation-id-abc-123");
859+
860+
await client.SendAsync(request);
861+
862+
Assert.True(capturedTokenRequest.Headers.Contains("x-ms-client-request-id"),
863+
"Token request must forward x-ms-client-request-id for log correlation.");
864+
Assert.Equal("correlation-id-abc-123",
865+
capturedTokenRequest.Headers.GetValues("x-ms-client-request-id").First());
866+
}
867+
844868
// ----- Token API failure returns clear error -----
845869

846870
[Fact]
@@ -849,12 +873,13 @@ public async Task E2E_TokenApiFails_ThrowsWithMessage()
849873
var tokenClient = CreateFailingTokenServer(HttpStatusCode.Forbidden, "Access denied");
850874
var (handler, client) = CreateTestPipeline(tokenClient);
851875

852-
var ex = await Assert.ThrowsAsync<InvalidOperationException>(() =>
876+
var ex = await Assert.ThrowsAsync<AzPSCloudException>(() =>
853877
client.SendAsync(new HttpRequestMessage(HttpMethod.Delete,
854878
$"{BaseUrl}/subscriptions/{TestSubId}/resourceGroups/rg/providers/Microsoft.Storage/storageAccounts/sa1?api-version=2024-01-01")));
855879

856-
Assert.Contains("Failed to acquire policy token", ex.Message);
880+
Assert.Contains("Policy token acquisition failed", ex.Message);
857881
Assert.Contains("403", ex.Message);
882+
Assert.Equal(ErrorKind.ServiceError, ex.ErrorKind);
858883
}
859884

860885
// ----- Token API returns 200 but no token field -----
@@ -873,12 +898,13 @@ public async Task E2E_TokenApiReturns200ButNoToken_ThrowsWithResponse()
873898
var tokenClient = new HttpClient(handler);
874899
var (pipelineHandler, client) = CreateTestPipeline(tokenClient);
875900

876-
var ex = await Assert.ThrowsAsync<InvalidOperationException>(() =>
901+
var ex = await Assert.ThrowsAsync<AzPSCloudException>(() =>
877902
client.SendAsync(new HttpRequestMessage(HttpMethod.Delete,
878903
$"{BaseUrl}/subscriptions/{TestSubId}/resourceGroups/rg/providers/Microsoft.Storage/storageAccounts/sa1?api-version=2024-01-01")));
879904

880-
Assert.Contains("No token returned", ex.Message);
905+
Assert.Contains("no token was returned", ex.Message);
881906
Assert.Contains("validator error", ex.Message);
907+
Assert.Equal(ErrorKind.ServiceError, ex.ErrorKind);
882908
}
883909

884910
// ----- GET request is NOT intercepted even with flag -----

src/Common/AcquirePolicyTokenHandler.cs

Lines changed: 41 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@
2121
using System.Text.RegularExpressions;
2222
using System.Threading;
2323
using System.Threading.Tasks;
24+
using Microsoft.Azure.Commands.Common;
25+
using Microsoft.Azure.Commands.Common.Exceptions;
2426
using Newtonsoft.Json;
2527
using Newtonsoft.Json.Linq;
2628

@@ -112,10 +114,22 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
112114
EnqueueDebug("No token returned (null/empty).");
113115
}
114116
}
117+
catch (AzPSInvalidOperationException)
118+
{
119+
throw;
120+
}
121+
catch (AzPSCloudException)
122+
{
123+
throw;
124+
}
115125
catch (Exception ex)
116126
{
117127
EnqueueDebug($"Exception: {ex.GetType().Name}: {ex.Message}");
118-
throw new InvalidOperationException($"Failed to acquire policy token: {ex.Message}", ex);
128+
throw new AzPSInvalidOperationException(
129+
$"Failed to acquire policy token for {request.Method} {request.RequestUri}: {ex.Message}",
130+
ErrorKind.ServiceError,
131+
ex,
132+
desensitizedMessage: "Failed to acquire policy token.");
119133
}
120134

121135
return await base.SendAsync(request, cancellationToken).ConfigureAwait(false);
@@ -126,8 +140,11 @@ private async Task<string> AcquirePolicyTokenAsync(HttpRequestMessage originalRe
126140
var subscriptionId = ExtractSubscriptionId(originalRequest.RequestUri);
127141
if (string.IsNullOrEmpty(subscriptionId))
128142
{
129-
EnqueueDebug("Failed: subscription id not found in URI.");
130-
throw new InvalidOperationException("Unable to determine subscription ID for policy token acquisition.");
143+
EnqueueDebug($"Failed: subscription id not found in URI {originalRequest.RequestUri}.");
144+
throw new AzPSInvalidOperationException(
145+
$"Unable to determine subscription ID for policy token acquisition from URI: {originalRequest.RequestUri}",
146+
ErrorKind.UserError,
147+
desensitizedMessage: "Unable to determine subscription ID for policy token acquisition.");
131148
}
132149

133150
var authority = originalRequest.RequestUri.GetLeftPart(UriPartial.Authority);
@@ -180,6 +197,10 @@ private async Task<string> AcquirePolicyTokenAsync(HttpRequestMessage originalRe
180197
{
181198
tokenRequest.Headers.TryAddWithoutValidation("x-ms-authorization-auxiliary", auxValues);
182199
}
200+
if (originalRequest.Headers.TryGetValues("x-ms-client-request-id", out var clientRequestIdValues))
201+
{
202+
tokenRequest.Headers.TryAddWithoutValidation("x-ms-client-request-id", clientRequestIdValues);
203+
}
183204

184205
var isOwnedClient = _tokenHttpClient == null;
185206
var http = _tokenHttpClient ?? new HttpClient();
@@ -198,21 +219,33 @@ private async Task<string> AcquirePolicyTokenAsync(HttpRequestMessage originalRe
198219
if (string.IsNullOrEmpty(token))
199220
{
200221
EnqueueDebug("Response OK but token missing.");
201-
throw new InvalidOperationException($"No token returned. Response:{responseContent}");
222+
throw new AzPSCloudException(
223+
$"Policy token acquisition succeeded but no token was returned. Response: {responseContent}",
224+
ErrorKind.ServiceError,
225+
desensitizedMessage: "Policy token acquisition succeeded but no token was returned.");
202226
}
203227
return token;
204228
}
205-
throw new InvalidOperationException("Empty response body when acquiring policy token.");
229+
throw new AzPSCloudException(
230+
"Policy token acquisition returned an empty response body.",
231+
ErrorKind.ServiceError,
232+
desensitizedMessage: "Policy token acquisition returned an empty response body.");
206233
}
207234
else if (response.StatusCode == HttpStatusCode.Accepted)
208235
{
209236
EnqueueDebug("202 Accepted received (async not supported).");
210-
throw new InvalidOperationException("Asynchronous policy token acquisition (202 Accepted) is not supported.");
237+
throw new AzPSCloudException(
238+
"Asynchronous policy token acquisition (202 Accepted) is not supported.",
239+
ErrorKind.ServiceError,
240+
desensitizedMessage: "Asynchronous policy token acquisition (202 Accepted) is not supported.");
211241
}
212242
else
213243
{
214-
EnqueueDebug("Non-success status; will throw.");
215-
throw new InvalidOperationException($"Policy token acquisition failed with {(int)response.StatusCode} {response.StatusCode}: {responseContent}");
244+
EnqueueDebug($"Non-success status {(int)response.StatusCode}; will throw.");
245+
throw new AzPSCloudException(
246+
$"Policy token acquisition failed with {(int)response.StatusCode} {response.StatusCode}: {responseContent}",
247+
ErrorKind.ServiceError,
248+
desensitizedMessage: $"Policy token acquisition failed with status {(int)response.StatusCode}.");
216249
}
217250
}
218251
finally

0 commit comments

Comments
 (0)