[LLC] Create RequestFailedException from Response

sdk/core/Azure.Core.Experimental/api/Azure.Core.Experimental.netstandard2.0.cs

@@ -142,6 +142,7 @@ namespace Azure.Core.Pipeline
 {
     public static partial class ResponseExtensions
     {
+        public static Azure.RequestFailedException CreateRequestFailedException(this Azure.Response response) { throw null; }
         public static bool IsError(this Azure.Response response) { throw null; }
     }
 }

sdk/core/Azure.Core.Experimental/src/Azure.Core.Experimental.csproj

@@ -1,4 +1,4 @@
-<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
     <Description>Experimental types that might eventually move to Azure.Core</Description>
     <AssemblyTitle>Microsoft Azure Client Pipeline Experimental Extensions</AssemblyTitle>
@@ -16,6 +16,12 @@
 
   <ItemGroup>
     <Compile Include="$(AzureCoreSharedSources)Argument.cs" />
+    <Compile Include="$(AzureCoreSharedSources)AppContextSwitchHelper.cs" />
+    <Compile Include="$(AzureCoreSharedSources)ClientDiagnostics.cs" />
+    <Compile Include="$(AzureCoreSharedSources)ContentTypeUtilities.cs" />
+    <Compile Include="$(AzureCoreSharedSources)DiagnosticScope.cs" />
+    <Compile Include="$(AzureCoreSharedSources)DiagnosticScopeFactory.cs" />
+    <Compile Include="$(AzureCoreSharedSources)HttpMessageSanitizer.cs" />
     <Compile Include="$(AzureCoreSharedSources)HashCodeBuilder.cs" />
     <Compile Include="$(AzureCoreSharedSources)TaskExtensions.cs" />
     <Compile Include="$(AzureCoreSharedSources)NullableAttributes.cs" />

sdk/core/Azure.Core.Experimental/src/ClassifiedResponse.cs

@@ -7,6 +7,7 @@
 using System.Diagnostics.CodeAnalysis;
 using System.IO;
 using System.Text;
+using Azure.Core.Pipeline;
 
 namespace Azure.Core
 {
@@ -23,6 +24,8 @@ public class ClassifiedResponse : Response
         /// </summary>
         public bool IsError { get; private set; }
 
+        internal ExceptionFormattingResponseClassifier? ResponseClassifier { get; set; }
+
         internal void EvaluateError(HttpMessage message)
         {
             IsError = message.ResponseClassifier.IsErrorResponse(message);

sdk/core/Azure.Core.Experimental/src/ExceptionFormattingResponseClassifier.cs

@@ -0,0 +1,35 @@
+// 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
+{
+    /// <summary>
+    /// Wrap ResponseClassifier and add exception formatting methods.
+    /// </summary>
+    internal class ExceptionFormattingResponseClassifier : ResponseClassifier
+    {
+        private ResponseClassifier _responseClassifier;
+
+        internal HttpMessageSanitizer MessageSanitizer { get; set; }
+
+        public override bool IsErrorResponse(HttpMessage message) => _responseClassifier.IsErrorResponse(message);
+
+        public override bool IsRetriable(HttpMessage message, Exception exception) => _responseClassifier.IsRetriable(message, exception);
+
+        public override bool IsRetriableException(Exception exception) => _responseClassifier.IsRetriableException(exception);
+
+        public override bool IsRetriableResponse(HttpMessage message) => _responseClassifier.IsRetriableResponse(message);
+
+        /// <summary>
+        /// </summary>
+        public ExceptionFormattingResponseClassifier(ResponseClassifier responseClassifier, DiagnosticsOptions diagnostics)
+        {
+            _responseClassifier = responseClassifier;
+            MessageSanitizer = ClientDiagnostics.CreateMessageSanitizer(diagnostics);
+        }
+    }
+}

sdk/core/Azure.Core.Experimental/src/ResponseExtensions.cs

@@ -29,5 +29,39 @@ public static bool IsError(this Response response)
 
             return classifiedResponse.IsError;
         }
+
+        /// <summary>
+        /// </summary>
+        /// <param name="response"></param>
+        /// <returns></returns>
+        public static RequestFailedException CreateRequestFailedException(this Response response)
+        {
+            var classifiedResponse = response as ClassifiedResponse;
+
+            if (classifiedResponse == null)
+            {
+                throw new InvalidOperationException("ResponseClassifier was not set on the response. " +
+                    "Please ensure the pipeline includes ResponsePropertiesPolicy.");
+            }
+
+            string message = null;
+            string errorCode = null;
+
+            string content = ClientDiagnostics.ReadContentAsync(response, false).EnsureCompleted();
+            ClientDiagnostics.ExtractAzureErrorContent(content, ref message, ref errorCode);
+            string exceptionMessage = ClientDiagnostics.CreateRequestFailedMessageWithContent(
+                response,
+                message,
+                content,
+                errorCode,
+                null,
+                classifiedResponse.ResponseClassifier.MessageSanitizer);
+
+            return new RequestFailedException(
+                response.Status,
+                exceptionMessage,
+                errorCode,
+                null);
+        }
     }
 }

