From fce3b2a900ff9742ebb496cedfa30872b5aea686 Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Tue, 17 Aug 2021 13:14:33 -0700 Subject: [PATCH 01/15] 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 50f6ae6f1918e..497eb0123dee1 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 70437c33a89ff..1daf3a7bda92a 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 4ecaa9333be08..3e85530abe188 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 a837b7ffcfe5c..1e05f94afebaa 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 89d64c1d254f0..95dae92494fcb 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 8be378f54fee2..77e004e479612 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 7627b8d80956cc1402de7e97aed43f68f5fe355a Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Tue, 17 Aug 2021 16:17:20 -0700 Subject: [PATCH 02/15] 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 cbf592b907773..96abbf3999e6a 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 c283475c0d86e..1accded7ef619 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 42c9c8af5bfae..9d898027a9a60 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 e31a6ec4752ad..1890972558a0a 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 0000000000000..c0bb8132cbbb1 --- /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 a7dac8c8e88f5..e6c6c23ce536b 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 d83b721e7b72b..27ea9f870e175 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 f249fb6df6d9a63b82fa7df1a835bade5fb5f767 Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Tue, 17 Aug 2021 17:00:36 -0700 Subject: [PATCH 03/15] 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 1accded7ef619..ad3e61f9e02b6 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 9d898027a9a60..3466cd57b34e0 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 1890972558a0a..7add0bbe8f9c9 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 63d221028140efc4eba521c3df9b9753a562163b Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Wed, 18 Aug 2021 09:42:57 -0700 Subject: [PATCH 04/15] 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 96abbf3999e6a..792eef1464e4a 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 ad3e61f9e02b6..c7020087ca10f 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 c434652c3aee8..a813535984efb 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 7add0bbe8f9c9..0571e6eddd617 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 c0bb8132cbbb1..cfa803cc6e160 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 e6c6c23ce536b..c655b4f0aa33b 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 dcae3044f31d8ca47555eadd7827b8fe00101956 Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Wed, 18 Aug 2021 09:45:40 -0700 Subject: [PATCH 05/15] 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 3466cd57b34e0..42c9c8af5bfae 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 90739ec7f9c5b6f185944127881a1d5c4e1a2cb6 Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Thu, 19 Aug 2021 17:50:04 -0700 Subject: [PATCH 06/15] 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 792eef1464e4a..940c912b95bf9 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 c7020087ca10f..9a69c025bd5c8 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 a813535984efb..c434652c3aee8 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 0571e6eddd617..8306f60adc43a 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 e770e6e43271f..1b778479247eb 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 cfa803cc6e160..0000000000000 --- 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 c655b4f0aa33b..440270a002c6a 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 2a12a3c5b376b..04dcf1ba7c489 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 3e721954d7ac698c37d591873d42e68a42c397fb Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Fri, 20 Aug 2021 11:23:10 -0700 Subject: [PATCH 07/15] 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 940c912b95bf9..cbf592b907773 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 8306f60adc43a..47d6e672e1c88 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 04dcf1ba7c489..60cd1c70b9a83 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 ff0c952855ac0e95f92d0ce973b9b8cf37ad3871 Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Fri, 20 Aug 2021 17:02:58 -0700 Subject: [PATCH 08/15] 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 79b58e6219a35..f83422919ef61 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); From 2338b564b5ca01c0a3a8bb803d7f6f2ec1e68896 Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Mon, 23 Aug 2021 09:33:40 -0700 Subject: [PATCH 09/15] put Message on Response instead of classifier --- .../Azure.Core/src/Pipeline/HttpPipeline.cs | 4 +-- sdk/core/Azure.Core/src/Response.cs | 14 ++++++-- .../src/Models/SynapseRoleAssignment.cs | 18 +++++----- .../tests/AccessControlClientLiveTests.cs | 33 ++++++++++++------- 4 files changed, 44 insertions(+), 25 deletions(-) diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs index 9a69c025bd5c8..b28244d63f788 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs @@ -71,7 +71,7 @@ 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 = ResponseClassifier; + message.Response.Message = message; return value; } @@ -85,7 +85,7 @@ 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.Message = message; } /// /// Invokes the pipeline asynchronously with the provided request. diff --git a/sdk/core/Azure.Core/src/Response.cs b/sdk/core/Azure.Core/src/Response.cs index 47d6e672e1c88..4f64ecb830b53 100644 --- a/sdk/core/Azure.Core/src/Response.cs +++ b/sdk/core/Azure.Core/src/Response.cs @@ -113,14 +113,22 @@ public virtual BinaryData Content /// The enumerating in the response. protected internal abstract IEnumerable EnumerateHeaders(); - internal ResponseClassifier? ResponseClassifier { get; set; } + internal HttpMessage? Message { get; set; } + + /// + /// + /// + public bool IsError() + { + return this.Message!.ResponseClassifier.IsErrorResponse(this.Message); + } /// /// Throw a RequestFailedException appropriate to the Response. /// public void Throw() { - throw this.ResponseClassifier!.CreateRequestFailedException(this); + throw this.Message!.ResponseClassifier.CreateRequestFailedException(this); //throw new RequestFailedException(""); } @@ -129,7 +137,7 @@ public void Throw() /// public async Task ThrowAsync() { - throw await this.ResponseClassifier!.CreateRequestFailedExceptionAsync(this).ConfigureAwait(false); + throw await this.Message!.ResponseClassifier.CreateRequestFailedExceptionAsync(this).ConfigureAwait(false); //throw new RequestFailedException(""); } 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 27ea9f870e175..155eed6ec8451 100644 --- a/sdk/synapse/Azure.Analytics.Synapse.AccessControl/src/Models/SynapseRoleAssignment.cs +++ b/sdk/synapse/Azure.Analytics.Synapse.AccessControl/src/Models/SynapseRoleAssignment.cs @@ -44,19 +44,19 @@ public static implicit operator RequestContent(SynapseRoleAssignment value) => R public static implicit operator SynapseRoleAssignment(Response response) { - switch (response.Status) + if (!response.IsError()) { - case 200: - return DeserializeResponse(response); - default: + return DeserializeResponse(response); + } + else + { + response.Throw(); // What to do about Async here? Can you put awaits in operators? + } + // we should never get here, since we throw in the else statement above. #pragma warning disable CA1065 // Do not raise exceptions in unexpected locations - throw new NotImplementedException(); + throw new NotSupportedException(); #pragma warning restore CA1065 // Do not raise exceptions in unexpected locations - - //response.Throw(); - //break; - } } private static SynapseRoleAssignment DeserializeResponse(Response response) diff --git a/sdk/synapse/Azure.Analytics.Synapse.AccessControl/tests/AccessControlClientLiveTests.cs b/sdk/synapse/Azure.Analytics.Synapse.AccessControl/tests/AccessControlClientLiveTests.cs index f83422919ef61..abbc4a2a099c4 100644 --- a/sdk/synapse/Azure.Analytics.Synapse.AccessControl/tests/AccessControlClientLiveTests.cs +++ b/sdk/synapse/Azure.Analytics.Synapse.AccessControl/tests/AccessControlClientLiveTests.cs @@ -87,18 +87,29 @@ public async Task CreateRoleAssignment_ModelTypeParameterOverload() SynapseRoleAssignment roleAssignment = new SynapseRoleAssignment(roleId, principalId, scope); - SynapseRoleAssignment returnedRoleAssignment = await client.CreateRoleAssignmentAsync(roleAssignmentId, roleAssignment, new RequestOptions() + // Calling the LLC directly: + Response response = await client.CreateRoleAssignmentAsync(roleAssignmentId, roleAssignment, + new RequestOptions() + { + StatusOption = ResponseStatusOption.NoThrow + }); + + // This is the implicit cast -- it will throw if the response is an error + // according to the classifier + SynapseRoleAssignment returnedRoleAssignment = response; + + // But since we suppressed the error, we might want to think + // about it the following way: + if (response.IsError()) { - StatusOption = ResponseStatusOption.NoThrow - }); - - // TODO: Finish this test and figure out the rest. - - await using DisposableClientRole role = await DisposableClientRole.Create(client, TestEnvironment); - - Assert.NotNull(role.Assignment.Id); - Assert.NotNull(role.Assignment.Properties.RoleDefinitionId); - Assert.NotNull(role.Assignment.Properties.PrincipalId); + await response.ThrowAsync(); + } + else + { + Assert.NotNull(returnedRoleAssignment.Id); + Assert.NotNull(returnedRoleAssignment.Properties.RoleDefinitionId); + Assert.NotNull(returnedRoleAssignment.Properties.PrincipalId); + } } [Test] From c9ae9f04c70e7f07bf02a73d43e546851a31cd49 Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Mon, 23 Aug 2021 14:34:18 -0700 Subject: [PATCH 10/15] move to ThrowIfError semantics --- sdk/core/Azure.Core/src/Response.cs | 30 +++++++++---------- .../Generated/SynapseAccessControlClient.cs | 27 ++++++++--------- .../src/Models/SynapseRoleAssignment.cs | 15 ++-------- .../tests/AccessControlClientLiveTests.cs | 17 +++++------ 4 files changed, 35 insertions(+), 54 deletions(-) diff --git a/sdk/core/Azure.Core/src/Response.cs b/sdk/core/Azure.Core/src/Response.cs index 4f64ecb830b53..ba151f194794a 100644 --- a/sdk/core/Azure.Core/src/Response.cs +++ b/sdk/core/Azure.Core/src/Response.cs @@ -116,29 +116,27 @@ public virtual BinaryData Content internal HttpMessage? Message { get; set; } /// + /// Throw a RequestFailedException appropriate to the Response if the response classifer determines + /// this is response represents an error. /// - /// - public bool IsError() + public void ThrowIfError() { - return this.Message!.ResponseClassifier.IsErrorResponse(this.Message); - } - - /// - /// Throw a RequestFailedException appropriate to the Response. - /// - public void Throw() - { - throw this.Message!.ResponseClassifier.CreateRequestFailedException(this); - //throw new RequestFailedException(""); + if (this.Message!.ResponseClassifier.IsErrorResponse(this.Message)) + { + throw this.Message!.ResponseClassifier.CreateRequestFailedException(this); + } } /// - /// Throw a RequestFailedException appropriate to the Response. + /// Throw a RequestFailedException appropriate to the Response if the response classifer determines + /// this is response represents an error. /// - public async Task ThrowAsync() + public async Task ThrowIfErrorAsync() { - throw await this.Message!.ResponseClassifier.CreateRequestFailedExceptionAsync(this).ConfigureAwait(false); - //throw new RequestFailedException(""); + if (this.Message!.ResponseClassifier.IsErrorResponse(this.Message)) + { + throw await this.Message!.ResponseClassifier.CreateRequestFailedExceptionAsync(this).ConfigureAwait(false); + } } /// 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 60cd1c70b9a83..5666338325d03 100644 --- a/sdk/synapse/Azure.Analytics.Synapse.AccessControl/src/Generated/SynapseAccessControlClient.cs +++ b/sdk/synapse/Azure.Analytics.Synapse.AccessControl/src/Generated/SynapseAccessControlClient.cs @@ -463,10 +463,15 @@ public virtual async Task CreateRoleAssignmentAsync(string roleAssignm { options ??= new RequestOptions(); HttpMessage message = CreateCreateRoleAssignmentRequest(roleAssignmentId, content, options); - if (options.PerCallPolicy != null) - { - message.SetProperty("RequestOptionsPerCallPolicyCallback", options.PerCallPolicy); - } + + // The following: + // 1. Optionally adds a policy to this pipeline for the scope of the message + // 2. Overwrites this message's ResponseClassifier with classification options set on RequestOptions + // + // Note: we are essentially configuring the pipeline for the request and response + // before we call Pipeline.SendAsync(); + RequestOptions.Apply(options, message); + using var scope0 = _clientDiagnostics.CreateScope("SynapseAccessControlClient.CreateRoleAssignment"); scope0.Start(); try @@ -474,18 +479,10 @@ public virtual async Task CreateRoleAssignmentAsync(string roleAssignm await Pipeline.SendAsync(message, options.CancellationToken).ConfigureAwait(false); if (options.StatusOption == ResponseStatusOption.Default) { - switch (message.Response.Status) - { - case 200: - return message.Response; - default: - throw await _responseClassifier.CreateRequestFailedExceptionAsync(message.Response).ConfigureAwait(false); - } - } - else - { - return message.Response; + await message.Response.ThrowIfErrorAsync().ConfigureAwait(false); } + + return message.Response; } catch (Exception e) { 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 155eed6ec8451..20bf1bc88b1a0 100644 --- a/sdk/synapse/Azure.Analytics.Synapse.AccessControl/src/Models/SynapseRoleAssignment.cs +++ b/sdk/synapse/Azure.Analytics.Synapse.AccessControl/src/Models/SynapseRoleAssignment.cs @@ -44,19 +44,8 @@ public static implicit operator RequestContent(SynapseRoleAssignment value) => R public static implicit operator SynapseRoleAssignment(Response response) { - if (!response.IsError()) - { - return DeserializeResponse(response); - } - else - { - response.Throw(); // What to do about Async here? Can you put awaits in operators? - } - - // we should never get here, since we throw in the else statement above. -#pragma warning disable CA1065 // Do not raise exceptions in unexpected locations - throw new NotSupportedException(); -#pragma warning restore CA1065 // Do not raise exceptions in unexpected locations + response.ThrowIfError(); // What about async? + return DeserializeResponse(response); } private static SynapseRoleAssignment DeserializeResponse(Response response) diff --git a/sdk/synapse/Azure.Analytics.Synapse.AccessControl/tests/AccessControlClientLiveTests.cs b/sdk/synapse/Azure.Analytics.Synapse.AccessControl/tests/AccessControlClientLiveTests.cs index abbc4a2a099c4..c402ac4d324ec 100644 --- a/sdk/synapse/Azure.Analytics.Synapse.AccessControl/tests/AccessControlClientLiveTests.cs +++ b/sdk/synapse/Azure.Analytics.Synapse.AccessControl/tests/AccessControlClientLiveTests.cs @@ -100,16 +100,13 @@ public async Task CreateRoleAssignment_ModelTypeParameterOverload() // But since we suppressed the error, we might want to think // about it the following way: - if (response.IsError()) - { - await response.ThrowAsync(); - } - else - { - Assert.NotNull(returnedRoleAssignment.Id); - Assert.NotNull(returnedRoleAssignment.Properties.RoleDefinitionId); - Assert.NotNull(returnedRoleAssignment.Properties.PrincipalId); - } + + await response.ThrowIfErrorAsync(); + + // else + Assert.NotNull(returnedRoleAssignment.Id); + Assert.NotNull(returnedRoleAssignment.Properties.RoleDefinitionId); + Assert.NotNull(returnedRoleAssignment.Properties.PrincipalId); } [Test] From d529f6e0b74a4637d1d24648270faf94a8f03efb Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Mon, 23 Aug 2021 15:34:39 -0700 Subject: [PATCH 11/15] optionally cache error --- .../src/RequestOptions.cs | 4 +- sdk/core/Azure.Core/src/HttpMessage.cs | 5 ++ .../Azure.Core/src/Pipeline/HttpPipeline.cs | 5 +- sdk/core/Azure.Core/src/Response.cs | 56 ++++++++++++++++--- 4 files changed, 59 insertions(+), 11 deletions(-) diff --git a/sdk/core/Azure.Core.Experimental/src/RequestOptions.cs b/sdk/core/Azure.Core.Experimental/src/RequestOptions.cs index a529e58a12ae3..e520fb2c8efb6 100644 --- a/sdk/core/Azure.Core.Experimental/src/RequestOptions.cs +++ b/sdk/core/Azure.Core.Experimental/src/RequestOptions.cs @@ -115,6 +115,8 @@ public static void Apply(RequestOptions requestOptions, HttpMessage message) { message.ResponseClassifier = new PerCallResponseClassifier(message.ResponseClassifier, requestOptions._classifiers); } + + message.CacheError = requestOptions.StatusOption == ResponseStatusOption.NoThrow; } /// @@ -236,4 +238,4 @@ public enum ResponseClassification /// Success, } -} \ No newline at end of file +} diff --git a/sdk/core/Azure.Core/src/HttpMessage.cs b/sdk/core/Azure.Core/src/HttpMessage.cs index 6a16bb58e6941..a5fc5a9242bba 100644 --- a/sdk/core/Azure.Core/src/HttpMessage.cs +++ b/sdk/core/Azure.Core/src/HttpMessage.cs @@ -70,6 +70,11 @@ public Response Response /// public ResponseClassifier ResponseClassifier { get; set; } + /// + /// We can make this internal if RequestOptions moves out of Core.Experimental into Core. + /// + public bool CacheError { get; set; } + /// /// Gets or sets the value indicating if response would be buffered as part of the pipeline. Defaults to true. /// diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs index b28244d63f788..737ea638460f1 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs @@ -71,7 +71,7 @@ public ValueTask SendAsync(HttpMessage message, CancellationToken cancellationTo message.CancellationToken = cancellationToken; AddHttpMessageProperties(message); var value = _pipeline.Span[0].ProcessAsync(message, _pipeline.Slice(1)); - message.Response.Message = message; + message.Response.ApplyErrorSettings(message); // Async variant? return value; } @@ -85,8 +85,9 @@ public void Send(HttpMessage message, CancellationToken cancellationToken) message.CancellationToken = cancellationToken; AddHttpMessageProperties(message); _pipeline.Span[0].Process(message, _pipeline.Slice(1)); - message.Response.Message = message; + message.Response.ApplyErrorSettings(message); } + /// /// Invokes the pipeline asynchronously with the provided request. /// diff --git a/sdk/core/Azure.Core/src/Response.cs b/sdk/core/Azure.Core/src/Response.cs index ba151f194794a..da3a2741c393c 100644 --- a/sdk/core/Azure.Core/src/Response.cs +++ b/sdk/core/Azure.Core/src/Response.cs @@ -113,17 +113,47 @@ public virtual BinaryData Content /// The enumerating in the response. protected internal abstract IEnumerable EnumerateHeaders(); + /// + /// Note: this will be disposed after the service method completes. + /// internal HttpMessage? Message { get; set; } + // Note: these are better cached on the ResponseClassifier itself. + private bool? _isError; + private RequestFailedException? _requestFailedException; + /// - /// Throw a RequestFailedException appropriate to the Response if the response classifer determines - /// this is response represents an error. + /// Since the information needed to determine whether the response is an error gets disposed + /// after the service call, we'll want to cache it while we have the message in scope. /// - public void ThrowIfError() + /// + internal void ApplyErrorSettings(HttpMessage message) + { + if (message.CacheError) + { + _isError = message.ResponseClassifier.IsErrorResponse(message); + if (_isError!.Value) + { + _requestFailedException = message.ResponseClassifier.CreateRequestFailedException(this); + } + } + else + { + Message = message; + } + } + + /// + /// Since the information needed to determine whether the response is an error gets disposed + /// after the service call, we'll want to cache it while we have the message in scope. + /// + /// + internal async Task ApplyErrorSettingsAsync(HttpMessage message) { - if (this.Message!.ResponseClassifier.IsErrorResponse(this.Message)) + _isError = message.ResponseClassifier.IsErrorResponse(message); + if (_isError!.Value) { - throw this.Message!.ResponseClassifier.CreateRequestFailedException(this); + _requestFailedException = await message.ResponseClassifier.CreateRequestFailedExceptionAsync(this).ConfigureAwait(false); } } @@ -131,11 +161,21 @@ public void ThrowIfError() /// Throw a RequestFailedException appropriate to the Response if the response classifer determines /// this is response represents an error. /// - public async Task ThrowIfErrorAsync() + public void ThrowIfError() { - if (this.Message!.ResponseClassifier.IsErrorResponse(this.Message)) + if (!_isError.HasValue) { - throw await this.Message!.ResponseClassifier.CreateRequestFailedExceptionAsync(this).ConfigureAwait(false); + if (Message!.ResponseClassifier.IsErrorResponse(Message)) + { + throw Message.ResponseClassifier.CreateRequestFailedException(this); + } + } + else + { + if (_isError.Value) + { + throw _requestFailedException!; + } } } From 1589ac4b5966bc00377b9129c6f917e660022859 Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Mon, 23 Aug 2021 16:27:10 -0700 Subject: [PATCH 12/15] updates to cache implementation --- .../src/RequestOptions.cs | 2 - sdk/core/Azure.Core/src/HttpMessage.cs | 5 -- .../Azure.Core/src/Pipeline/HttpPipeline.cs | 4 +- sdk/core/Azure.Core/src/Response.cs | 58 +++++++------------ 4 files changed, 24 insertions(+), 45 deletions(-) diff --git a/sdk/core/Azure.Core.Experimental/src/RequestOptions.cs b/sdk/core/Azure.Core.Experimental/src/RequestOptions.cs index e520fb2c8efb6..a9ac22126eb44 100644 --- a/sdk/core/Azure.Core.Experimental/src/RequestOptions.cs +++ b/sdk/core/Azure.Core.Experimental/src/RequestOptions.cs @@ -115,8 +115,6 @@ public static void Apply(RequestOptions requestOptions, HttpMessage message) { message.ResponseClassifier = new PerCallResponseClassifier(message.ResponseClassifier, requestOptions._classifiers); } - - message.CacheError = requestOptions.StatusOption == ResponseStatusOption.NoThrow; } /// diff --git a/sdk/core/Azure.Core/src/HttpMessage.cs b/sdk/core/Azure.Core/src/HttpMessage.cs index a5fc5a9242bba..6a16bb58e6941 100644 --- a/sdk/core/Azure.Core/src/HttpMessage.cs +++ b/sdk/core/Azure.Core/src/HttpMessage.cs @@ -70,11 +70,6 @@ public Response Response /// public ResponseClassifier ResponseClassifier { get; set; } - /// - /// We can make this internal if RequestOptions moves out of Core.Experimental into Core. - /// - public bool CacheError { get; set; } - /// /// Gets or sets the value indicating if response would be buffered as part of the pipeline. Defaults to true. /// diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs index 737ea638460f1..2e41a8fad7442 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipeline.cs @@ -71,7 +71,7 @@ public ValueTask SendAsync(HttpMessage message, CancellationToken cancellationTo message.CancellationToken = cancellationToken; AddHttpMessageProperties(message); var value = _pipeline.Span[0].ProcessAsync(message, _pipeline.Slice(1)); - message.Response.ApplyErrorSettings(message); // Async variant? + message.Response.EvaluateError(message); return value; } @@ -85,7 +85,7 @@ public void Send(HttpMessage message, CancellationToken cancellationToken) message.CancellationToken = cancellationToken; AddHttpMessageProperties(message); _pipeline.Span[0].Process(message, _pipeline.Slice(1)); - message.Response.ApplyErrorSettings(message); + message.Response.EvaluateError(message); } /// diff --git a/sdk/core/Azure.Core/src/Response.cs b/sdk/core/Azure.Core/src/Response.cs index da3a2741c393c..4ecae8fab278a 100644 --- a/sdk/core/Azure.Core/src/Response.cs +++ b/sdk/core/Azure.Core/src/Response.cs @@ -113,47 +113,38 @@ public virtual BinaryData Content /// The enumerating in the response. protected internal abstract IEnumerable EnumerateHeaders(); - /// - /// Note: this will be disposed after the service method completes. - /// - internal HttpMessage? Message { get; set; } + internal bool? IsError { get; set; } - // Note: these are better cached on the ResponseClassifier itself. - private bool? _isError; - private RequestFailedException? _requestFailedException; + internal ResponseClassifier? ResponseClassifier { get; set; } /// /// Since the information needed to determine whether the response is an error gets disposed /// after the service call, we'll want to cache it while we have the message in scope. /// /// - internal void ApplyErrorSettings(HttpMessage message) + internal void EvaluateError(HttpMessage message) { - if (message.CacheError) - { - _isError = message.ResponseClassifier.IsErrorResponse(message); - if (_isError!.Value) - { - _requestFailedException = message.ResponseClassifier.CreateRequestFailedException(this); - } - } - else + if (!IsError.HasValue) { - Message = message; + IsError = message.ResponseClassifier.IsErrorResponse(message); + ResponseClassifier = message.ResponseClassifier; } } /// - /// Since the information needed to determine whether the response is an error gets disposed - /// after the service call, we'll want to cache it while we have the message in scope. + /// Throw a RequestFailedException appropriate to the Response if the response classifer determines + /// this is response represents an error. /// - /// - internal async Task ApplyErrorSettingsAsync(HttpMessage message) + public void ThrowIfError() { - _isError = message.ResponseClassifier.IsErrorResponse(message); - if (_isError!.Value) + if (!IsError.HasValue) { - _requestFailedException = await message.ResponseClassifier.CreateRequestFailedExceptionAsync(this).ConfigureAwait(false); + throw new InvalidOperationException("Error value should have been cached by the pipeline."); + } + + if (IsError.Value) + { + throw ResponseClassifier!.CreateRequestFailedException(this); } } @@ -161,21 +152,16 @@ internal async Task ApplyErrorSettingsAsync(HttpMessage message) /// Throw a RequestFailedException appropriate to the Response if the response classifer determines /// this is response represents an error. /// - public void ThrowIfError() + public async Task ThrowIfErrorAsync() { - if (!_isError.HasValue) + if (!IsError.HasValue) { - if (Message!.ResponseClassifier.IsErrorResponse(Message)) - { - throw Message.ResponseClassifier.CreateRequestFailedException(this); - } + throw new InvalidOperationException("Error value should have been cached by the pipeline."); } - else + + if (IsError.Value) { - if (_isError.Value) - { - throw _requestFailedException!; - } + throw await ResponseClassifier!.CreateRequestFailedExceptionAsync(this).ConfigureAwait(false); } } From 17dc878c2895239f8edf6c628a51f28c58f96a56 Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Mon, 23 Aug 2021 16:32:09 -0700 Subject: [PATCH 13/15] nits --- sdk/core/Azure.Core.Experimental/src/RequestOptions.cs | 2 +- sdk/core/Azure.Core/src/Response.cs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sdk/core/Azure.Core.Experimental/src/RequestOptions.cs b/sdk/core/Azure.Core.Experimental/src/RequestOptions.cs index a9ac22126eb44..a529e58a12ae3 100644 --- a/sdk/core/Azure.Core.Experimental/src/RequestOptions.cs +++ b/sdk/core/Azure.Core.Experimental/src/RequestOptions.cs @@ -236,4 +236,4 @@ public enum ResponseClassification /// Success, } -} +} \ No newline at end of file diff --git a/sdk/core/Azure.Core/src/Response.cs b/sdk/core/Azure.Core/src/Response.cs index 4ecae8fab278a..15bd54ca43459 100644 --- a/sdk/core/Azure.Core/src/Response.cs +++ b/sdk/core/Azure.Core/src/Response.cs @@ -139,7 +139,7 @@ public void ThrowIfError() { if (!IsError.HasValue) { - throw new InvalidOperationException("Error value should have been cached by the pipeline."); + throw new InvalidOperationException("IsError value should have been cached by the pipeline."); } if (IsError.Value) @@ -156,7 +156,7 @@ public async Task ThrowIfErrorAsync() { if (!IsError.HasValue) { - throw new InvalidOperationException("Error value should have been cached by the pipeline."); + throw new InvalidOperationException("IsError value should have been cached by the pipeline."); } if (IsError.Value) From 7edca7a694c71daa24e32cbd49a57fa4e7f23f80 Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Tue, 24 Aug 2021 08:38:43 -0700 Subject: [PATCH 14/15] implement deserialize --- sdk/core/Azure.Core/src/Response.cs | 11 ++--------- .../src/Models/SynapseRoleAssignment.cs | 8 +++++++- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/sdk/core/Azure.Core/src/Response.cs b/sdk/core/Azure.Core/src/Response.cs index 15bd54ca43459..65ea569f9d17b 100644 --- a/sdk/core/Azure.Core/src/Response.cs +++ b/sdk/core/Azure.Core/src/Response.cs @@ -117,11 +117,6 @@ public virtual BinaryData Content internal ResponseClassifier? ResponseClassifier { get; set; } - /// - /// Since the information needed to determine whether the response is an error gets disposed - /// after the service call, we'll want to cache it while we have the message in scope. - /// - /// internal void EvaluateError(HttpMessage message) { if (!IsError.HasValue) @@ -132,8 +127,7 @@ internal void EvaluateError(HttpMessage message) } /// - /// Throw a RequestFailedException appropriate to the Response if the response classifer determines - /// this is response represents an error. + /// If the response is an error response, throw a RequestFailedException. /// public void ThrowIfError() { @@ -149,8 +143,7 @@ public void ThrowIfError() } /// - /// Throw a RequestFailedException appropriate to the Response if the response classifer determines - /// this is response represents an error. + /// If the response is an error response, throw a RequestFailedException. /// public async Task ThrowIfErrorAsync() { 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 20bf1bc88b1a0..7e668fe285530 100644 --- a/sdk/synapse/Azure.Analytics.Synapse.AccessControl/src/Models/SynapseRoleAssignment.cs +++ b/sdk/synapse/Azure.Analytics.Synapse.AccessControl/src/Models/SynapseRoleAssignment.cs @@ -2,6 +2,7 @@ // Licensed under the MIT License. using System; +using System.Text.Json; using Azure.Core; namespace Azure.Analytics.Synapse.AccessControl @@ -50,7 +51,12 @@ public static implicit operator SynapseRoleAssignment(Response response) private static SynapseRoleAssignment DeserializeResponse(Response response) { - throw new NotImplementedException(); + JsonDocument roleAssignment = JsonDocument.Parse(response.Content.ToMemory()); + return new SynapseRoleAssignment( + roleAssignment.RootElement.GetProperty("id").GetGuid(), + roleAssignment.RootElement.GetProperty("principalId").GetGuid(), + roleAssignment.RootElement.GetProperty("roleDefinitionId").GetGuid(), + roleAssignment.RootElement.GetProperty("scope").ToString()); } } } From df94256002f6f57a986d470465c46be16860e83d Mon Sep 17 00:00:00 2001 From: Anne Thompson Date: Tue, 24 Aug 2021 13:16:15 -0700 Subject: [PATCH 15/15] updates to ResponseClassifier API --- .../Azure.Core/src/Pipeline/AzureError.cs | 37 +++++ sdk/core/Azure.Core/src/ResponseClassifier.cs | 152 ++++++++++-------- 2 files changed, 126 insertions(+), 63 deletions(-) create mode 100644 sdk/core/Azure.Core/src/Pipeline/AzureError.cs diff --git a/sdk/core/Azure.Core/src/Pipeline/AzureError.cs b/sdk/core/Azure.Core/src/Pipeline/AzureError.cs new file mode 100644 index 0000000000000..6a7a3b1519b74 --- /dev/null +++ b/sdk/core/Azure.Core/src/Pipeline/AzureError.cs @@ -0,0 +1,37 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +using System.Text; + +namespace Azure.Core.Pipeline +{ + /// + /// + public class AzureError + { + /// + /// + /// + public AzureError() + { + Data = new Dictionary(); + } + + /// + /// Gets or sets the error message. + /// + public string? Message { get; set; } + + /// + /// Gets or sets the error code. + /// + public string? ErrorCode { get; set; } + + /// + /// Gets an additional data returned with the error response. + /// + public IDictionary Data { get; } + } +} diff --git a/sdk/core/Azure.Core/src/ResponseClassifier.cs b/sdk/core/Azure.Core/src/ResponseClassifier.cs index 1b778479247eb..54d6226975acc 100644 --- a/sdk/core/Azure.Core/src/ResponseClassifier.cs +++ b/sdk/core/Azure.Core/src/ResponseClassifier.cs @@ -90,109 +90,108 @@ public virtual bool IsErrorResponse(HttpMessage message) } /// + /// Creates an instance of for the provided failed . /// - public async ValueTask CreateRequestFailedExceptionAsync(Response response, string? message = null, string? errorCode = null, IDictionary? additionalInfo = null, Exception? innerException = null) + /// + /// + /// + /// + public virtual async ValueTask CreateRequestFailedExceptionAsync( + Response response, + AzureError? error = 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); + return CreateRequestFailedExceptionWithContent(response, content, error, innerException); } /// + /// Creates an instance of for the provided failed . /// - public RequestFailedException CreateRequestFailedException(Response response, string? message = null, string? errorCode = null, IDictionary? additionalInfo = null, Exception? innerException = null) + /// + /// + /// + /// + public virtual RequestFailedException CreateRequestFailedException( + Response response, + AzureError? error = null, + Exception? innerException = null) { string? content = ReadContentAsync(response, false).EnsureCompleted(); - ExtractFailureContent(content, ref message, ref errorCode); - return CreateRequestFailedExceptionWithContent(response, message, content, errorCode, additionalInfo, innerException); + return CreateRequestFailedExceptionWithContent(response, content, error, 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) + /// + /// Partial method that can optionally be defined to extract the error + /// message, code, and details in a service specific manner. + /// + /// The response headers. + /// The extracted text content + protected virtual AzureError? ExtractFailureContent(Response response, string? textContent) { try { // Optimistic check for JSON object we expect - if (content == null || - !content.StartsWith("{", StringComparison.OrdinalIgnoreCase)) - return; + if (textContent == null || + !textContent.StartsWith("{", StringComparison.OrdinalIgnoreCase)) + return null; - string? parsedMessage = null; - using JsonDocument document = JsonDocument.Parse(content); + var extractFailureContent = new AzureError(); + + using JsonDocument document = JsonDocument.Parse(textContent); if (document.RootElement.TryGetProperty("error", out var errorProperty)) { if (errorProperty.TryGetProperty("code", out var codeProperty)) { - errorCode = codeProperty.GetString(); + extractFailureContent.ErrorCode = codeProperty.GetString(); } if (errorProperty.TryGetProperty("message", out var messageProperty)) { - parsedMessage = messageProperty.GetString(); + extractFailureContent.Message = messageProperty.GetString(); } } - // Make sure we parsed a message so we don't overwrite the value with null - if (parsedMessage != null) - { - message = parsedMessage; - } + return extractFailureContent; } catch (Exception) { // Ignore any failures - unexpected content will be // included verbatim in the detailed error message } + + return null; } private RequestFailedException CreateRequestFailedExceptionWithContent( Response response, - string? message = null, - string? content = null, - string? errorCode = null, - IDictionary? additionalInfo = null, - Exception? innerException = null) + string? content, + AzureError? details, + Exception? innerException) { - var formatMessage = CreateRequestFailedMessageWithContent(response, message, content, errorCode, additionalInfo); - var exception = new RequestFailedException(response.Status, formatMessage, errorCode, innerException); + var errorInformation = ExtractFailureContent(response, content); - if (additionalInfo != null) + var message = details?.Message ?? errorInformation?.Message ?? DefaultFailureMessage; + var errorCode = details?.ErrorCode ?? errorInformation?.ErrorCode; + + IDictionary? data = null; + if (errorInformation?.Data != null) { - foreach (KeyValuePair keyValuePair in additionalInfo) + if (details?.Data == null) { - exception.Data.Add(keyValuePair.Key, keyValuePair.Value); + data = errorInformation.Data; + } + else + { + data = new Dictionary(details.Data); + foreach (var pair in errorInformation.Data) + { + data[pair.Key] = pair.Value; + } } } - return exception; - } - - private string CreateRequestFailedMessageWithContent( - Response response, - string? message, - string? content, - string? errorCode, - IDictionary? additionalInfo) - { StringBuilder messageBuilder = new StringBuilder() - .AppendLine(message ?? DefaultFailureMessage) + .AppendLine(message) .Append("Status: ") .Append(response.Status.ToString(CultureInfo.InvariantCulture)); @@ -214,17 +213,18 @@ private string CreateRequestFailedMessageWithContent( .AppendLine(); } - if (additionalInfo != null && additionalInfo.Count > 0) + if (data != null && data.Count > 0) { messageBuilder .AppendLine() .AppendLine("Additional Information:"); - foreach (KeyValuePair info in additionalInfo) + foreach (KeyValuePair info in data) { messageBuilder .Append(info.Key) .Append(": ") - .AppendLine(info.Value); + .Append(info.Value) + .AppendLine(); } } @@ -246,7 +246,33 @@ private string CreateRequestFailedMessageWithContent( messageBuilder.AppendLine($"{responseHeader.Name}: {headerValue}"); } - return messageBuilder.ToString(); + var exception = new RequestFailedException(response.Status, messageBuilder.ToString(), errorCode, innerException); + + if (data != null) + { + foreach (KeyValuePair keyValuePair in data) + { + exception.Data.Add(keyValuePair.Key, keyValuePair.Value); + } + } + + return exception; + } + + 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; } } }