Add tests for RequestFailedException constructor (#25578)

* Tests + Bug Fix * initialize sanitizer on response; make IsError virtual * add test * add more tests * update TestFramework to use a project reference * consider parent result * pr fb * This-works * Add Core project reference to EventHubs solution * update API Co-authored-by: Pavel Krymets <[email protected]>
Azure · Dec 1, 2021 · fd9ad86 · fd9ad86
1 parent c1c61f1
commit fd9ad86
Show file tree

Hide file tree

Showing 14 changed files with 145 additions and 15 deletions.
diff --git a/eng/Directory.Build.Common.props b/eng/Directory.Build.Common.props
@@ -135,11 +135,6 @@
     <ProjectReference Include="$(MSBuildThisFileDirectory)/../sdk/resourcemanager/Azure.ResourceManager/src/Azure.ResourceManager.csproj" />
   </ItemGroup>
 
-  <ItemGroup Condition="'$(IsMgmtLibrary)' == 'true'">
-    <PackageReference Include="Azure.Core" />
-    <PackageReference Include="System.Text.Json" />
-  </ItemGroup>
-
   <PropertyGroup>
     <TargetFrameworks>$(RequiredTargetFrameworks)</TargetFrameworks>
 

diff --git a/eng/Directory.Build.Common.targets b/eng/Directory.Build.Common.targets
@@ -181,8 +181,11 @@
   </ItemGroup>
 
   <!--TODO: end-->
+
   <!-- *********** Management Client Library Override section ************* -->
   <ItemGroup Condition="'$(IsMgmtLibrary)' == 'true' and '$(IsTestProject)' != 'true'">
+
+    <PackageReference Include="Azure.Core" />
     <PackageReference Include="System.Text.Json" />
 
     <!-- TODO: Review these file references-->

diff --git a/sdk/core/Azure.Core.TestFramework/src/Azure.Core.TestFramework.csproj b/sdk/core/Azure.Core.TestFramework/src/Azure.Core.TestFramework.csproj
@@ -4,7 +4,7 @@
   </PropertyGroup>
 
   <ItemGroup>
-    <PackageReference Include="Azure.Core" />
+    <ProjectReference Include="../../Azure.Core/src/Azure.Core.csproj" />
     <PackageReference Include="Azure.Identity" />
     <PackageReference Include="Newtonsoft.Json" />
     <PackageReference Include="NUnit" />

diff --git a/sdk/core/Azure.Core.TestFramework/src/MockResponse.cs b/sdk/core/Azure.Core.TestFramework/src/MockResponse.cs
@@ -27,6 +27,10 @@ public MockResponse(int status, string reasonPhrase = null)
 
         public override string ClientRequestId { get; set; }
 
+        private bool? _isError;
+        public override bool IsError { get => _isError ?? base.IsError; }
+        public void SetIsError(bool value) => _isError = value;
+
         public bool IsDisposed { get; private set; }
 
         public void SetContent(byte[] content)

diff --git a/sdk/core/Azure.Core/api/Azure.Core.net461.cs b/sdk/core/Azure.Core/api/Azure.Core.net461.cs
@@ -217,7 +217,7 @@ protected Response() { }
         public virtual System.BinaryData Content { get { throw null; } }
         public abstract System.IO.Stream? ContentStream { get; set; }
         public virtual Azure.Core.ResponseHeaders Headers { get { throw null; } }
-        public bool IsError { get { throw null; } }
+        public virtual bool IsError { get { throw null; } }
         public abstract string ReasonPhrase { get; }
         public abstract int Status { get; }
         protected internal abstract bool ContainsHeader(string name);

diff --git a/sdk/core/Azure.Core/api/Azure.Core.net5.0.cs b/sdk/core/Azure.Core/api/Azure.Core.net5.0.cs
@@ -217,7 +217,7 @@ protected Response() { }
         public virtual System.BinaryData Content { get { throw null; } }
         public abstract System.IO.Stream? ContentStream { get; set; }
         public virtual Azure.Core.ResponseHeaders Headers { get { throw null; } }
-        public bool IsError { get { throw null; } }
+        public virtual bool IsError { get { throw null; } }
         public abstract string ReasonPhrase { get; }
         public abstract int Status { get; }
         protected internal abstract bool ContainsHeader(string name);

diff --git a/sdk/core/Azure.Core/api/Azure.Core.netcoreapp2.1.cs b/sdk/core/Azure.Core/api/Azure.Core.netcoreapp2.1.cs
@@ -217,7 +217,7 @@ protected Response() { }
         public virtual System.BinaryData Content { get { throw null; } }
         public abstract System.IO.Stream? ContentStream { get; set; }
         public virtual Azure.Core.ResponseHeaders Headers { get { throw null; } }
-        public bool IsError { get { throw null; } }
+        public virtual bool IsError { get { throw null; } }
         public abstract string ReasonPhrase { get; }
         public abstract int Status { get; }
         protected internal abstract bool ContainsHeader(string name);

diff --git a/sdk/core/Azure.Core/api/Azure.Core.netstandard2.0.cs b/sdk/core/Azure.Core/api/Azure.Core.netstandard2.0.cs
@@ -217,7 +217,7 @@ protected Response() { }
         public virtual System.BinaryData Content { get { throw null; } }
         public abstract System.IO.Stream? ContentStream { get; set; }
         public virtual Azure.Core.ResponseHeaders Headers { get { throw null; } }
-        public bool IsError { get { throw null; } }
+        public virtual bool IsError { get { throw null; } }
         public abstract string ReasonPhrase { get; }
         public abstract int Status { get; }
         protected internal abstract bool ContainsHeader(string name);

diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs
@@ -84,7 +84,7 @@ void AddCustomerPolicies(HttpPipelinePosition position)
 
             DiagnosticsOptions diagnostics = options.Diagnostics;
 
-            var sanitizer = new HttpMessageSanitizer(diagnostics.LoggedHeaderNames.ToArray(), diagnostics.LoggedQueryParameters.ToArray());
+            var sanitizer = new HttpMessageSanitizer(diagnostics.LoggedQueryParameters.ToArray(), diagnostics.LoggedHeaderNames.ToArray());
 
             bool isDistributedTracingEnabled = options.Diagnostics.IsDistributedTracingEnabled;
 

diff --git a/sdk/core/Azure.Core/src/Response.cs b/sdk/core/Azure.Core/src/Response.cs
@@ -86,9 +86,9 @@ public virtual BinaryData Content
         /// Indicates whether the message's <see cref="ResponseClassifier"/> considers this
         /// response an error.
         /// </summary>
-        public bool IsError { get; internal set; }
+        public virtual bool IsError { get; internal set; }
 
-        internal HttpMessageSanitizer? Sanitizer { get; set; }
+        internal HttpMessageSanitizer? Sanitizer { get; set; } = HttpMessageSanitizer.Default;
 
         /// <summary>
         /// Returns header value if the header is stored in the collection. If header has multiple values they are going to be joined with a comma.

diff --git a/sdk/core/Azure.Core/src/Shared/HttpMessageSanitizer.cs b/sdk/core/Azure.Core/src/Shared/HttpMessageSanitizer.cs
@@ -17,6 +17,8 @@ internal class HttpMessageSanitizer
         private readonly string _redactedPlaceholder;
         private readonly HashSet<string> _allowedHeaders;
 
+        internal static HttpMessageSanitizer Default = new HttpMessageSanitizer(Array.Empty<string>(), Array.Empty<string>());
+
         public HttpMessageSanitizer(string[] allowedQueryParameters, string[] allowedHeaders, string redactedPlaceholder = "REDACTED")
         {
             _logAllHeaders = allowedHeaders.Contains(LogAllValue);

diff --git a/sdk/core/Azure.Core/tests/FailedResponseExceptionTests.cs b/sdk/core/Azure.Core/tests/FailedResponseExceptionTests.cs
@@ -4,6 +4,7 @@
 using System;
 using System.Collections.Generic;
 using System.IO;
+using System.Linq;
 using System.Runtime.Serialization;
 using System.Threading.Tasks;
 using Azure.Core.Pipeline;
@@ -16,6 +17,7 @@ public class FailedResponseExceptionTests
     {
         private static readonly string s_nl = Environment.NewLine;
         private static ClientDiagnostics ClientDiagnostics = new ClientDiagnostics(new TestClientOption());
+        private static HttpMessageSanitizer Sanitizer = new TestClientOption().Sanitizer;
 
         [Test]
         public async Task FormatsResponse()
@@ -36,6 +38,45 @@ public async Task FormatsResponse()
             Assert.AreEqual(formattedResponse, exception.Message);
         }
 
+        [Test]
+        public void FormatsResponse_ResponseCtor()
+        {
+            var formattedResponse =
+                "Service request failed." + s_nl +
+                "Status: 210 (Reason)" + s_nl +
+                s_nl +
+                "Headers:" + s_nl +
+                "Custom-Header: Value" + s_nl +
+                "x-ms-requestId: 123" + s_nl;
+
+            var response = new MockResponse(210, "Reason");
+            response.AddHeader(new HttpHeader("Custom-Header", "Value"));
+            response.AddHeader(new HttpHeader("x-ms-requestId", "123"));
+            response.Sanitizer = Sanitizer;
+
+            RequestFailedException exception = new RequestFailedException(response);
+            Assert.AreEqual(formattedResponse, exception.Message);
+        }
+
+        [Test]
+        public void FormatsResponseWithoutSanitizer_ResponseCtor()
+        {
+            var formattedResponse =
+                "Service request failed." + s_nl +
+                "Status: 210 (Reason)" + s_nl +
+                s_nl +
+                "Headers:" + s_nl +
+                "Custom-Header: REDACTED" + s_nl +
+                "x-ms-requestId: REDACTED" + s_nl;
+
+            var response = new MockResponse(210, "Reason");
+            response.AddHeader(new HttpHeader("Custom-Header", "Value"));
+            response.AddHeader(new HttpHeader("x-ms-requestId", "123"));
+
+            RequestFailedException exception = new RequestFailedException(response);
+            Assert.AreEqual(formattedResponse, exception.Message);
+        }
+
         [Test]
         public async Task HeadersAreSanitized()
         {
@@ -55,6 +96,25 @@ public async Task HeadersAreSanitized()
             Assert.AreEqual(formattedResponse, exception.Message);
         }
 
+        [Test]
+        public void HeadersAreSanitized_ResponseCtor()
+        {
+            var formattedResponse =
+                "Service request failed." + s_nl +
+                "Status: 210 (Reason)" + s_nl +
+                s_nl +
+                "Headers:" + s_nl +
+                "Custom-Header-2: REDACTED" + s_nl +
+                "x-ms-requestId-2: REDACTED" + s_nl;
+
+            var response = new MockResponse(210, "Reason");
+            response.AddHeader(new HttpHeader("Custom-Header-2", "Value"));
+            response.AddHeader(new HttpHeader("x-ms-requestId-2", "123"));
+
+            RequestFailedException exception = new RequestFailedException(response);
+            Assert.AreEqual(formattedResponse, exception.Message);
+        }
+
         [Test]
         public async Task FormatsResponseContentForTextContentTypes()
         {
@@ -176,7 +236,7 @@ public void RequestFailedExceptionIsSerializeable()
             using var memoryStream = new MemoryStream();
             dataContractSerializer.WriteObject(memoryStream, exception);
             memoryStream.Position = 0;
-            deserialized = (RequestFailedException) dataContractSerializer.ReadObject(memoryStream);
+            deserialized = (RequestFailedException)dataContractSerializer.ReadObject(memoryStream);
 
             Assert.AreEqual(exception.Message, deserialized.Message);
             Assert.AreEqual(exception.Status, deserialized.Status);
@@ -206,6 +266,30 @@ public async Task ParsesJsonErrors()
             Assert.AreEqual("StatusCode", exception.ErrorCode);
         }
 
+        [Test]
+        public void ParsesJsonErrors_ResponseCtor()
+        {
+            var formattedResponse =
+                "Custom message" + s_nl +
+                "Status: 210 (Reason)" + s_nl +
+                "ErrorCode: StatusCode" + s_nl +
+                s_nl +
+                "Content:" + s_nl +
+                "{ \"error\": { \"code\":\"StatusCode\", \"message\":\"Custom message\" }}" + s_nl +
+                s_nl +
+                "Headers:" + s_nl +
+                "Content-Type: text/json" + s_nl;
+
+            var response = new MockResponse(210, "Reason");
+            response.SetContent("{ \"error\": { \"code\":\"StatusCode\", \"message\":\"Custom message\" }}");
+            response.AddHeader(new HttpHeader("Content-Type", "text/json"));
+            response.Sanitizer = Sanitizer;
+
+            RequestFailedException exception = new RequestFailedException(response);
+            Assert.AreEqual(formattedResponse, exception.Message);
+            Assert.AreEqual("StatusCode", exception.ErrorCode);
+        }
+
         [Test]
         public async Task IgnoresInvalidJsonErrors()
         {
@@ -250,6 +334,8 @@ public async Task IgnoresNonStandardJson()
 
         private class TestClientOption : ClientOptions
         {
+            public HttpMessageSanitizer Sanitizer => new HttpMessageSanitizer(Diagnostics.LoggedQueryParameters.ToArray(), Diagnostics.LoggedHeaderNames.ToArray());
+
             public TestClientOption()
             {
                 Diagnostics.LoggedHeaderNames.Add("x-ms-requestId");

diff --git a/sdk/core/Azure.Core/tests/ResponseTests.cs b/sdk/core/Azure.Core/tests/ResponseTests.cs
@@ -6,6 +6,7 @@
 using System.Linq;
 using Azure.Core.TestFramework;
 using NUnit.Framework;
+using Moq;
 
 namespace Azure.Core.Tests
 {
@@ -145,6 +146,30 @@ public void ContentPropertyWorksForMemoryStreamsWithPrivateBuffers()
             CollectionAssert.AreEqual(responseBody, response.Content.ToArray());
         }
 
+        [Test]
+        public void CanMockIsError()
+        {
+            var response = new MockResponse(500);
+
+            Assert.IsFalse(response.IsError);
+
+            response.SetIsError(true);
+
+            Assert.IsTrue(response.IsError);
+        }
+
+        [Test]
+        public void CanMoqIsError()
+        {
+            var response = new Mock<Response>();
+
+            Assert.IsFalse(response.Object.IsError);
+
+            response.SetupGet(x => x.IsError).Returns(true);
+
+            Assert.IsTrue(response.Object.IsError);
+        }
+
         internal class TestPayload
         {
             public string Name { get; }

diff --git a/sdk/eventhub/Azure.Messaging.EventHubs/Azure.Messaging.EventHubs.sln b/sdk/eventhub/Azure.Messaging.EventHubs/Azure.Messaging.EventHubs.sln
@@ -11,6 +11,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Azure.Core.TestFramework",
 EndProject
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "External", "External", "{2DB233D3-E757-423C-8F8D-742B0AFF4713}"
 EndProject
+Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Azure.Core", "..\..\core\Azure.Core\src\Azure.Core.csproj", "{6C67B4C6-0ABF-4E10-8BAF-FCF2AD053DE3}"
+EndProject
 Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Azure.Core.Experimental", "..\..\core\Azure.Core.Experimental\src\Azure.Core.Experimental.csproj", "{FB10A6B3-88DD-4296-9241-53ECDF942842}"
 EndProject
 Global
@@ -59,6 +61,18 @@ Global
 		{2CFDB3D6-5CFB-428C-9C89-29DD169B5433}.Release|x64.Build.0 = Release|Any CPU
 		{2CFDB3D6-5CFB-428C-9C89-29DD169B5433}.Release|x86.ActiveCfg = Release|Any CPU
 		{2CFDB3D6-5CFB-428C-9C89-29DD169B5433}.Release|x86.Build.0 = Release|Any CPU
+		{6C67B4C6-0ABF-4E10-8BAF-FCF2AD053DE3}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{6C67B4C6-0ABF-4E10-8BAF-FCF2AD053DE3}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{6C67B4C6-0ABF-4E10-8BAF-FCF2AD053DE3}.Debug|x64.ActiveCfg = Debug|Any CPU
+		{6C67B4C6-0ABF-4E10-8BAF-FCF2AD053DE3}.Debug|x64.Build.0 = Debug|Any CPU
+		{6C67B4C6-0ABF-4E10-8BAF-FCF2AD053DE3}.Debug|x86.ActiveCfg = Debug|Any CPU
+		{6C67B4C6-0ABF-4E10-8BAF-FCF2AD053DE3}.Debug|x86.Build.0 = Debug|Any CPU
+		{6C67B4C6-0ABF-4E10-8BAF-FCF2AD053DE3}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{6C67B4C6-0ABF-4E10-8BAF-FCF2AD053DE3}.Release|Any CPU.Build.0 = Release|Any CPU
+		{6C67B4C6-0ABF-4E10-8BAF-FCF2AD053DE3}.Release|x64.ActiveCfg = Release|Any CPU
+		{6C67B4C6-0ABF-4E10-8BAF-FCF2AD053DE3}.Release|x64.Build.0 = Release|Any CPU
+		{6C67B4C6-0ABF-4E10-8BAF-FCF2AD053DE3}.Release|x86.ActiveCfg = Release|Any CPU
+		{6C67B4C6-0ABF-4E10-8BAF-FCF2AD053DE3}.Release|x86.Build.0 = Release|Any CPU
 		{FB10A6B3-88DD-4296-9241-53ECDF942842}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
 		{FB10A6B3-88DD-4296-9241-53ECDF942842}.Debug|Any CPU.Build.0 = Debug|Any CPU
 		{FB10A6B3-88DD-4296-9241-53ECDF942842}.Debug|x64.ActiveCfg = Debug|Any CPU
@@ -70,13 +84,14 @@ Global
 		{FB10A6B3-88DD-4296-9241-53ECDF942842}.Release|x64.ActiveCfg = Release|Any CPU
 		{FB10A6B3-88DD-4296-9241-53ECDF942842}.Release|x64.Build.0 = Release|Any CPU
 		{FB10A6B3-88DD-4296-9241-53ECDF942842}.Release|x86.ActiveCfg = Release|Any CPU
-		{FB10A6B3-88DD-4296-9241-53ECDF942842}.Release|x86.Build.0 = Release|Any CPU
+		{FB10A6B3-88DD-4296-9241-53ECDF942842}.Release|x86.Build.0 = Release|Any CPU		
 	EndGlobalSection
 	GlobalSection(SolutionProperties) = preSolution
 		HideSolutionNode = FALSE
 	EndGlobalSection
 	GlobalSection(NestedProjects) = preSolution
 		{2CFDB3D6-5CFB-428C-9C89-29DD169B5433} = {2DB233D3-E757-423C-8F8D-742B0AFF4713}
+		{6C67B4C6-0ABF-4E10-8BAF-FCF2AD053DE3} = {2DB233D3-E757-423C-8F8D-742B0AFF4713}
 	EndGlobalSection
 	GlobalSection(ExtensibilityGlobals) = postSolution
 		SolutionGuid = {509F2EE0-3348-4506-8AC7-9945B602CB43}