sdk/core/Azure.Core.Experimental/src/ResponsePropertiesPolicy.cs

@@ -11,6 +11,13 @@ namespace Azure.Core
     /// </summary>
     internal class ResponsePropertiesPolicy : HttpPipelinePolicy
     {
+        private ClientOptions _clientOptions;
+
+        public ResponsePropertiesPolicy(ClientOptions options)
+        {
+            _clientOptions = options;
+        }
+
         /// <inheritdoc/>
         public override void Process(HttpMessage message, ReadOnlyMemory<HttpPipelinePolicy> pipeline)
         {
@@ -23,7 +30,7 @@ public override ValueTask ProcessAsync(HttpMessage message, ReadOnlyMemory<HttpP
             return ProcessAsync(message, pipeline, true);
         }
 
-        private static async ValueTask ProcessAsync(HttpMessage message, ReadOnlyMemory<HttpPipelinePolicy> pipeline, bool async)
+        private async ValueTask ProcessAsync(HttpMessage message, ReadOnlyMemory<HttpPipelinePolicy> pipeline, bool async)
         {
             if (async)
             {
@@ -39,6 +46,10 @@ private static async ValueTask ProcessAsync(HttpMessage message, ReadOnlyMemory<
             ClassifiedResponse response = new ClassifiedResponse(message.Response);
             response.EvaluateError(message);
             message.Response = response;
+
+            // The non-experimental version of this functionality is roughly described in:
+            // https://github.com/Azure/azure-sdk-for-net/pull/24248
+            response.ResponseClassifier = new ExceptionFormattingResponseClassifier(message.ResponseClassifier, _clientOptions.Diagnostics);
         }
     }
 }

sdk/core/Azure.Core.Experimental/tests/Azure.Core.Experimental.Tests.csproj

@@ -23,11 +23,6 @@
   <ItemGroup>
     <Compile Include="$(AzureCoreSharedSources)AppContextSwitchHelper.cs" LinkBase="Shared" />
     <Compile Include="$(AzureCoreSharedSources)ArrayBufferWriter.cs" LinkBase="Shared" />
-    <Compile Include="$(AzureCoreSharedSources)ClientDiagnostics.cs" LinkBase="Shared" />
-    <Compile Include="$(AzureCoreSharedSources)ContentTypeUtilities.cs" LinkBase="Shared" />
-    <Compile Include="$(AzureCoreSharedSources)DiagnosticScope.cs" LinkBase="Shared" />
-    <Compile Include="$(AzureCoreSharedSources)DiagnosticScopeFactory.cs" LinkBase="Shared" />
-    <Compile Include="$(AzureCoreSharedSources)HttpMessageSanitizer.cs" LinkBase="Shared" />
   </ItemGroup>
 
 </Project>

sdk/core/Azure.Core.Experimental/tests/LowLevelClient/LowLevelClientModels/Pet.cs

@@ -35,10 +35,15 @@ public static implicit operator Pet(Response response)
         {
             // [X] TODO: Add in HLC error semantics
             // [X] TODO: Use response.IsError
-            // [ ] TODO: Use throw new ResponseFailedException(response);
+            // [X] TODO: Use throw new ResponseFailedException(response);
+
+            // TODO: When we move this functionality out of Experimental into Core, it will be replaced by
+            // > if (response.IsError)
             if (response.IsError())
             {
-                throw new RequestFailedException("Received a non-success status code.");
+                // TODO: When we move this functionality out of Experimental into Core, it will be replaced by
+                // > throw new RequestFailedException(response);
+                throw response.CreateRequestFailedException();
             }
 
             return DeserializePet(JsonDocument.Parse(response.Content.ToMemory()));

sdk/core/Azure.Core.Experimental/tests/LowLevelClient/PetStoreClient.cs

@@ -58,8 +58,8 @@ public PetStoreClient(Uri endpoint, TokenCredential credential, PetStoreClientOp
             _tokenCredential = credential;
             var authPolicy = new BearerTokenAuthenticationPolicy(_tokenCredential, AuthorizationScopes);
 
-            // When we move the IsError functionality into Core, we'll move the addition of ResponsePropertiesPolicy to the pipeline into HttpPipelineBuilder.
-            Pipeline = HttpPipelineBuilder.Build(options, new HttpPipelinePolicy[] { new LowLevelCallbackPolicy() }, new HttpPipelinePolicy[] { authPolicy, new ResponsePropertiesPolicy() }, new ResponseClassifier());
+            // TODO: When we move the IsError functionality into Core, we'll move the addition of ResponsePropertiesPolicy to the pipeline into HttpPipelineBuilder.
+            Pipeline = HttpPipelineBuilder.Build(options, new HttpPipelinePolicy[] { new LowLevelCallbackPolicy() }, new HttpPipelinePolicy[] { authPolicy, new ResponsePropertiesPolicy(options) }, new ResponseClassifier());
             this.endpoint = endpoint;
             apiVersion = options.Version;
         }

sdk/core/Azure.Core.Experimental/tests/LowLevelClientTests.cs

@@ -91,19 +91,63 @@ public void CannotGetOutputModelOnFailureCodeAsync()
             Assert.ThrowsAsync<RequestFailedException>(async () => await client.GetPetAsync("pet1"));
         }
 
-        #region Helpers
+        // Note: these two tests *currently* test different code paths:
+        // 1) In the ResponseStatusOptions.Default case, we're going through code paths in
+        //    ClientDiagnostics that we'll later migrate to ResponseClassifier (see https://github.com/azure/azure-sdk-for-net/issues/24031)
+        //
+        //    Because this one is thrown from client's `GetPetAsync()` method, where it calls
+        //    _clientDiagnostics.CreateRFException() -- this happens via different constructors (Note: does it have to?  Could we refactor this?)
+        //
+        // 2) In the ResponseStatusOptions.NoThrow case, we're going through code paths in
+        //    ResponseClassifier, which will become the only path after resolution of #24031
+        //
+        // Importantly, having these two tests validates our premise:
+        //   ** The Grow-Up Story/HLC Helper approach has the same semantics
 
-        private void SerializePet(ref Utf8JsonWriter writer, Pet pet)
+        [Test]
+        public async Task GetRequestFailedException_StatusOptionDefault()
         {
-            writer.WriteStartObject();
+            var mockResponse = new MockResponse(404);
+            mockResponse.SetContent("{\"error\": { \"code\": \"MockStatusCode\" }}");
+            mockResponse.AddHeader(HttpHeader.Names.ContentType, "application/json");
 
-            writer.WritePropertyName("name");
-            writer.WriteStringValue(pet.Name);
+            var mockTransport = new MockTransport(mockResponse);
+            PetStoreClient client = CreateClient(mockTransport);
 
-            writer.WritePropertyName("species");
-            writer.WriteStringValue(pet.Species);
+            try
+            {
+                Pet pet = await client.GetPetAsync("pet1");
+            }
+            catch (RequestFailedException e)
+            {
+                Assert.AreEqual(404, e.Status);
+                Assert.That(() => e.Message.StartsWith("Service request failed."));
+                Assert.That(() => e.ErrorCode.StartsWith("MockStatusCode"));
+            }
+        }
 
-            writer.WriteEndObject();
+        [Test]
+        public async Task GetRequestFailedException_StatusOptionNoThrow()
+        {
+            var mockResponse = new MockResponse(404);
+            mockResponse.SetContent("{\"error\": { \"code\": \"MockStatusCode\" }}");
+            mockResponse.AddHeader(HttpHeader.Names.ContentType, "application/json");
+
+            var mockTransport = new MockTransport(mockResponse);
+            PetStoreClient client = CreateClient(mockTransport);
+
+            try
+            {
+                // NOTE: is it weird that we're saying NoThrow here and it throws?
+                // This looks confusing to me as someone reading this code.
+                Pet pet = await client.GetPetAsync("pet1", ResponseStatusOption.NoThrow);
+            }
+            catch (RequestFailedException e)
+            {
+                Assert.AreEqual(404, e.Status);
+                Assert.That(() => e.Message.StartsWith("Service request failed."));
+                Assert.That(() => e.ErrorCode.StartsWith("MockStatusCode"));
+            }
         }
 
         [Test]
@@ -167,6 +211,19 @@ public void ThrowOnErrorThrowsOnError()
             });
         }
 
