Skip to content

Commit bfb56a8

Browse files
AlexanderSherAlexanderSher
authored
Replace ClientDiagnostics-derived classes with RequestFailedDetailsParser implementations (Azure#33001)
* Replace ClientDiagnostics-derived classes with RequestFailedDetailsParser implementations * Fix merge error * Fix batch support * Add support for non-buffered responses in Storage * Add saniter to responses in FailedResponseExceptionTests * Fix one more test
1 parent b2a9005 commit bfb56a8

File tree

58 files changed

+717
-868
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

58 files changed

+717
-868
lines changed

‎sdk/communication/Azure.Communication.PhoneNumbers/src/CommunicationClientDiagnostics.cs‎

Lines changed: 0 additions & 63 deletions
This file was deleted.

‎sdk/communication/Azure.Communication.PhoneNumbers/src/CommunicationRequestFailedDetailsParser.cs‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
using System.Collections.Generic;
5+
using System.Text.Json;
6+
using Azure.Communication.PhoneNumbers;
7+
8+
namespace Azure.Core.Pipeline
9+
{
10+
internal sealed class CommunicationRequestFailedDetailsParser : RequestFailedDetailsParser
11+
{
12+
public override bool TryParse(Response response, out ResponseError error, out IDictionary<string, string> data)
13+
{
14+
if (response.ContentStream is { CanSeek: true })
15+
{
16+
var position = response.ContentStream.Position;
17+
try
18+
{
19+
response.ContentStream.Position = 0;
20+
using var document = JsonDocument.Parse(response.ContentStream);
21+
22+
if (document.RootElement.TryGetProperty("error", out var errorValue))
23+
{
24+
var communicationError = CommunicationError.DeserializeCommunicationError(errorValue);
25+
data = new Dictionary<string, string> { ["target"] = communicationError.Target };
26+
error = new ResponseError(communicationError.Code, communicationError.Message);
27+
}
28+
else
29+
{
30+
error = default;
31+
data = default;
32+
}
33+
34+
return true;
35+
}
36+
catch
37+
{ }
38+
finally
39+
{
40+
response.ContentStream.Position = position;
41+
}
42+
}
43+
44+
error = default;
45+
data = default;
46+
return false;
47+
}
48+
}
49+
}

‎sdk/communication/Azure.Communication.PhoneNumbers/src/PhoneNumbersClient.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ private PhoneNumbersClient(string endpoint, TokenCredential tokenCredential, Pho
8080
{ }
8181

8282
private PhoneNumbersClient(string endpoint, HttpPipeline httpPipeline, PhoneNumbersClientOptions options)
83-
: this(new CommunicationClientDiagnostics(options), httpPipeline, endpoint, options.Version)
83+
: this(new ClientDiagnostics(options), httpPipeline, endpoint, options.Version)
8484
{ }
8585

8686
/// <summary> Initializes a new instance of PhoneNumbersClient. </summary>

‎sdk/communication/Azure.Communication.PhoneNumbers/src/PhoneNumbersClientOptionsExtensions.cs‎

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,23 +14,44 @@ internal static class PhoneNumbersClientOptionsExtensions
1414
{
1515
public static HttpPipeline BuildPhoneNumbersHttpPipeline(this ClientOptions options, ConnectionString connectionString)
1616
{
17-
var authPolicy = new HMACAuthenticationPolicy(new AzureKeyCredential(connectionString.GetRequired("accesskey")));
18-
var msUserAgentPolicy = new MSUserAgentPolicy();
19-
return HttpPipelineBuilder.Build(options, authPolicy, msUserAgentPolicy);
17+
var pipelineOptions = new HttpPipelineOptions(options)
18+
{
19+
RequestFailedDetailsParser = new CommunicationRequestFailedDetailsParser(),
20+
PerRetryPolicies =
21+
{
22+
new HMACAuthenticationPolicy(new AzureKeyCredential(connectionString.GetRequired("accesskey"))),
23+
new MSUserAgentPolicy()
24+
}
25+
};
26+
return HttpPipelineBuilder.Build(pipelineOptions);
2027
}
2128

2229
public static HttpPipeline BuildPhoneNumbersHttpPipeline(this ClientOptions options, AzureKeyCredential keyCredential)
2330
{
24-
var authPolicy = new HMACAuthenticationPolicy(keyCredential);
25-
var msUserAgentPolicy = new MSUserAgentPolicy();
26-
return HttpPipelineBuilder.Build(options, authPolicy, msUserAgentPolicy);
31+
var pipelineOptions = new HttpPipelineOptions(options)
32+
{
33+
RequestFailedDetailsParser = new CommunicationRequestFailedDetailsParser(),
34+
PerRetryPolicies =
35+
{
36+
new HMACAuthenticationPolicy(keyCredential),
37+
new MSUserAgentPolicy()
38+
}
39+
};
40+
return HttpPipelineBuilder.Build(pipelineOptions);
2741
}
2842

2943
public static HttpPipeline BuildPhoneNumbersHttpPipeline(this ClientOptions options, TokenCredential tokenCredential)
3044
{
31-
var authPolicy = new BearerTokenAuthenticationPolicy(tokenCredential, "https://communication.azure.com//.default");
32-
var msUserAgentPolicy = new MSUserAgentPolicy();
33-
return HttpPipelineBuilder.Build(options, authPolicy, msUserAgentPolicy);
45+
var pipelineOptions = new HttpPipelineOptions(options)
46+
{
47+
RequestFailedDetailsParser = new CommunicationRequestFailedDetailsParser(),
48+
PerRetryPolicies =
49+
{
50+
new BearerTokenAuthenticationPolicy(tokenCredential, "https://communication.azure.com//.default"),
51+
new MSUserAgentPolicy()
52+
}
53+
};
54+
return HttpPipelineBuilder.Build(pipelineOptions);
3455
}
3556
}
3657
}

‎sdk/core/Azure.Core/src/RequestFailedException.cs‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -168,16 +168,12 @@ internal static (string FormattedError, string? ErrorCode, IDictionary<string, s
168168
}
169169
}
170170

