From 153dd70b7366d03ae9075654096189e1fa8c6913 Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Tue, 17 Aug 2021 13:14:33 -0700 Subject: [PATCH 1/8] move to project referencse for Core --- .../src/Azure.Analytics.Synapse.AccessControl.csproj | 6 ++++-- .../src/Azure.Analytics.Synapse.Artifacts.csproj | 4 ++++ ....Analytics.Synapse.ManagedPrivateEndpoints.csproj | 4 ++++ .../src/Azure.Analytics.Synapse.Monitoring.csproj | 4 ++++ .../src/Azure.Analytics.Synapse.Spark.csproj | 4 ++++ sdk/synapse/Azure.Analytics.Synapse.sln | 12 ++++++++++++ 6 files changed, 32 insertions(+), 2 deletions(-) diff --git a/sdk/synapse/Azure.Analytics.Synapse.AccessControl/src/Azure.Analytics.Synapse.AccessControl.csproj b/sdk/synapse/Azure.Analytics.Synapse.AccessControl/src/Azure.Analytics.Synapse.AccessControl.csproj index 9d7934071d4b..33c0f3a8cb58 100644 --- a/sdk/synapse/Azure.Analytics.Synapse.AccessControl/src/Azure.Analytics.Synapse.AccessControl.csproj +++ b/sdk/synapse/Azure.Analytics.Synapse.AccessControl/src/Azure.Analytics.Synapse.AccessControl.csproj @@ -18,8 +18,6 @@ - - @@ -38,5 +36,9 @@ + + + + diff --git a/sdk/synapse/Azure.Analytics.Synapse.Artifacts/src/Azure.Analytics.Synapse.Artifacts.csproj b/sdk/synapse/Azure.Analytics.Synapse.Artifacts/src/Azure.Analytics.Synapse.Artifacts.csproj index 70437c33a89f..1daf3a7bda92 100644 --- a/sdk/synapse/Azure.Analytics.Synapse.Artifacts/src/Azure.Analytics.Synapse.Artifacts.csproj +++ b/sdk/synapse/Azure.Analytics.Synapse.Artifacts/src/Azure.Analytics.Synapse.Artifacts.csproj @@ -37,5 +37,9 @@ + + + + diff --git a/sdk/synapse/Azure.Analytics.Synapse.ManagedPrivateEndpoints/src/Azure.Analytics.Synapse.ManagedPrivateEndpoints.csproj b/sdk/synapse/Azure.Analytics.Synapse.ManagedPrivateEndpoints/src/Azure.Analytics.Synapse.ManagedPrivateEndpoints.csproj index 1b9411ee3043..bec7c08a78ea 100644 --- a/sdk/synapse/Azure.Analytics.Synapse.ManagedPrivateEndpoints/src/Azure.Analytics.Synapse.ManagedPrivateEndpoints.csproj +++ b/sdk/synapse/Azure.Analytics.Synapse.ManagedPrivateEndpoints/src/Azure.Analytics.Synapse.ManagedPrivateEndpoints.csproj @@ -37,5 +37,9 @@ + + + + diff --git a/sdk/synapse/Azure.Analytics.Synapse.Monitoring/src/Azure.Analytics.Synapse.Monitoring.csproj b/sdk/synapse/Azure.Analytics.Synapse.Monitoring/src/Azure.Analytics.Synapse.Monitoring.csproj index a837b7ffcfe5..1e05f94afeba 100644 --- a/sdk/synapse/Azure.Analytics.Synapse.Monitoring/src/Azure.Analytics.Synapse.Monitoring.csproj +++ b/sdk/synapse/Azure.Analytics.Synapse.Monitoring/src/Azure.Analytics.Synapse.Monitoring.csproj @@ -37,5 +37,9 @@ + + + + diff --git a/sdk/synapse/Azure.Analytics.Synapse.Spark/src/Azure.Analytics.Synapse.Spark.csproj b/sdk/synapse/Azure.Analytics.Synapse.Spark/src/Azure.Analytics.Synapse.Spark.csproj index c02ac073e566..50dfccd57302 100644 --- a/sdk/synapse/Azure.Analytics.Synapse.Spark/src/Azure.Analytics.Synapse.Spark.csproj +++ b/sdk/synapse/Azure.Analytics.Synapse.Spark/src/Azure.Analytics.Synapse.Spark.csproj @@ -39,5 +39,9 @@ + + + + diff --git a/sdk/synapse/Azure.Analytics.Synapse.sln b/sdk/synapse/Azure.Analytics.Synapse.sln index 8be378f54fee..77e004e47961 100644 --- a/sdk/synapse/Azure.Analytics.Synapse.sln +++ b/sdk/synapse/Azure.Analytics.Synapse.sln @@ -29,6 +29,10 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Azure.Analytics.Synapse.Mon EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Azure.Analytics.Synapse.Monitoring.Tests", "Azure.Analytics.Synapse.Monitoring\tests\Azure.Analytics.Synapse.Monitoring.Tests.csproj", "{729E92B7-E47B-442C-836F-27B8A7806D2F}" EndProject +Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Azure.Core", "..\core\Azure.Core\src\Azure.Core.csproj", "{431E8B75-EE0F-46E9-BC1A-2BC4436E8C5D}" +EndProject +Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Azure.Core.Experimental", "..\core\Azure.Core.Experimental\src\Azure.Core.Experimental.csproj", "{8D8EB800-3496-4E0D-A2D0-F46CC2BA44DB}" +EndProject Global GlobalSection(SharedMSBuildProjectFiles) = preSolution Azure.Analytics.Synapse.Shared\tests\Azure.Analytics.Synapse.Shared.Tests.projitems*{1afa2644-a1d9-419f-b87d-9b519b673f24}*SharedItemsImports = 13 @@ -93,6 +97,14 @@ Global {729E92B7-E47B-442C-836F-27B8A7806D2F}.Debug|Any CPU.Build.0 = Debug|Any CPU {729E92B7-E47B-442C-836F-27B8A7806D2F}.Release|Any CPU.ActiveCfg = Release|Any CPU {729E92B7-E47B-442C-836F-27B8A7806D2F}.Release|Any CPU.Build.0 = Release|Any CPU + {431E8B75-EE0F-46E9-BC1A-2BC4436E8C5D}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {431E8B75-EE0F-46E9-BC1A-2BC4436E8C5D}.Debug|Any CPU.Build.0 = Debug|Any CPU + {431E8B75-EE0F-46E9-BC1A-2BC4436E8C5D}.Release|Any CPU.ActiveCfg = Release|Any CPU + {431E8B75-EE0F-46E9-BC1A-2BC4436E8C5D}.Release|Any CPU.Build.0 = Release|Any CPU + {8D8EB800-3496-4E0D-A2D0-F46CC2BA44DB}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {8D8EB800-3496-4E0D-A2D0-F46CC2BA44DB}.Debug|Any CPU.Build.0 = Debug|Any CPU + {8D8EB800-3496-4E0D-A2D0-F46CC2BA44DB}.Release|Any CPU.ActiveCfg = Release|Any CPU + {8D8EB800-3496-4E0D-A2D0-F46CC2BA44DB}.Release|Any CPU.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE From 02c0fb2b22e14a94b6f7f35296649171df473f59 Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Tue, 17 Aug 2021 16:17:20 -0700 Subject: [PATCH 2/8] refactor to put Sanitizer and Classifier on transport --- .../src/Pipeline/HttpClientTransport.cs | 14 +- .../Azure.Core/src/Pipeline/HttpPipeline.cs | 1 + .../src/Pipeline/HttpPipelineTransport.cs | 6 + sdk/core/Azure.Core/src/Response.cs | 24 +++ .../src/ResponseExceptionFactory.cs | 184 ++++++++++++++++++ .../src/Shared/ClientDiagnostics.cs | 6 +- .../src/Models/SynapseRoleAssignment.cs | 22 +++ 7 files changed, 251 insertions(+), 6 deletions(-) create mode 100644 sdk/core/Azure.Core/src/ResponseExceptionFactory.cs diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpClientTransport.cs b/sdk/core/Azure.Core/src/Pipeline/HttpClientTransport.cs index e51dd0eb5132..b5e404011a78 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpClientTransport.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpClientTransport.cs @@ -53,6 +53,10 @@ public HttpClientTransport(HttpClient client) /// public static readonly HttpClientTransport Shared = new HttpClientTransport(); + internal ClientOptions? ClientOptions { get; set; } + + internal ResponseClassifier? ResponseClassifier { get; set; } + /// public sealed override Request CreateRequest() => new PipelineRequest(); @@ -129,7 +133,7 @@ private async ValueTask ProcessAsync(HttpMessage message, bool async) throw new RequestFailedException(e.Message, e); } - message.Response = new PipelineResponse(message.Request.ClientRequestId, responseMessage, contentStream); + message.Response = new PipelineResponse(message.Request.ClientRequestId, responseMessage, contentStream, this.ClientOptions!, this.ResponseClassifier!); } private static HttpClient CreateDefaultClient() @@ -500,9 +504,11 @@ private sealed class PipelineResponse : Response private Stream? _contentStream; #pragma warning restore CA2213 - public PipelineResponse(string requestId, HttpResponseMessage responseMessage, Stream? contentStream) + public PipelineResponse(string requestId, HttpResponseMessage responseMessage, Stream? contentStream, ClientOptions options, ResponseClassifier classifier) { ClientRequestId = requestId ?? throw new ArgumentNullException(nameof(requestId)); + ExceptionFactory = new ResponseExceptionFactory(options); + ResponseClassifier = classifier; _responseMessage = responseMessage ?? throw new ArgumentNullException(nameof(responseMessage)); _contentStream = contentStream; _responseContent = _responseMessage.Content; @@ -526,6 +532,10 @@ public override Stream? ContentStream public override string ClientRequestId { get; set; } + internal override ResponseExceptionFactory ExceptionFactory { get; } + + internal override ResponseClassifier ResponseClassifier { get; } + protected internal override bool TryGetHeader(string name, [NotNullWhen(true)] out string? value) => HttpClientTransport.TryGetHeader(_responseMessage.Headers, _responseContent, name, out value); protected internal override bool TryGetHeaderValues(string name, [NotNullWhen(true)] out IEnumerable? values) => HttpClientTransport.TryGetHeader(_responseMessage.Headers, _responseContent, name, out values); diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs index c283475c0d86..1accded7ef61 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs @@ -33,6 +33,7 @@ public HttpPipeline(HttpPipelineTransport transport, HttpPipelinePolicy[]? polic policies = policies ?? Array.Empty(); var all = new HttpPipelinePolicy[policies.Length + 1]; + _transport.ResponseClassifier = ResponseClassifier; all[policies.Length] = new HttpPipelineTransportPolicy(_transport); policies.CopyTo(all, 0); diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipelineTransport.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipelineTransport.cs index 42c9c8af5bfa..9d898027a9a6 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipelineTransport.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipelineTransport.cs @@ -28,5 +28,11 @@ public abstract class HttpPipelineTransport /// /// public abstract Request CreateRequest(); + + + internal ClientOptions? ClientOptions { get; set; } + + internal ResponseClassifier? ResponseClassifier { get; set; } + } } diff --git a/sdk/core/Azure.Core/src/Response.cs b/sdk/core/Azure.Core/src/Response.cs index af2c3596a955..3af16ccc82fb 100644 --- a/sdk/core/Azure.Core/src/Response.cs +++ b/sdk/core/Azure.Core/src/Response.cs @@ -5,7 +5,9 @@ using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; using System.IO; +using System.Threading.Tasks; using Azure.Core; +using Azure.Core.Pipeline; namespace Azure { @@ -111,6 +113,28 @@ public virtual BinaryData Content /// The enumerating in the response. protected internal abstract IEnumerable EnumerateHeaders(); + internal abstract ResponseExceptionFactory ExceptionFactory { get; } + + internal abstract ResponseClassifier ResponseClassifier { get; } + + /// + /// Throw a RequestFailedException appropriate to the Response. + /// + public void Throw() + { + throw this.ExceptionFactory.CreateRequestFailedException(this); + //throw new RequestFailedException(""); + } + + /// + /// Throw a RequestFailedException appropriate to the Response. + /// + public async Task ThrowAsync() + { + throw await this.ExceptionFactory.CreateRequestFailedExceptionAsync(this).ConfigureAwait(false); + //throw new RequestFailedException(""); + } + /// /// Creates a new instance of with the provided value and HTTP response. /// diff --git a/sdk/core/Azure.Core/src/ResponseExceptionFactory.cs b/sdk/core/Azure.Core/src/ResponseExceptionFactory.cs new file mode 100644 index 000000000000..c0bb8132cbbb --- /dev/null +++ b/sdk/core/Azure.Core/src/ResponseExceptionFactory.cs @@ -0,0 +1,184 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +using System.Globalization; +using System.IO; +using System.Linq; +using System.Text; +using System.Text.Json; +using System.Threading.Tasks; + +namespace Azure.Core.Pipeline +{ + internal class ResponseExceptionFactory + { + private const string DefaultMessage = "Service request failed."; + + private readonly HttpMessageSanitizer _sanitizer; + + public ResponseExceptionFactory(ClientOptions options) + { + _sanitizer = new HttpMessageSanitizer( + options.Diagnostics.LoggedQueryParameters.ToArray(), + options.Diagnostics.LoggedHeaderNames.ToArray()); + } + + public async ValueTask CreateRequestFailedExceptionAsync(Response response, string? message = null, string? errorCode = null, IDictionary? additionalInfo = null, Exception? innerException = null) + { + var content = await ReadContentAsync(response, true).ConfigureAwait(false); + ExtractFailureContent(content, ref message, ref errorCode); + return CreateRequestFailedExceptionWithContent(response, message, content, errorCode, additionalInfo, innerException); + } + + public RequestFailedException CreateRequestFailedException(Response response, string? message = null, string? errorCode = null, IDictionary? additionalInfo = null, Exception? innerException = null) + { + string? content = ReadContentAsync(response, false).EnsureCompleted(); + ExtractFailureContent(content, ref message, ref errorCode); + return CreateRequestFailedExceptionWithContent(response, message, content, errorCode, additionalInfo, innerException); + } + + private static async ValueTask ReadContentAsync(Response response, bool async) + { + string? content = null; + + if (response.ContentStream != null && + ContentTypeUtilities.TryGetTextEncoding(response.Headers.ContentType, out var encoding)) + { + using (var streamReader = new StreamReader(response.ContentStream, encoding)) + { + content = async ? await streamReader.ReadToEndAsync().ConfigureAwait(false) : streamReader.ReadToEnd(); + } + } + + return content; + } + + private static void ExtractFailureContent( + string? content, + ref string? message, + ref string? errorCode) + { + try + { + // Optimistic check for JSON object we expect + if (content == null || + !content.StartsWith("{", StringComparison.OrdinalIgnoreCase)) + return; + + string? parsedMessage = null; + using JsonDocument document = JsonDocument.Parse(content); + if (document.RootElement.TryGetProperty("error", out var errorProperty)) + { + if (errorProperty.TryGetProperty("code", out var codeProperty)) + { + errorCode = codeProperty.GetString(); + } + if (errorProperty.TryGetProperty("message", out var messageProperty)) + { + parsedMessage = messageProperty.GetString(); + } + } + + // Make sure we parsed a message so we don't overwrite the value with null + if (parsedMessage != null) + { + message = parsedMessage; + } + } + catch (Exception) + { + // Ignore any failures - unexpected content will be + // included verbatim in the detailed error message + } + } + + private RequestFailedException CreateRequestFailedExceptionWithContent( + Response response, + string? message = null, + string? content = null, + string? errorCode = null, + IDictionary? additionalInfo = null, + Exception? innerException = null) + { + var formatMessage = CreateRequestFailedMessageWithContent(response, message, content, errorCode, additionalInfo); + var exception = new RequestFailedException(response.Status, formatMessage, errorCode, innerException); + + if (additionalInfo != null) + { + foreach (KeyValuePair keyValuePair in additionalInfo) + { + exception.Data.Add(keyValuePair.Key, keyValuePair.Value); + } + } + + return exception; + } + + private string CreateRequestFailedMessageWithContent( + Response response, + string? message, + string? content, + string? errorCode, + IDictionary? additionalInfo) + { + StringBuilder messageBuilder = new StringBuilder() + .AppendLine(message ?? DefaultMessage) + .Append("Status: ") + .Append(response.Status.ToString(CultureInfo.InvariantCulture)); + + if (!string.IsNullOrEmpty(response.ReasonPhrase)) + { + messageBuilder.Append(" (") + .Append(response.ReasonPhrase) + .AppendLine(")"); + } + else + { + messageBuilder.AppendLine(); + } + + if (!string.IsNullOrWhiteSpace(errorCode)) + { + messageBuilder.Append("ErrorCode: ") + .Append(errorCode) + .AppendLine(); + } + + if (additionalInfo != null && additionalInfo.Count > 0) + { + messageBuilder + .AppendLine() + .AppendLine("Additional Information:"); + foreach (KeyValuePair info in additionalInfo) + { + messageBuilder + .Append(info.Key) + .Append(": ") + .AppendLine(info.Value); + } + } + + if (content != null) + { + messageBuilder + .AppendLine() + .AppendLine("Content:") + .AppendLine(content); + } + + messageBuilder + .AppendLine() + .AppendLine("Headers:"); + + foreach (HttpHeader responseHeader in response.Headers) + { + string headerValue = _sanitizer.SanitizeHeader(responseHeader.Name, responseHeader.Value); + messageBuilder.AppendLine($"{responseHeader.Name}: {headerValue}"); + } + + return messageBuilder.ToString(); + } + } +} diff --git a/sdk/core/Azure.Core/src/Shared/ClientDiagnostics.cs b/sdk/core/Azure.Core/src/Shared/ClientDiagnostics.cs index a7dac8c8e88f..e6c6c23ce536 100644 --- a/sdk/core/Azure.Core/src/Shared/ClientDiagnostics.cs +++ b/sdk/core/Azure.Core/src/Shared/ClientDiagnostics.cs @@ -3,16 +3,13 @@ using System; using System.Collections.Generic; -using System.Diagnostics; using System.Globalization; using System.IO; using System.Linq; -using System.Net.Http.Headers; using System.Reflection; using System.Text; using System.Text.Json; using System.Threading.Tasks; -using Azure.Core.Pipeline; #nullable enable @@ -23,6 +20,7 @@ internal class ClientDiagnostics : DiagnosticScopeFactory private const string DefaultMessage = "Service request failed."; private readonly HttpMessageSanitizer _sanitizer; + public ClientDiagnostics(ClientOptions options) : base( options.GetType().Namespace!, GetResourceProviderNamespace(options.GetType().Assembly), @@ -124,7 +122,7 @@ public async ValueTask CreateRequestFailedMessageAsync(Response response return CreateRequestFailedMessageWithContent(response, message, content, errorCode, additionalInfo); } - public string CreateRequestFailedMessageWithContent(Response response, string? message, string? content, string? errorCode, IDictionary? additionalInfo) + private string CreateRequestFailedMessageWithContent(Response response, string? message, string? content, string? errorCode, IDictionary? additionalInfo) { StringBuilder messageBuilder = new StringBuilder() .AppendLine(message ?? DefaultMessage) diff --git a/sdk/synapse/Azure.Analytics.Synapse.AccessControl/src/Models/SynapseRoleAssignment.cs b/sdk/synapse/Azure.Analytics.Synapse.AccessControl/src/Models/SynapseRoleAssignment.cs index d83b721e7b72..27ea9f870e17 100644 --- a/sdk/synapse/Azure.Analytics.Synapse.AccessControl/src/Models/SynapseRoleAssignment.cs +++ b/sdk/synapse/Azure.Analytics.Synapse.AccessControl/src/Models/SynapseRoleAssignment.cs @@ -41,5 +41,27 @@ public static implicit operator RequestContent(SynapseRoleAssignment value) => R Id = value.Id, Properties = value.Properties }); + + public static implicit operator SynapseRoleAssignment(Response response) + { + switch (response.Status) + { + case 200: + return DeserializeResponse(response); + default: + +#pragma warning disable CA1065 // Do not raise exceptions in unexpected locations + throw new NotImplementedException(); +#pragma warning restore CA1065 // Do not raise exceptions in unexpected locations + + //response.Throw(); + //break; + } + } + + private static SynapseRoleAssignment DeserializeResponse(Response response) + { + throw new NotImplementedException(); + } } } From ff6adbe394b4884f23a88a955aecf6649cf07d49 Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Tue, 17 Aug 2021 17:00:36 -0700 Subject: [PATCH 3/8] wip; need to resolve nullables --- sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs | 5 +++-- sdk/core/Azure.Core/src/Pipeline/HttpPipelineTransport.cs | 6 ++---- sdk/core/Azure.Core/src/Response.cs | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs index 1accded7ef61..ad3e61f9e02b 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs @@ -33,7 +33,6 @@ public HttpPipeline(HttpPipelineTransport transport, HttpPipelinePolicy[]? polic policies = policies ?? Array.Empty(); var all = new HttpPipelinePolicy[policies.Length + 1]; - _transport.ResponseClassifier = ResponseClassifier; all[policies.Length] = new HttpPipelineTransportPolicy(_transport); policies.CopyTo(all, 0); @@ -71,7 +70,9 @@ public ValueTask SendAsync(HttpMessage message, CancellationToken cancellationTo { message.CancellationToken = cancellationToken; AddHttpMessageProperties(message); - return _pipeline.Span[0].ProcessAsync(message, _pipeline.Slice(1)); + var value = _pipeline.Span[0].ProcessAsync(message, _pipeline.Slice(1)); + message.Response.ResponseClassifier = this.ResponseClassifier; + return value; } /// diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipelineTransport.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipelineTransport.cs index 9d898027a9a6..3466cd57b34e 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipelineTransport.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipelineTransport.cs @@ -29,10 +29,8 @@ public abstract class HttpPipelineTransport /// public abstract Request CreateRequest(); + //internal ClientOptions? ClientOptions { get; set; } - internal ClientOptions? ClientOptions { get; set; } - - internal ResponseClassifier? ResponseClassifier { get; set; } - + //internal ResponseClassifier? ResponseClassifier { get; set; } } } diff --git a/sdk/core/Azure.Core/src/Response.cs b/sdk/core/Azure.Core/src/Response.cs index 3af16ccc82fb..de9370ae4c86 100644 --- a/sdk/core/Azure.Core/src/Response.cs +++ b/sdk/core/Azure.Core/src/Response.cs @@ -115,7 +115,7 @@ public virtual BinaryData Content internal abstract ResponseExceptionFactory ExceptionFactory { get; } - internal abstract ResponseClassifier ResponseClassifier { get; } + internal abstract ResponseClassifier ResponseClassifier { get; set; } /// /// Throw a RequestFailedException appropriate to the Response. From 37c3bcf4ecee9f64b917a31398f0597a492e22d8 Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Wed, 18 Aug 2021 09:42:57 -0700 Subject: [PATCH 4/8] set options and classifier in SendAsync --- .../src/Pipeline/HttpClientTransport.cs | 14 +-- .../Azure.Core/src/Pipeline/HttpPipeline.cs | 13 ++- .../src/Pipeline/HttpPipelineBuilder.cs | 3 +- sdk/core/Azure.Core/src/Response.cs | 8 +- .../src/ResponseExceptionFactory.cs | 12 +- .../src/Shared/ClientDiagnostics.cs | 107 ++---------------- 6 files changed, 41 insertions(+), 116 deletions(-) diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpClientTransport.cs b/sdk/core/Azure.Core/src/Pipeline/HttpClientTransport.cs index b5e404011a78..f92f8efdfd0a 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpClientTransport.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpClientTransport.cs @@ -53,10 +53,6 @@ public HttpClientTransport(HttpClient client) /// public static readonly HttpClientTransport Shared = new HttpClientTransport(); - internal ClientOptions? ClientOptions { get; set; } - - internal ResponseClassifier? ResponseClassifier { get; set; } - /// public sealed override Request CreateRequest() => new PipelineRequest(); @@ -133,7 +129,7 @@ private async ValueTask ProcessAsync(HttpMessage message, bool async) throw new RequestFailedException(e.Message, e); } - message.Response = new PipelineResponse(message.Request.ClientRequestId, responseMessage, contentStream, this.ClientOptions!, this.ResponseClassifier!); + message.Response = new PipelineResponse(message.Request.ClientRequestId, responseMessage, contentStream); } private static HttpClient CreateDefaultClient() @@ -504,11 +500,9 @@ private sealed class PipelineResponse : Response private Stream? _contentStream; #pragma warning restore CA2213 - public PipelineResponse(string requestId, HttpResponseMessage responseMessage, Stream? contentStream, ClientOptions options, ResponseClassifier classifier) + public PipelineResponse(string requestId, HttpResponseMessage responseMessage, Stream? contentStream) { ClientRequestId = requestId ?? throw new ArgumentNullException(nameof(requestId)); - ExceptionFactory = new ResponseExceptionFactory(options); - ResponseClassifier = classifier; _responseMessage = responseMessage ?? throw new ArgumentNullException(nameof(responseMessage)); _contentStream = contentStream; _responseContent = _responseMessage.Content; @@ -532,9 +526,9 @@ public override Stream? ContentStream public override string ClientRequestId { get; set; } - internal override ResponseExceptionFactory ExceptionFactory { get; } + internal override ResponseExceptionFactory? ExceptionFactory { get; set; } - internal override ResponseClassifier ResponseClassifier { get; } + internal override ResponseClassifier? ResponseClassifier { get; set; } protected internal override bool TryGetHeader(string name, [NotNullWhen(true)] out string? value) => HttpClientTransport.TryGetHeader(_responseMessage.Headers, _responseContent, name, out value); diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs index ad3e61f9e02b..c7020087ca10 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs @@ -26,9 +26,15 @@ public class HttpPipeline /// Policies to be invoked as part of the pipeline in order. /// The response classifier to be used in invocations. public HttpPipeline(HttpPipelineTransport transport, HttpPipelinePolicy[]? policies = null, ResponseClassifier? responseClassifier = null) + : this(transport, policies, responseClassifier, null) + { + } + + internal HttpPipeline(HttpPipelineTransport transport, HttpPipelinePolicy[]? policies = null, ResponseClassifier? responseClassifier = null, ResponseExceptionFactory? exceptionFactory = null) { _transport = transport ?? throw new ArgumentNullException(nameof(transport)); ResponseClassifier = responseClassifier ?? new ResponseClassifier(); + ExceptionFactory = exceptionFactory ?? new ResponseExceptionFactory(new DefaultClientOptions()); policies = policies ?? Array.Empty(); @@ -60,6 +66,8 @@ public HttpMessage CreateMessage() /// public ResponseClassifier ResponseClassifier { get; } + internal ResponseExceptionFactory ExceptionFactory { get; } + /// /// Invokes the pipeline asynchronously. After the task completes response would be set to the property. /// @@ -71,7 +79,8 @@ public ValueTask SendAsync(HttpMessage message, CancellationToken cancellationTo message.CancellationToken = cancellationToken; AddHttpMessageProperties(message); var value = _pipeline.Span[0].ProcessAsync(message, _pipeline.Slice(1)); - message.Response.ResponseClassifier = this.ResponseClassifier; + message.Response.ResponseClassifier = ResponseClassifier; + message.Response.ExceptionFactory = ExceptionFactory; return value; } @@ -85,6 +94,8 @@ public void Send(HttpMessage message, CancellationToken cancellationToken) message.CancellationToken = cancellationToken; AddHttpMessageProperties(message); _pipeline.Span[0].Process(message, _pipeline.Slice(1)); + message.Response.ResponseClassifier = ResponseClassifier; + message.Response.ExceptionFactory = ExceptionFactory; } /// /// Invokes the pipeline asynchronously with the provided request. diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs index c434652c3aee..a813535984ef 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs @@ -106,7 +106,8 @@ void AddCustomerPolicies(HttpPipelinePosition position) return new HttpPipeline(options.Transport, policies.ToArray(), - responseClassifier); + responseClassifier, + new ResponseExceptionFactory(options)); } // internal for testing diff --git a/sdk/core/Azure.Core/src/Response.cs b/sdk/core/Azure.Core/src/Response.cs index de9370ae4c86..6676ee8dca27 100644 --- a/sdk/core/Azure.Core/src/Response.cs +++ b/sdk/core/Azure.Core/src/Response.cs @@ -113,16 +113,16 @@ public virtual BinaryData Content /// The enumerating in the response. protected internal abstract IEnumerable EnumerateHeaders(); - internal abstract ResponseExceptionFactory ExceptionFactory { get; } + internal abstract ResponseExceptionFactory? ExceptionFactory { get; set; } - internal abstract ResponseClassifier ResponseClassifier { get; set; } + internal abstract ResponseClassifier? ResponseClassifier { get; set; } /// /// Throw a RequestFailedException appropriate to the Response. /// public void Throw() { - throw this.ExceptionFactory.CreateRequestFailedException(this); + throw this.ExceptionFactory!.CreateRequestFailedException(this); //throw new RequestFailedException(""); } @@ -131,7 +131,7 @@ public void Throw() /// public async Task ThrowAsync() { - throw await this.ExceptionFactory.CreateRequestFailedExceptionAsync(this).ConfigureAwait(false); + throw await this.ExceptionFactory!.CreateRequestFailedExceptionAsync(this).ConfigureAwait(false); //throw new RequestFailedException(""); } diff --git a/sdk/core/Azure.Core/src/ResponseExceptionFactory.cs b/sdk/core/Azure.Core/src/ResponseExceptionFactory.cs index c0bb8132cbbb..cfa803cc6e16 100644 --- a/sdk/core/Azure.Core/src/ResponseExceptionFactory.cs +++ b/sdk/core/Azure.Core/src/ResponseExceptionFactory.cs @@ -10,14 +10,20 @@ using System.Text.Json; using System.Threading.Tasks; +#nullable enable + namespace Azure.Core.Pipeline { - internal class ResponseExceptionFactory + /// + /// + public class ResponseExceptionFactory { private const string DefaultMessage = "Service request failed."; private readonly HttpMessageSanitizer _sanitizer; + /// + /// public ResponseExceptionFactory(ClientOptions options) { _sanitizer = new HttpMessageSanitizer( @@ -25,6 +31,8 @@ public ResponseExceptionFactory(ClientOptions options) options.Diagnostics.LoggedHeaderNames.ToArray()); } + /// + /// public async ValueTask CreateRequestFailedExceptionAsync(Response response, string? message = null, string? errorCode = null, IDictionary? additionalInfo = null, Exception? innerException = null) { var content = await ReadContentAsync(response, true).ConfigureAwait(false); @@ -32,6 +40,8 @@ public async ValueTask CreateRequestFailedExceptionAsync return CreateRequestFailedExceptionWithContent(response, message, content, errorCode, additionalInfo, innerException); } + /// + /// public RequestFailedException CreateRequestFailedException(Response response, string? message = null, string? errorCode = null, IDictionary? additionalInfo = null, Exception? innerException = null) { string? content = ReadContentAsync(response, false).EnsureCompleted(); diff --git a/sdk/core/Azure.Core/src/Shared/ClientDiagnostics.cs b/sdk/core/Azure.Core/src/Shared/ClientDiagnostics.cs index e6c6c23ce536..c655b4f0aa33 100644 --- a/sdk/core/Azure.Core/src/Shared/ClientDiagnostics.cs +++ b/sdk/core/Azure.Core/src/Shared/ClientDiagnostics.cs @@ -17,18 +17,14 @@ namespace Azure.Core.Pipeline { internal class ClientDiagnostics : DiagnosticScopeFactory { - private const string DefaultMessage = "Service request failed."; - - private readonly HttpMessageSanitizer _sanitizer; + private readonly ResponseExceptionFactory _exceptionFactory; public ClientDiagnostics(ClientOptions options) : base( options.GetType().Namespace!, GetResourceProviderNamespace(options.GetType().Assembly), options.Diagnostics.IsDistributedTracingEnabled) { - _sanitizer = new HttpMessageSanitizer( - options.Diagnostics.LoggedQueryParameters.ToArray(), - options.Diagnostics.LoggedHeaderNames.ToArray()); + _exceptionFactory = new ResponseExceptionFactory(options); } /// @@ -82,19 +78,15 @@ protected virtual void ExtractFailureContent( public async ValueTask CreateRequestFailedExceptionAsync(Response response, string? message = null, string? errorCode = null, IDictionary? additionalInfo = null, Exception? innerException = null) { - var content = await ReadContentAsync(response, true).ConfigureAwait(false); - ExtractFailureContent(content, response.Headers, ref message, ref errorCode, ref additionalInfo); - return CreateRequestFailedExceptionWithContent(response, message, content, errorCode, additionalInfo, innerException); + return await _exceptionFactory.CreateRequestFailedExceptionAsync(response, message, errorCode, additionalInfo, innerException).ConfigureAwait(false); } public RequestFailedException CreateRequestFailedException(Response response, string? message = null, string? errorCode = null, IDictionary? additionalInfo = null, Exception? innerException = null) { - string? content = ReadContentAsync(response, false).EnsureCompleted(); - ExtractFailureContent(content, response.Headers, ref message, ref errorCode, ref additionalInfo); - return CreateRequestFailedExceptionWithContent(response, message, content, errorCode, additionalInfo, innerException); + return _exceptionFactory.CreateRequestFailedException(response, message, errorCode, additionalInfo, innerException); } - public RequestFailedException CreateRequestFailedExceptionWithContent( + public RequestFailedException CreateRequestFailedMessageAsync( Response response, string? message = null, string? content = null, @@ -102,100 +94,17 @@ public RequestFailedException CreateRequestFailedExceptionWithContent( IDictionary? additionalInfo = null, Exception? innerException = null) { - var formatMessage = CreateRequestFailedMessageWithContent(response, message, content, errorCode, additionalInfo); - var exception = new RequestFailedException(response.Status, formatMessage, errorCode, innerException); - - if (additionalInfo != null) - { - foreach (KeyValuePair keyValuePair in additionalInfo) - { - exception.Data.Add(keyValuePair.Key, keyValuePair.Value); - } - } - - return exception; + return _exceptionFactory.CreateRequestFailedMessageAsync(response, message, content, errorCode, additionalInfo, innerException); } public async ValueTask CreateRequestFailedMessageAsync(Response response, string? message, string? errorCode, IDictionary? additionalInfo, bool async) { - var content = await ReadContentAsync(response, async).ConfigureAwait(false); - return CreateRequestFailedMessageWithContent(response, message, content, errorCode, additionalInfo); + return _exceptionFactory.CreateRequestFailedMessageAsync(response, message, errorCode, additionalInfo, async); } private string CreateRequestFailedMessageWithContent(Response response, string? message, string? content, string? errorCode, IDictionary? additionalInfo) { - StringBuilder messageBuilder = new StringBuilder() - .AppendLine(message ?? DefaultMessage) - .Append("Status: ") - .Append(response.Status.ToString(CultureInfo.InvariantCulture)); - - if (!string.IsNullOrEmpty(response.ReasonPhrase)) - { - messageBuilder.Append(" (") - .Append(response.ReasonPhrase) - .AppendLine(")"); - } - else - { - messageBuilder.AppendLine(); - } - - if (!string.IsNullOrWhiteSpace(errorCode)) - { - messageBuilder.Append("ErrorCode: ") - .Append(errorCode) - .AppendLine(); - } - - if (additionalInfo != null && additionalInfo.Count > 0) - { - messageBuilder - .AppendLine() - .AppendLine("Additional Information:"); - foreach (KeyValuePair info in additionalInfo) - { - messageBuilder - .Append(info.Key) - .Append(": ") - .AppendLine(info.Value); - } - } - - if (content != null) - { - messageBuilder - .AppendLine() - .AppendLine("Content:") - .AppendLine(content); - } - - messageBuilder - .AppendLine() - .AppendLine("Headers:"); - - foreach (HttpHeader responseHeader in response.Headers) - { - string headerValue = _sanitizer.SanitizeHeader(responseHeader.Name, responseHeader.Value); - messageBuilder.AppendLine($"{responseHeader.Name}: {headerValue}"); - } - - return messageBuilder.ToString(); - } - - private static async ValueTask ReadContentAsync(Response response, bool async) - { - string? content = null; - - if (response.ContentStream != null && - ContentTypeUtilities.TryGetTextEncoding(response.Headers.ContentType, out var encoding)) - { - using (var streamReader = new StreamReader(response.ContentStream, encoding)) - { - content = async ? await streamReader.ReadToEndAsync().ConfigureAwait(false) : streamReader.ReadToEnd(); - } - } - - return content; + return _exceptionFactory.CreateRequestFailedMessageWithContent(response, message, content, errorCode, additionalInfo); } internal static string? GetResourceProviderNamespace(Assembly assembly) From 6461b5daf7973924778fba9c57927339b46e6eb8 Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Wed, 18 Aug 2021 09:45:40 -0700 Subject: [PATCH 5/8] missed file --- sdk/core/Azure.Core/src/Pipeline/HttpPipelineTransport.cs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipelineTransport.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipelineTransport.cs index 3466cd57b34e..42c9c8af5bfa 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipelineTransport.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipelineTransport.cs @@ -28,9 +28,5 @@ public abstract class HttpPipelineTransport /// /// public abstract Request CreateRequest(); - - //internal ClientOptions? ClientOptions { get; set; } - - //internal ResponseClassifier? ResponseClassifier { get; set; } } } From 5ec8cd28c8b37b034bf729a037b15e995215061e Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Thu, 19 Aug 2021 17:50:04 -0700 Subject: [PATCH 6/8] update to move exception formatting logic to RequestClassifier --- .../src/Pipeline/HttpClientTransport.cs | 2 - .../Azure.Core/src/Pipeline/HttpPipeline.cs | 10 - .../src/Pipeline/HttpPipelineBuilder.cs | 3 +- sdk/core/Azure.Core/src/Response.cs | 6 +- sdk/core/Azure.Core/src/ResponseClassifier.cs | 191 +++++++++++++++++ .../src/ResponseExceptionFactory.cs | 194 ------------------ .../src/Shared/ClientDiagnostics.cs | 83 -------- .../Generated/SynapseAccessControlClient.cs | 33 +-- 8 files changed, 211 insertions(+), 311 deletions(-) delete mode 100644 sdk/core/Azure.Core/src/ResponseExceptionFactory.cs diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpClientTransport.cs b/sdk/core/Azure.Core/src/Pipeline/HttpClientTransport.cs index f92f8efdfd0a..ab6dbab2e712 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpClientTransport.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpClientTransport.cs @@ -526,8 +526,6 @@ public override Stream? ContentStream public override string ClientRequestId { get; set; } - internal override ResponseExceptionFactory? ExceptionFactory { get; set; } - internal override ResponseClassifier? ResponseClassifier { get; set; } protected internal override bool TryGetHeader(string name, [NotNullWhen(true)] out string? value) => HttpClientTransport.TryGetHeader(_responseMessage.Headers, _responseContent, name, out value); diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs index c7020087ca10..9a69c025bd5c 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs @@ -26,15 +26,9 @@ public class HttpPipeline /// Policies to be invoked as part of the pipeline in order. /// The response classifier to be used in invocations. public HttpPipeline(HttpPipelineTransport transport, HttpPipelinePolicy[]? policies = null, ResponseClassifier? responseClassifier = null) - : this(transport, policies, responseClassifier, null) - { - } - - internal HttpPipeline(HttpPipelineTransport transport, HttpPipelinePolicy[]? policies = null, ResponseClassifier? responseClassifier = null, ResponseExceptionFactory? exceptionFactory = null) { _transport = transport ?? throw new ArgumentNullException(nameof(transport)); ResponseClassifier = responseClassifier ?? new ResponseClassifier(); - ExceptionFactory = exceptionFactory ?? new ResponseExceptionFactory(new DefaultClientOptions()); policies = policies ?? Array.Empty(); @@ -66,8 +60,6 @@ public HttpMessage CreateMessage() /// public ResponseClassifier ResponseClassifier { get; } - internal ResponseExceptionFactory ExceptionFactory { get; } - /// /// Invokes the pipeline asynchronously. After the task completes response would be set to the property. /// @@ -80,7 +72,6 @@ public ValueTask SendAsync(HttpMessage message, CancellationToken cancellationTo AddHttpMessageProperties(message); var value = _pipeline.Span[0].ProcessAsync(message, _pipeline.Slice(1)); message.Response.ResponseClassifier = ResponseClassifier; - message.Response.ExceptionFactory = ExceptionFactory; return value; } @@ -95,7 +86,6 @@ public void Send(HttpMessage message, CancellationToken cancellationToken) AddHttpMessageProperties(message); _pipeline.Span[0].Process(message, _pipeline.Slice(1)); message.Response.ResponseClassifier = ResponseClassifier; - message.Response.ExceptionFactory = ExceptionFactory; } /// /// Invokes the pipeline asynchronously with the provided request. diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs index a813535984ef..c434652c3aee 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs @@ -106,8 +106,7 @@ void AddCustomerPolicies(HttpPipelinePosition position) return new HttpPipeline(options.Transport, policies.ToArray(), - responseClassifier, - new ResponseExceptionFactory(options)); + responseClassifier); } // internal for testing diff --git a/sdk/core/Azure.Core/src/Response.cs b/sdk/core/Azure.Core/src/Response.cs index 6676ee8dca27..9c6dd634dbfb 100644 --- a/sdk/core/Azure.Core/src/Response.cs +++ b/sdk/core/Azure.Core/src/Response.cs @@ -113,8 +113,6 @@ public virtual BinaryData Content /// The enumerating in the response. protected internal abstract IEnumerable EnumerateHeaders(); - internal abstract ResponseExceptionFactory? ExceptionFactory { get; set; } - internal abstract ResponseClassifier? ResponseClassifier { get; set; } /// @@ -122,7 +120,7 @@ public virtual BinaryData Content /// public void Throw() { - throw this.ExceptionFactory!.CreateRequestFailedException(this); + throw this.ResponseClassifier!.CreateRequestFailedException(this); //throw new RequestFailedException(""); } @@ -131,7 +129,7 @@ public void Throw() /// public async Task ThrowAsync() { - throw await this.ExceptionFactory!.CreateRequestFailedExceptionAsync(this).ConfigureAwait(false); + throw await this.ResponseClassifier!.CreateRequestFailedExceptionAsync(this).ConfigureAwait(false); //throw new RequestFailedException(""); } diff --git a/sdk/core/Azure.Core/src/ResponseClassifier.cs b/sdk/core/Azure.Core/src/ResponseClassifier.cs index e770e6e43271..1b778479247e 100644 --- a/sdk/core/Azure.Core/src/ResponseClassifier.cs +++ b/sdk/core/Azure.Core/src/ResponseClassifier.cs @@ -2,7 +2,14 @@ // Licensed under the MIT License. using System; +using System.Collections.Generic; +using System.Globalization; using System.IO; +using System.Linq; +using System.Text; +using System.Text.Json; +using System.Threading.Tasks; +using Azure.Core.Pipeline; namespace Azure.Core { @@ -11,6 +18,30 @@ namespace Azure.Core /// public class ResponseClassifier { + private const string DefaultFailureMessage = "Service request failed."; + + private readonly HttpMessageSanitizer _sanitizer; + + /// + /// Initializes a new instance of . + /// + public ResponseClassifier() : this(new DefaultClientOptions()) + { + } + + /// + /// + + /// + /// Initializes a new instance of . + /// + public ResponseClassifier(ClientOptions options) + { + _sanitizer = new HttpMessageSanitizer( + options?.Diagnostics.LoggedQueryParameters.ToArray() ?? Array.Empty(), + options?.Diagnostics.LoggedHeaderNames.ToArray() ?? Array.Empty()); + } + /// /// Specifies if the request contained in the should be retried. /// @@ -57,5 +88,165 @@ public virtual bool IsErrorResponse(HttpMessage message) var statusKind = message.Response.Status / 100; return statusKind == 4 || statusKind == 5; } + + /// + /// + public async ValueTask CreateRequestFailedExceptionAsync(Response response, string? message = null, string? errorCode = null, IDictionary? additionalInfo = null, Exception? innerException = null) + { + var content = await ReadContentAsync(response, true).ConfigureAwait(false); + ExtractFailureContent(content, ref message, ref errorCode); + return CreateRequestFailedExceptionWithContent(response, message, content, errorCode, additionalInfo, innerException); + } + + /// + /// + public RequestFailedException CreateRequestFailedException(Response response, string? message = null, string? errorCode = null, IDictionary? additionalInfo = null, Exception? innerException = null) + { + string? content = ReadContentAsync(response, false).EnsureCompleted(); + ExtractFailureContent(content, ref message, ref errorCode); + return CreateRequestFailedExceptionWithContent(response, message, content, errorCode, additionalInfo, innerException); + } + + private static async ValueTask ReadContentAsync(Response response, bool async) + { + string? content = null; + + if (response.ContentStream != null && + ContentTypeUtilities.TryGetTextEncoding(response.Headers.ContentType, out var encoding)) + { + using (var streamReader = new StreamReader(response.ContentStream, encoding)) + { + content = async ? await streamReader.ReadToEndAsync().ConfigureAwait(false) : streamReader.ReadToEnd(); + } + } + + return content; + } + + private static void ExtractFailureContent( + string? content, + ref string? message, + ref string? errorCode) + { + try + { + // Optimistic check for JSON object we expect + if (content == null || + !content.StartsWith("{", StringComparison.OrdinalIgnoreCase)) + return; + + string? parsedMessage = null; + using JsonDocument document = JsonDocument.Parse(content); + if (document.RootElement.TryGetProperty("error", out var errorProperty)) + { + if (errorProperty.TryGetProperty("code", out var codeProperty)) + { + errorCode = codeProperty.GetString(); + } + if (errorProperty.TryGetProperty("message", out var messageProperty)) + { + parsedMessage = messageProperty.GetString(); + } + } + + // Make sure we parsed a message so we don't overwrite the value with null + if (parsedMessage != null) + { + message = parsedMessage; + } + } + catch (Exception) + { + // Ignore any failures - unexpected content will be + // included verbatim in the detailed error message + } + } + + private RequestFailedException CreateRequestFailedExceptionWithContent( + Response response, + string? message = null, + string? content = null, + string? errorCode = null, + IDictionary? additionalInfo = null, + Exception? innerException = null) + { + var formatMessage = CreateRequestFailedMessageWithContent(response, message, content, errorCode, additionalInfo); + var exception = new RequestFailedException(response.Status, formatMessage, errorCode, innerException); + + if (additionalInfo != null) + { + foreach (KeyValuePair keyValuePair in additionalInfo) + { + exception.Data.Add(keyValuePair.Key, keyValuePair.Value); + } + } + + return exception; + } + + private string CreateRequestFailedMessageWithContent( + Response response, + string? message, + string? content, + string? errorCode, + IDictionary? additionalInfo) + { + StringBuilder messageBuilder = new StringBuilder() + .AppendLine(message ?? DefaultFailureMessage) + .Append("Status: ") + .Append(response.Status.ToString(CultureInfo.InvariantCulture)); + + if (!string.IsNullOrEmpty(response.ReasonPhrase)) + { + messageBuilder.Append(" (") + .Append(response.ReasonPhrase) + .AppendLine(")"); + } + else + { + messageBuilder.AppendLine(); + } + + if (!string.IsNullOrWhiteSpace(errorCode)) + { + messageBuilder.Append("ErrorCode: ") + .Append(errorCode) + .AppendLine(); + } + + if (additionalInfo != null && additionalInfo.Count > 0) + { + messageBuilder + .AppendLine() + .AppendLine("Additional Information:"); + foreach (KeyValuePair info in additionalInfo) + { + messageBuilder + .Append(info.Key) + .Append(": ") + .AppendLine(info.Value); + } + } + + if (content != null) + { + messageBuilder + .AppendLine() + .AppendLine("Content:") + .AppendLine(content); + } + + messageBuilder + .AppendLine() + .AppendLine("Headers:"); + + foreach (HttpHeader responseHeader in response.Headers) + { + string headerValue = _sanitizer.SanitizeHeader(responseHeader.Name, responseHeader.Value); + messageBuilder.AppendLine($"{responseHeader.Name}: {headerValue}"); + } + + return messageBuilder.ToString(); + } } } diff --git a/sdk/core/Azure.Core/src/ResponseExceptionFactory.cs b/sdk/core/Azure.Core/src/ResponseExceptionFactory.cs deleted file mode 100644 index cfa803cc6e16..000000000000 --- a/sdk/core/Azure.Core/src/ResponseExceptionFactory.cs +++ /dev/null @@ -1,194 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -using System; -using System.Collections.Generic; -using System.Globalization; -using System.IO; -using System.Linq; -using System.Text; -using System.Text.Json; -using System.Threading.Tasks; - -#nullable enable - -namespace Azure.Core.Pipeline -{ - /// - /// - public class ResponseExceptionFactory - { - private const string DefaultMessage = "Service request failed."; - - private readonly HttpMessageSanitizer _sanitizer; - - /// - /// - public ResponseExceptionFactory(ClientOptions options) - { - _sanitizer = new HttpMessageSanitizer( - options.Diagnostics.LoggedQueryParameters.ToArray(), - options.Diagnostics.LoggedHeaderNames.ToArray()); - } - - /// - /// - public async ValueTask CreateRequestFailedExceptionAsync(Response response, string? message = null, string? errorCode = null, IDictionary? additionalInfo = null, Exception? innerException = null) - { - var content = await ReadContentAsync(response, true).ConfigureAwait(false); - ExtractFailureContent(content, ref message, ref errorCode); - return CreateRequestFailedExceptionWithContent(response, message, content, errorCode, additionalInfo, innerException); - } - - /// - /// - public RequestFailedException CreateRequestFailedException(Response response, string? message = null, string? errorCode = null, IDictionary? additionalInfo = null, Exception? innerException = null) - { - string? content = ReadContentAsync(response, false).EnsureCompleted(); - ExtractFailureContent(content, ref message, ref errorCode); - return CreateRequestFailedExceptionWithContent(response, message, content, errorCode, additionalInfo, innerException); - } - - private static async ValueTask ReadContentAsync(Response response, bool async) - { - string? content = null; - - if (response.ContentStream != null && - ContentTypeUtilities.TryGetTextEncoding(response.Headers.ContentType, out var encoding)) - { - using (var streamReader = new StreamReader(response.ContentStream, encoding)) - { - content = async ? await streamReader.ReadToEndAsync().ConfigureAwait(false) : streamReader.ReadToEnd(); - } - } - - return content; - } - - private static void ExtractFailureContent( - string? content, - ref string? message, - ref string? errorCode) - { - try - { - // Optimistic check for JSON object we expect - if (content == null || - !content.StartsWith("{", StringComparison.OrdinalIgnoreCase)) - return; - - string? parsedMessage = null; - using JsonDocument document = JsonDocument.Parse(content); - if (document.RootElement.TryGetProperty("error", out var errorProperty)) - { - if (errorProperty.TryGetProperty("code", out var codeProperty)) - { - errorCode = codeProperty.GetString(); - } - if (errorProperty.TryGetProperty("message", out var messageProperty)) - { - parsedMessage = messageProperty.GetString(); - } - } - - // Make sure we parsed a message so we don't overwrite the value with null - if (parsedMessage != null) - { - message = parsedMessage; - } - } - catch (Exception) - { - // Ignore any failures - unexpected content will be - // included verbatim in the detailed error message - } - } - - private RequestFailedException CreateRequestFailedExceptionWithContent( - Response response, - string? message = null, - string? content = null, - string? errorCode = null, - IDictionary? additionalInfo = null, - Exception? innerException = null) - { - var formatMessage = CreateRequestFailedMessageWithContent(response, message, content, errorCode, additionalInfo); - var exception = new RequestFailedException(response.Status, formatMessage, errorCode, innerException); - - if (additionalInfo != null) - { - foreach (KeyValuePair keyValuePair in additionalInfo) - { - exception.Data.Add(keyValuePair.Key, keyValuePair.Value); - } - } - - return exception; - } - - private string CreateRequestFailedMessageWithContent( - Response response, - string? message, - string? content, - string? errorCode, - IDictionary? additionalInfo) - { - StringBuilder messageBuilder = new StringBuilder() - .AppendLine(message ?? DefaultMessage) - .Append("Status: ") - .Append(response.Status.ToString(CultureInfo.InvariantCulture)); - - if (!string.IsNullOrEmpty(response.ReasonPhrase)) - { - messageBuilder.Append(" (") - .Append(response.ReasonPhrase) - .AppendLine(")"); - } - else - { - messageBuilder.AppendLine(); - } - - if (!string.IsNullOrWhiteSpace(errorCode)) - { - messageBuilder.Append("ErrorCode: ") - .Append(errorCode) - .AppendLine(); - } - - if (additionalInfo != null && additionalInfo.Count > 0) - { - messageBuilder - .AppendLine() - .AppendLine("Additional Information:"); - foreach (KeyValuePair info in additionalInfo) - { - messageBuilder - .Append(info.Key) - .Append(": ") - .AppendLine(info.Value); - } - } - - if (content != null) - { - messageBuilder - .AppendLine() - .AppendLine("Content:") - .AppendLine(content); - } - - messageBuilder - .AppendLine() - .AppendLine("Headers:"); - - foreach (HttpHeader responseHeader in response.Headers) - { - string headerValue = _sanitizer.SanitizeHeader(responseHeader.Name, responseHeader.Value); - messageBuilder.AppendLine($"{responseHeader.Name}: {headerValue}"); - } - - return messageBuilder.ToString(); - } - } -} diff --git a/sdk/core/Azure.Core/src/Shared/ClientDiagnostics.cs b/sdk/core/Azure.Core/src/Shared/ClientDiagnostics.cs index c655b4f0aa33..440270a002c6 100644 --- a/sdk/core/Azure.Core/src/Shared/ClientDiagnostics.cs +++ b/sdk/core/Azure.Core/src/Shared/ClientDiagnostics.cs @@ -17,94 +17,11 @@ namespace Azure.Core.Pipeline { internal class ClientDiagnostics : DiagnosticScopeFactory { - private readonly ResponseExceptionFactory _exceptionFactory; - public ClientDiagnostics(ClientOptions options) : base( options.GetType().Namespace!, GetResourceProviderNamespace(options.GetType().Assembly), options.Diagnostics.IsDistributedTracingEnabled) { - _exceptionFactory = new ResponseExceptionFactory(options); - } - - /// - /// Partial method that can optionally be defined to extract the error - /// message, code, and details in a service specific manner. - /// - /// The error content. - /// The response headers. - /// The error message. - /// The error code. - /// Additional error details. - protected virtual void ExtractFailureContent( - string? content, - ResponseHeaders responseHeaders, - ref string? message, - ref string? errorCode, - ref IDictionary? additionalInfo) - { - try - { - // Optimistic check for JSON object we expect - if (content == null || - !content.StartsWith("{", StringComparison.OrdinalIgnoreCase)) return; - - string? parsedMessage = null; - using JsonDocument document = JsonDocument.Parse(content); - if (document.RootElement.TryGetProperty("error", out var errorProperty)) - { - if (errorProperty.TryGetProperty("code", out var codeProperty)) - { - errorCode = codeProperty.GetString(); - } - if (errorProperty.TryGetProperty("message", out var messageProperty)) - { - parsedMessage = messageProperty.GetString(); - } - } - - // Make sure we parsed a message so we don't overwrite the value with null - if (parsedMessage != null) - { - message = parsedMessage; - } - } - catch (Exception) - { - // Ignore any failures - unexpected content will be - // included verbatim in the detailed error message - } - } - - public async ValueTask CreateRequestFailedExceptionAsync(Response response, string? message = null, string? errorCode = null, IDictionary? additionalInfo = null, Exception? innerException = null) - { - return await _exceptionFactory.CreateRequestFailedExceptionAsync(response, message, errorCode, additionalInfo, innerException).ConfigureAwait(false); - } - - public RequestFailedException CreateRequestFailedException(Response response, string? message = null, string? errorCode = null, IDictionary? additionalInfo = null, Exception? innerException = null) - { - return _exceptionFactory.CreateRequestFailedException(response, message, errorCode, additionalInfo, innerException); - } - - public RequestFailedException CreateRequestFailedMessageAsync( - Response response, - string? message = null, - string? content = null, - string? errorCode = null, - IDictionary? additionalInfo = null, - Exception? innerException = null) - { - return _exceptionFactory.CreateRequestFailedMessageAsync(response, message, content, errorCode, additionalInfo, innerException); - } - - public async ValueTask CreateRequestFailedMessageAsync(Response response, string? message, string? errorCode, IDictionary? additionalInfo, bool async) - { - return _exceptionFactory.CreateRequestFailedMessageAsync(response, message, errorCode, additionalInfo, async); - } - - private string CreateRequestFailedMessageWithContent(Response response, string? message, string? content, string? errorCode, IDictionary? additionalInfo) - { - return _exceptionFactory.CreateRequestFailedMessageWithContent(response, message, content, errorCode, additionalInfo); } internal static string? GetResourceProviderNamespace(Assembly assembly) diff --git a/sdk/synapse/Azure.Analytics.Synapse.AccessControl/src/Generated/SynapseAccessControlClient.cs b/sdk/synapse/Azure.Analytics.Synapse.AccessControl/src/Generated/SynapseAccessControlClient.cs index 2a12a3c5b376..04dcf1ba7c48 100644 --- a/sdk/synapse/Azure.Analytics.Synapse.AccessControl/src/Generated/SynapseAccessControlClient.cs +++ b/sdk/synapse/Azure.Analytics.Synapse.AccessControl/src/Generated/SynapseAccessControlClient.cs @@ -23,6 +23,7 @@ public partial class SynapseAccessControlClient private Uri endpoint; private readonly string apiVersion; private readonly ClientDiagnostics _clientDiagnostics; + private readonly ResponseClassifier _responseClassifier; /// Initializes a new instance of SynapseAccessControlClient for mocking. protected SynapseAccessControlClient() @@ -149,7 +150,7 @@ public virtual async Task CheckPrincipalAccessAsync(RequestContent con case 200: return message.Response; default: - throw await _clientDiagnostics.CreateRequestFailedExceptionAsync(message.Response).ConfigureAwait(false); + throw await _responseClassifier.CreateRequestFailedExceptionAsync(message.Response).ConfigureAwait(false); } } else @@ -260,7 +261,7 @@ public virtual Response CheckPrincipalAccess(RequestContent content, RequestOpti case 200: return message.Response; default: - throw _clientDiagnostics.CreateRequestFailedException(message.Response); + throw _responseClassifier.CreateRequestFailedException(message.Response); } } else @@ -322,7 +323,7 @@ public virtual async Task ListRoleAssignmentsAsync(string roleId = nul case 200: return message.Response; default: - throw await _clientDiagnostics.CreateRequestFailedExceptionAsync(message.Response).ConfigureAwait(false); + throw await _responseClassifier.CreateRequestFailedExceptionAsync(message.Response).ConfigureAwait(false); } } else @@ -365,7 +366,7 @@ public virtual Response ListRoleAssignments(string roleId = null, string princip case 200: return message.Response; default: - throw _clientDiagnostics.CreateRequestFailedException(message.Response); + throw _responseClassifier.CreateRequestFailedException(message.Response); } } else @@ -477,7 +478,7 @@ public virtual async Task CreateRoleAssignmentAsync(string roleAssignm case 200: return message.Response; default: - throw await _clientDiagnostics.CreateRequestFailedExceptionAsync(message.Response).ConfigureAwait(false); + throw await _responseClassifier.CreateRequestFailedExceptionAsync(message.Response).ConfigureAwait(false); } } else @@ -553,7 +554,7 @@ public virtual Response CreateRoleAssignment(string roleAssignmentId, RequestCon case 200: return message.Response; default: - throw _clientDiagnostics.CreateRequestFailedException(message.Response); + throw _responseClassifier.CreateRequestFailedException(message.Response); } } else @@ -614,7 +615,7 @@ public virtual async Task GetRoleAssignmentByIdAsync(string roleAssign case 200: return message.Response; default: - throw await _clientDiagnostics.CreateRequestFailedExceptionAsync(message.Response).ConfigureAwait(false); + throw await _responseClassifier.CreateRequestFailedExceptionAsync(message.Response).ConfigureAwait(false); } } else @@ -654,7 +655,7 @@ public virtual Response GetRoleAssignmentById(string roleAssignmentId, RequestOp case 200: return message.Response; default: - throw _clientDiagnostics.CreateRequestFailedException(message.Response); + throw _responseClassifier.CreateRequestFailedException(message.Response); } } else @@ -714,7 +715,7 @@ public virtual async Task DeleteRoleAssignmentByIdAsync(string roleAss case 204: return message.Response; default: - throw await _clientDiagnostics.CreateRequestFailedExceptionAsync(message.Response).ConfigureAwait(false); + throw await _responseClassifier.CreateRequestFailedExceptionAsync(message.Response).ConfigureAwait(false); } } else @@ -756,7 +757,7 @@ public virtual Response DeleteRoleAssignmentById(string roleAssignmentId, string case 204: return message.Response; default: - throw _clientDiagnostics.CreateRequestFailedException(message.Response); + throw _responseClassifier.CreateRequestFailedException(message.Response); } } else @@ -820,7 +821,7 @@ public virtual async Task ListRoleDefinitionsAsync(bool? isBuiltIn = n case 200: return message.Response; default: - throw await _clientDiagnostics.CreateRequestFailedExceptionAsync(message.Response).ConfigureAwait(false); + throw await _responseClassifier.CreateRequestFailedExceptionAsync(message.Response).ConfigureAwait(false); } } else @@ -861,7 +862,7 @@ public virtual Response ListRoleDefinitions(bool? isBuiltIn = null, string scope case 200: return message.Response; default: - throw _clientDiagnostics.CreateRequestFailedException(message.Response); + throw _responseClassifier.CreateRequestFailedException(message.Response); } } else @@ -927,7 +928,7 @@ public virtual async Task GetRoleDefinitionByIdAsync(string roleDefini case 200: return message.Response; default: - throw await _clientDiagnostics.CreateRequestFailedExceptionAsync(message.Response).ConfigureAwait(false); + throw await _responseClassifier.CreateRequestFailedExceptionAsync(message.Response).ConfigureAwait(false); } } else @@ -967,7 +968,7 @@ public virtual Response GetRoleDefinitionById(string roleDefinitionId, RequestOp case 200: return message.Response; default: - throw _clientDiagnostics.CreateRequestFailedException(message.Response); + throw _responseClassifier.CreateRequestFailedException(message.Response); } } else @@ -1024,7 +1025,7 @@ public virtual async Task ListScopesAsync(RequestOptions options = nul case 200: return message.Response; default: - throw await _clientDiagnostics.CreateRequestFailedExceptionAsync(message.Response).ConfigureAwait(false); + throw await _responseClassifier.CreateRequestFailedExceptionAsync(message.Response).ConfigureAwait(false); } } else @@ -1063,7 +1064,7 @@ public virtual Response ListScopes(RequestOptions options = null) case 200: return message.Response; default: - throw _clientDiagnostics.CreateRequestFailedException(message.Response); + throw _responseClassifier.CreateRequestFailedException(message.Response); } } else From 43a9fee72515189541cf4a0dd1bd8db69211dec9 Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Fri, 20 Aug 2021 11:23:10 -0700 Subject: [PATCH 7/8] updates --- sdk/core/Azure.Core/src/Pipeline/HttpClientTransport.cs | 2 -- sdk/core/Azure.Core/src/Response.cs | 2 +- .../src/Generated/SynapseAccessControlClient.cs | 1 + 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpClientTransport.cs b/sdk/core/Azure.Core/src/Pipeline/HttpClientTransport.cs index ab6dbab2e712..e51dd0eb5132 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpClientTransport.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpClientTransport.cs @@ -526,8 +526,6 @@ public override Stream? ContentStream public override string ClientRequestId { get; set; } - internal override ResponseClassifier? ResponseClassifier { get; set; } - protected internal override bool TryGetHeader(string name, [NotNullWhen(true)] out string? value) => HttpClientTransport.TryGetHeader(_responseMessage.Headers, _responseContent, name, out value); protected internal override bool TryGetHeaderValues(string name, [NotNullWhen(true)] out IEnumerable? values) => HttpClientTransport.TryGetHeader(_responseMessage.Headers, _responseContent, name, out values); diff --git a/sdk/core/Azure.Core/src/Response.cs b/sdk/core/Azure.Core/src/Response.cs index 9c6dd634dbfb..5a64970f7b71 100644 --- a/sdk/core/Azure.Core/src/Response.cs +++ b/sdk/core/Azure.Core/src/Response.cs @@ -113,7 +113,7 @@ public virtual BinaryData Content /// The enumerating in the response. protected internal abstract IEnumerable EnumerateHeaders(); - internal abstract ResponseClassifier? ResponseClassifier { get; set; } + internal ResponseClassifier? ResponseClassifier { get; set; } /// /// Throw a RequestFailedException appropriate to the Response. diff --git a/sdk/synapse/Azure.Analytics.Synapse.AccessControl/src/Generated/SynapseAccessControlClient.cs b/sdk/synapse/Azure.Analytics.Synapse.AccessControl/src/Generated/SynapseAccessControlClient.cs index 04dcf1ba7c48..60cd1c70b9a8 100644 --- a/sdk/synapse/Azure.Analytics.Synapse.AccessControl/src/Generated/SynapseAccessControlClient.cs +++ b/sdk/synapse/Azure.Analytics.Synapse.AccessControl/src/Generated/SynapseAccessControlClient.cs @@ -47,6 +47,7 @@ public SynapseAccessControlClient(Uri endpoint, TokenCredential credential, Syna options ??= new SynapseAdministrationClientOptions(); _clientDiagnostics = new ClientDiagnostics(options); + _responseClassifier = new ResponseClassifier(options); _tokenCredential = credential; var authPolicy = new BearerTokenAuthenticationPolicy(_tokenCredential, AuthorizationScopes); Pipeline = HttpPipelineBuilder.Build(options, new HttpPipelinePolicy[] { new LowLevelCallbackPolicy() }, new HttpPipelinePolicy[] { authPolicy }, new ResponseClassifier()); From 3301093c0beaa6eae4a497716de1fd6f94511d87 Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Fri, 20 Aug 2021 17:02:58 -0700 Subject: [PATCH 8/8] WIP --- .../tests/AccessControlClientLiveTests.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/sdk/synapse/Azure.Analytics.Synapse.AccessControl/tests/AccessControlClientLiveTests.cs b/sdk/synapse/Azure.Analytics.Synapse.AccessControl/tests/AccessControlClientLiveTests.cs index 79b58e6219a3..f83422919ef6 100644 --- a/sdk/synapse/Azure.Analytics.Synapse.AccessControl/tests/AccessControlClientLiveTests.cs +++ b/sdk/synapse/Azure.Analytics.Synapse.AccessControl/tests/AccessControlClientLiveTests.cs @@ -87,7 +87,12 @@ public async Task CreateRoleAssignment_ModelTypeParameterOverload() SynapseRoleAssignment roleAssignment = new SynapseRoleAssignment(roleId, principalId, scope); - await client.CreateRoleAssignmentAsync(roleAssignmentId, roleAssignment); + SynapseRoleAssignment returnedRoleAssignment = await client.CreateRoleAssignmentAsync(roleAssignmentId, roleAssignment, new RequestOptions() + { + StatusOption = ResponseStatusOption.NoThrow + }); + + // TODO: Finish this test and figure out the rest. await using DisposableClientRole role = await DisposableClientRole.Create(client, TestEnvironment);