+        #region Helpers
+        private void SerializePet(ref Utf8JsonWriter writer, Pet pet)
+        {
+            writer.WriteStartObject();
+
+            writer.WritePropertyName("name");
+            writer.WriteStringValue(pet.Name);
+
+            writer.WritePropertyName("species");
+            writer.WriteStringValue(pet.Species);
+
+            writer.WriteEndObject();
+        }
         #endregion
     }
 }

sdk/core/Azure.Core.Experimental/tests/PipelineTests.cs

@@ -26,7 +26,7 @@ public async Task PipelineSetsResponseIsErrorTrue()
             var mockTransport = new MockTransport(
                 new MockResponse(500));
 
-            var pipeline = new HttpPipeline(mockTransport, new[] { new ResponsePropertiesPolicy() });
+            var pipeline = new HttpPipeline(mockTransport, new[] { new ResponsePropertiesPolicy(ClientOptions.Default) });
 
             Request request = pipeline.CreateRequest();
             request.Method = RequestMethod.Get;
@@ -42,7 +42,7 @@ public async Task PipelineSetsResponseIsErrorFalse()
             var mockTransport = new MockTransport(
                 new MockResponse(200));
 
-            var pipeline = new HttpPipeline(mockTransport, new[] { new ResponsePropertiesPolicy() });
+            var pipeline = new HttpPipeline(mockTransport, new[] { new ResponsePropertiesPolicy(ClientOptions.Default) });
 
             Request request = pipeline.CreateRequest();
             request.Method = RequestMethod.Get;