171-
if (response.ContentStream is MemoryStream)
171+
if (response.ContentStream is MemoryStream && ContentTypeUtilities.TryGetTextEncoding(response.Headers.ContentType, out Encoding _))
172172
{
173-
string content = response.Content.ToString();
174-
if (content.Length > 0)
175-
{
176-
messageBuilder
177-
.AppendLine()
178-
.AppendLine("Content:")
179-
.AppendLine(content);
180-
}
173+
messageBuilder
174+
.AppendLine()
175+
.AppendLine("Content:")
176+
.AppendLine(response.Content.ToString());
181177
}
182178

183179
messageBuilder

‎sdk/core/Azure.Core/src/Shared/ClientDiagnostics.cs‎

Lines changed: 11 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -65,21 +65,6 @@ internal static HttpMessageSanitizer CreateMessageSanitizer(DiagnosticsOptions d
6565
diagnostics.LoggedHeaderNames.ToArray());
6666
}
6767

68-
/// <summary>
69-
/// Partial method that can optionally be defined to extract the error
70-
/// message, code, and details in a service specific manner.
71-
/// </summary>
72-
/// <param name="content">The error content.</param>
73-
/// <param name="responseHeaders">The response headers.</param>
74-
/// <param name="additionalInfo">Additional error details.</param>
75-
protected virtual ResponseError? ExtractFailureContent(
76-
string? content,
77-
ResponseHeaders responseHeaders,
78-
ref IDictionary<string, string>? additionalInfo)
79-
{
80-
return ExtractAzureErrorContent(content);
81-
}
82-
8368
internal static ResponseError? ExtractAzureErrorContent(string? content)
8469
{
8570
try
@@ -101,16 +86,23 @@ internal static HttpMessageSanitizer CreateMessageSanitizer(DiagnosticsOptions d
10186

10287
public async ValueTask<RequestFailedException> CreateRequestFailedExceptionAsync(Response response, ResponseError? error = null, IDictionary<string, string>? additionalInfo = null, Exception? innerException = null)
10388
{
89+
if (GetType() == typeof(ClientDiagnostics) && error is null && additionalInfo is null)
90+
{
91+
return new RequestFailedException(response, innerException);
92+
}
93+
10494
var content = await ReadContentAsync(response, true).ConfigureAwait(false);
105-
error ??= ExtractFailureContent(content, response.Headers, ref additionalInfo);
10695
return CreateRequestFailedExceptionWithContent(response, error, content, additionalInfo, innerException);
10796
}
10897

10998
public RequestFailedException CreateRequestFailedException(Response response, ResponseError? error = null, IDictionary<string, string>? additionalInfo = null, Exception? innerException = null)
11099
{
111-
string? content = ReadContentAsync(response, false).EnsureCompleted();
112-
error ??= ExtractFailureContent(content, response.Headers, ref additionalInfo);
100+
if (GetType() == typeof(ClientDiagnostics) && error is null && additionalInfo is null)
101+
{
102+
return new RequestFailedException(response, innerException);
103+
}
113104

105+
string? content = ReadContentAsync(response, false).EnsureCompleted();
114106
return CreateRequestFailedExceptionWithContent(response, error, content, additionalInfo, innerException);
115107
}
116108

@@ -121,6 +113,7 @@ private RequestFailedException CreateRequestFailedExceptionWithContent(
121113
IDictionary<string, string>? additionalInfo = null,
122114
Exception? innerException = null)
123115
{
116+
error ??= ExtractAzureErrorContent(content);
124117
var formatMessage = CreateRequestFailedMessageWithContent(response, error, content, additionalInfo, _sanitizer);
125118
var exception = new RequestFailedException(response.Status, formatMessage, error?.Code, innerException);
126119

‎sdk/core/Azure.Core/tests/FailedResponseExceptionTests.cs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ public async Task FormatsResponse()
3636
var response = new MockResponse(210, "Reason");
3737
response.AddHeader(new HttpHeader("Custom-Header", "Value"));
3838
response.AddHeader(new HttpHeader("x-ms-requestId", "123"));
39+
response.Sanitizer = Sanitizer;
3940

4041
RequestFailedException exception = await ClientDiagnostics.CreateRequestFailedExceptionAsync(response);
4142
Assert.AreEqual(formattedResponse, exception.Message);
@@ -136,6 +137,7 @@ public async Task FormatsResponseContentForTextContentTypes()
136137
response.AddHeader(new HttpHeader("Content-Type", "text/json"));
137138
response.AddHeader(new HttpHeader("x-ms-requestId", "123"));
138139
response.SetContent("{\"errorCode\": 1}");
140+
response.Sanitizer = Sanitizer;
139141

140142
RequestFailedException exception = await ClientDiagnostics.CreateRequestFailedExceptionAsync(response);
141143
Assert.AreEqual(formattedResponse, exception.Message);
@@ -154,6 +156,7 @@ public async Task DoesntFormatsResponseContentForNonTextContentTypes()
154156
var response = new MockResponse(210, "Reason");
155157
response.AddHeader(new HttpHeader("Content-Type", "binary"));
156158
response.SetContent("{\"errorCode\": 1}");
159+
response.Sanitizer = Sanitizer;
157160

158161
RequestFailedException exception = await ClientDiagnostics.CreateRequestFailedExceptionAsync(response);
159162
Assert.AreEqual(formattedResponse, exception.Message);
@@ -223,6 +226,7 @@ public async Task IncludesInnerException()
223226
var response = new MockResponse(210, "Reason");
224227
response.AddHeader(new HttpHeader("Custom-Header", "Value"));
225228
response.AddHeader(new HttpHeader("x-ms-requestId", "123"));
229+
response.Sanitizer = Sanitizer;
226230

227231
var innerException = new Exception();
228232
RequestFailedException exception = await ClientDiagnostics.CreateRequestFailedExceptionAsync(response, innerException: innerException);
@@ -263,6 +267,7 @@ public async Task ParsesJsonErrors()
263267
var response = new MockResponse(210, "Reason");
264268
response.SetContent("{ \"error\": { \"code\":\"StatusCode\", \"message\":\"Custom message\" }}");
265269
response.AddHeader(new HttpHeader("Content-Type", "text/json"));
270+
response.Sanitizer = Sanitizer;
266271

267272
RequestFailedException exception = await ClientDiagnostics.CreateRequestFailedExceptionAsync(response);
268273
Assert.AreEqual(formattedResponse, exception.Message);
@@ -358,6 +363,7 @@ public async Task IgnoresInvalidJsonErrors()
358363
var response = new MockResponse(210, "Reason");
359364
response.SetContent("{ \"error\": { \"code\":\"StatusCode\"");
360365
response.AddHeader(new HttpHeader("Content-Type", "text/json"));
366+
response.Sanitizer = Sanitizer;
361367

362368
RequestFailedException exception = await ClientDiagnostics.CreateRequestFailedExceptionAsync(response);
363369
Assert.AreEqual(formattedResponse, exception.Message);
@@ -379,6 +385,7 @@ public async Task IgnoresNonStandardJson()
379385
var response = new MockResponse(210, "Reason");
380386
response.SetContent("{ \"code\":\"StatusCode\" }");
381387
response.AddHeader(new HttpHeader("Content-Type", "text/json"));
388+
response.Sanitizer = Sanitizer;
382389

383390
RequestFailedException exception = await ClientDiagnostics.CreateRequestFailedExceptionAsync(response);
384391
Assert.AreEqual(formattedResponse, exception.Message);

‎sdk/servicebus/Azure.Messaging.ServiceBus/src/Administration/ServiceBusAdministrationClient.cs‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,11 @@ public ServiceBusAdministrationClient(
9494
var sharedCredential = new SharedAccessCredential(sharedAccessSignature);
9595
var tokenCredential = new ServiceBusTokenCredential(sharedCredential);
9696

97-
HttpPipeline pipeline = HttpPipelineBuilder.Build(options);
98-
_clientDiagnostics = new ServiceBusClientDiagnostics(options);
97+
var pipeline = HttpPipelineBuilder.Build(new HttpPipelineOptions(options)
98+
{
99+
RequestFailedDetailsParser = new ServiceBusRequestFailedDetailsParser()
100+
});
101+
_clientDiagnostics = new ClientDiagnostics(options);
99102

100103
_httpRequestAndResponse = new HttpRequestAndResponse(
101104
pipeline,

‎sdk/servicebus/Azure.Messaging.ServiceBus/src/Administration/ServiceBusClientDiagnostics.cs‎

Lines changed: 0 additions & 53 deletions
This file was deleted.

0 commit comments

Comments
 (0)