@@ -59,7 +59,7 @@ public async Task CustomClassifierSetsResponseIsError()
                 new MockResponse(404));
 
             var pipeline = new HttpPipeline(mockTransport,
-                new[] { new ResponsePropertiesPolicy() },
+                new[] { new ResponsePropertiesPolicy(ClientOptions.Default) },
                 new CustomResponseClassifier());
 
             Request request = pipeline.CreateRequest();

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

@@ -23,14 +23,20 @@ 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),
             options.Diagnostics.IsDistributedTracingEnabled)
         {
-            _sanitizer = new HttpMessageSanitizer(
-                options.Diagnostics.LoggedQueryParameters.ToArray(),
-                options.Diagnostics.LoggedHeaderNames.ToArray());
+            _sanitizer = CreateMessageSanitizer(options.Diagnostics);
+        }
+
+        internal static HttpMessageSanitizer CreateMessageSanitizer(DiagnosticsOptions diagnostics)
+        {
+            return new HttpMessageSanitizer(
+                diagnostics.LoggedQueryParameters.ToArray(),
+                diagnostics.LoggedHeaderNames.ToArray());
         }
 
         /// <summary>
@@ -48,6 +54,14 @@ protected virtual void ExtractFailureContent(
             ref string? message,
             ref string? errorCode,
             ref IDictionary<string, string>? additionalInfo)
+        {
+            ExtractAzureErrorContent(content, ref message, ref errorCode);
+        }
+
+        internal static void ExtractAzureErrorContent(
+            string? content,
+            ref string? message,
+            ref string? errorCode)
         {
             try
             {
@@ -126,7 +140,14 @@ public async ValueTask<string> CreateRequestFailedMessageAsync(Response response
 
         public string CreateRequestFailedMessageWithContent(Response response, string? message, string? content, string? errorCode, IDictionary<string, string>? additionalInfo)
         {
-            StringBuilder messageBuilder = new StringBuilder()
+            return CreateRequestFailedMessageWithContent(response, message, content, errorCode, additionalInfo, _sanitizer);
+        }
+
+        internal static string CreateRequestFailedMessageWithContent(Response response, string? message, string? content, string? errorCode, IDictionary<string, string>? additionalInfo, HttpMessageSanitizer sanitizer)
+        {
+            StringBuilder messageBuilder = new StringBuilder();
+
+            messageBuilder
                 .AppendLine(message ?? DefaultMessage)
                 .Append("Status: ")
                 .Append(response.Status.ToString(CultureInfo.InvariantCulture));
@@ -177,14 +198,14 @@ public string CreateRequestFailedMessageWithContent(Response response, string? m
 
             foreach (HttpHeader responseHeader in response.Headers)
             {
-                string headerValue = _sanitizer.SanitizeHeader(responseHeader.Name, responseHeader.Value);
+                string headerValue = sanitizer.SanitizeHeader(responseHeader.Name, responseHeader.Value);
                 messageBuilder.AppendLine($"{responseHeader.Name}: {headerValue}");
             }
 
             return messageBuilder.ToString();
         }
 
-        private static async ValueTask<string?> ReadContentAsync(Response response, bool async)
+        internal static async ValueTask<string?> ReadContentAsync(Response response, bool async)
         {
             string? content = null;