some refactoring and tests. I want to see if I can mine more tests la…

…ter.

Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs

@@ -134,40 +134,63 @@ internal static INameValueCollection ExtractResponseHeaders(HttpResponseMessage
             return headers;
         }
 
+        /// <summary>
+        /// Creating a new DocumentClientException using the Gateway response message.
+        ///   Is the media type not "application/json"?
+        ///      return DocumentClientExcpetion with responseMessage and header information.
+        ///
+        ///   Is the header content-length == 0 and media type is "application/json"? Test case sensitivity.
+        ///      return DocumentClientException with message 'No response content from gateway.'
+        ///
+        ///   Is the content actual length == 0 after a trim and media type is "application/json"? Test case sensitivity. Whitespace scenarios.
+        ///      return DocumentClientException with message 'No response content from gateway.'
+        ///
+        ///   Is the content not parseable as json, but content length != 0 and media type is "application/json"? Test case sensitivity.
+        ///      return DocumentClientException with message set to raw non-json message from response.
+        /// </summary>
+        /// <param name="responseMessage"></param>
+        /// <param name="requestStatistics"></param>
         internal static async Task<DocumentClientException> CreateDocumentClientExceptionAsync(
             HttpResponseMessage responseMessage,
             IClientSideRequestStatistics requestStatistics)
         {
-            bool isNameBased = false;
-            bool isFeed = false;
-            string resourceTypeString;
-            string resourceIdOrFullName;
+            if (responseMessage is null)
+            {
+                throw new ArgumentNullException(nameof(responseMessage));
+            }
+
+            if (requestStatistics is null)
+            {
+                throw new ArgumentNullException(nameof(requestStatistics));
+            }
+
+            // Ask, what is the purpose of this, really?
+            // The only impact of try parse fail is an empty resourceIdOrFullName.
 
-            string resourceLink = responseMessage.RequestMessage.RequestUri.LocalPath;
-            if (!PathsHelper.TryParsePathSegments(resourceLink, out isFeed, out resourceTypeString, out resourceIdOrFullName, out isNameBased))
+            if (!PathsHelper.TryParsePathSegments(
+                resourceUrl: responseMessage.RequestMessage.RequestUri.LocalPath,
+                isFeed: out _,
+                resourcePath: out _,
+                resourceIdOrFullName: out string resourceIdOrFullName,
+                isNameBased: out _))
             {
                 // if resourceLink is invalid - we will not set resourceAddress in exception.
             }
 
-            // If service rejects the initial payload like header is to large it will return an HTML error instead of JSON.
-            if (string.Equals(responseMessage.Content?.Headers?.ContentType?.MediaType, "application/json", StringComparison.OrdinalIgnoreCase))
+            try
             {
-                // For more information, see https://github.com/Azure/azure-cosmos-dotnet-v3/issues/4162.
-                Error error;
+                Stream readStream = await responseMessage.Content.ReadAsStreamAsync();
+                Error error = Documents.Resource.LoadFrom<Error>(readStream);
 
-                if (await GatewayStoreClient.IsJsonHTTPResponseFromGatewayInvalidAsync(responseMessage))
+                if (responseMessage.Content?.Headers?.ContentLength == 0 ||
+                    error.Message.Trim().Length == 0)
                 {
                     error = new Error
                     {
                         Code = responseMessage.StatusCode.ToString(),
                         Message = "No response content from gateway."
                     };
                 }
-                else
-                {
-                    Stream readStream = await responseMessage.Content.ReadAsStreamAsync();
-                    error = Documents.Resource.LoadFrom<Error>(readStream);
-                }
 
                 return new DocumentClientException(
                     error,
@@ -179,15 +202,15 @@ internal static async Task<DocumentClientException> CreateDocumentClientExceptio
                     RequestStatistics = requestStatistics
                 };
             }
-            else
+            catch
             {
                 StringBuilder context = new StringBuilder();
                 context.AppendLine(await responseMessage.Content.ReadAsStringAsync());
 
                 HttpRequestMessage requestMessage = responseMessage.RequestMessage;
                 if (requestMessage != null)
                 {
-                    context.AppendLine($"RequestUri: {requestMessage.RequestUri.ToString()};");
+                    context.AppendLine($"RequestUri: {requestMessage.RequestUri};");
                     context.AppendLine($"RequestMethod: {requestMessage.Method.Method};");
 
                     if (requestMessage.Headers != null)
@@ -199,7 +222,6 @@ internal static async Task<DocumentClientException> CreateDocumentClientExceptio
                     }
                 }
 
-                String message = await responseMessage.Content.ReadAsStringAsync();
                 return new DocumentClientException(
                     message: context.ToString(),
                     innerException: null,
@@ -214,29 +236,6 @@ internal static async Task<DocumentClientException> CreateDocumentClientExceptio
             }
         }
 
-        /// <summary>
-        /// Checking if exception response (deserializable Error object) is valid based on the content length.
-        /// For more information, see <see href="https://github.com/Azure/azure-cosmos-dotnet-v3/issues/4162."/>.
-        /// </summary>
-        /// <param name="responseMessage"></param>
-        private static async Task<bool> IsJsonHTTPResponseFromGatewayInvalidAsync(HttpResponseMessage responseMessage)
-        {
-            string readString = await responseMessage.Content.ReadAsStringAsync();
-
-            try
-            {
-                _ = JToken.Parse(readString);
-
-                return responseMessage.Content?.Headers?.ContentLength == 0 ||
-                    readString.Trim().Length == 0;
-            }
-            catch (JsonReaderException)
-            {
-                return true;
-            }
-
-        }
-
         internal static bool IsAllowedRequestHeader(string headerName)
         {
             if (!headerName.StartsWith("x-ms", StringComparison.OrdinalIgnoreCase))

Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/GatewayStoreClientTests.cs

@@ -6,6 +6,8 @@ namespace Microsoft.Azure.Cosmos
     using System;
     using System.Net;
     using System.Net.Http;
+    using System.Reflection.Metadata;
+    using System.Text;
     using System.Threading.Tasks;
     using Microsoft.Azure.Cosmos.Tracing;
     using Microsoft.Azure.Cosmos.Tracing.TraceData;
@@ -20,13 +22,81 @@ namespace Microsoft.Azure.Cosmos
     public class GatewayStoreClientTests
     {
         /// <summary>
-        /// Testing the exception behavior when a response from the Gateway has no response (deserializable Error object) based on the content length.
-        /// For more information, see <see href="https://github.com/Azure/azure-cosmos-dotnet-v3/issues/4162"/>.
+        /// Testing CreateDocumentClientExceptionAsync when media type is not application/json. Not meant to be an exhaustive test for all
+        /// legitimate content media types.
+        /// </summary>
+        [TestMethod]
+        [DataRow("text/html", "<!DOCTYPE html><html><body></body></html>")]
+        [DataRow("text/plain", "This is a test error message.")]
+        public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsNotApplicationJsonAsync(
+            string mediaType,
+            string contentMessage)
+        {
+            HttpResponseMessage responseMessage = new(statusCode: System.Net.HttpStatusCode.NotFound)
+            {
+                RequestMessage = new HttpRequestMessage(
+                    method: HttpMethod.Get,
+                    requestUri: @"https://pt_ac_test_uri.com/"),
+                Content = new StringContent(
+                    mediaType: mediaType,
+                    encoding: Encoding.UTF8,
+                    content: JsonConvert.SerializeObject(
+                        value: new Error() { Code = HttpStatusCode.NotFound.ToString(), Message = contentMessage })),
+            };
+
+            DocumentClientException documentClientException = await GatewayStoreClient.CreateDocumentClientExceptionAsync(
+                responseMessage: responseMessage,
+                requestStatistics: GatewayStoreClientTests.CreateClientSideRequestStatistics());
+
+            Assert.IsNotNull(value: documentClientException);
+            Assert.AreEqual(expected: HttpStatusCode.NotFound, actual: documentClientException.StatusCode);
+            Assert.IsTrue(condition: documentClientException.Message.Contains(contentMessage));
+
+            Assert.IsNotNull(value: documentClientException.Error);
+            Assert.AreEqual(expected: HttpStatusCode.NotFound.ToString(), actual: documentClientException.Error.Code);
+            Assert.IsTrue(documentClientException.Error.Message.Contains(contentMessage));
+        }
+
+        /// <summary>
+        /// Testing CreateDocumentClientExceptionAsync when media type is application/json and the header content length is zero.
+        /// </summary>
+        [TestMethod]
+        public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsApplicationJsonAndHeaderContentLengthIsZeroAsync()
+        {
+            HttpResponseMessage responseMessage = new(statusCode: System.Net.HttpStatusCode.NotFound)
+            {
+                RequestMessage = new HttpRequestMessage(
+                    method: HttpMethod.Get,
+                    requestUri: @"https://pt_ac_test_uri.com/"),
+                Content = new StringContent(
+                    mediaType: "application/json",
+                    encoding: Encoding.UTF8,
+                    content: JsonConvert.SerializeObject(
+                        value: new Error() { Code = HttpStatusCode.NotFound.ToString(), Message = "" })),
+            };
+
+            IClientSideRequestStatistics requestStatistics = new ClientSideRequestStatisticsTraceDatum(
+                startTime: DateTime.UtcNow,
+                trace: NoOpTrace.Singleton);
+
+            DocumentClientException documentClientException = await GatewayStoreClient.CreateDocumentClientExceptionAsync(
+                responseMessage: responseMessage,
+                requestStatistics: requestStatistics);
+
+            Assert.IsNotNull(value: documentClientException);
+            Assert.AreEqual(expected: HttpStatusCode.NotFound, actual: documentClientException.StatusCode);
+            Assert.IsTrue(condition: documentClientException.Message.Contains("No response content from gateway."));
+
+            Assert.IsNotNull(value: documentClientException.Error);
+            Assert.AreEqual(expected: HttpStatusCode.NotFound.ToString(), actual: documentClientException.Error.Code);
+            Assert.AreEqual(expected: "No response content from gateway.", actual: documentClientException.Error.Message);
+        }
+
+        /// <summary>
+        /// Testing CreateDocumentClientExceptionAsync when media type is application/json and the content is not valid json
+        /// and has a content length that is not zero after trim.
         /// </summary>
-        /// <returns></returns>
         [TestMethod]
-        [DataRow(@"")]
-        [DataRow(@"    ")]
         [DataRow(@"<!DOCTYPE html><html><body></body></html>")]
         [DataRow(@"   <!DOCTYPE html><html><body></body></html>")]
         [DataRow(@"<!DOCTYPE html><html><body></body></html>   ")]
@@ -35,44 +105,45 @@ public class GatewayStoreClientTests
         [DataRow(@"   ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890")]
         [DataRow(@"ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890   ")]
         [DataRow(@"   ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890   ")]
-        public async Task CreateDocumentClientExceptionInvalidJsonResponseFromGatewayTestAsync(string content)
+        public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsApplicationJsonAndContentIsNotValidJsonAndContentLengthIsNotZeroAsync(string contentMessage)
         {
             HttpResponseMessage responseMessage = new(statusCode: System.Net.HttpStatusCode.NotFound)
             {
                 RequestMessage = new HttpRequestMessage(
                     method: HttpMethod.Get,
                     requestUri: @"https://pt_ac_test_uri.com/"),
                 Content = new StringContent(
-                    content: content),
+                    mediaType: "application/json",
+                    encoding: Encoding.UTF8,
+                    content: JsonConvert.SerializeObject(
+                        value: new Error() { Code = HttpStatusCode.NotFound.ToString(), Message = contentMessage })),
             };
 
-            responseMessage.Content.Headers.ContentType = new System.Net.Http.Headers.MediaTypeHeaderValue("application/json");
-
             IClientSideRequestStatistics requestStatistics = new ClientSideRequestStatisticsTraceDatum(
-                startTime: DateTime.UtcNow, 
+                startTime: DateTime.UtcNow,
                 trace: NoOpTrace.Singleton);
 
             DocumentClientException documentClientException = await GatewayStoreClient.CreateDocumentClientExceptionAsync(
-                responseMessage: responseMessage,
-                requestStatistics: requestStatistics);
+                    responseMessage: responseMessage,
+                    requestStatistics: requestStatistics);
 
             Assert.IsNotNull(value: documentClientException);
             Assert.AreEqual(expected: HttpStatusCode.NotFound, actual: documentClientException.StatusCode);
-            Assert.IsTrue(condition: documentClientException.Message.Contains("No response content from gateway."));
+            Assert.IsTrue(condition: documentClientException.Message.Contains(contentMessage));
 
             Assert.IsNotNull(value: documentClientException.Error);
             Assert.AreEqual(expected: HttpStatusCode.NotFound.ToString(), actual: documentClientException.Error.Code);
-            Assert.AreEqual(expected: "No response content from gateway.", actual: documentClientException.Error.Message);
+            Assert.AreEqual(expected: contentMessage, actual: documentClientException.Error.Message);
         }
 
         /// <summary>
-        /// Testing the exception behavior when a response from the Gateway has a response (deserializable Error object) based the content length.
-        /// For more information, see <see href="https://github.com/Azure/azure-cosmos-dotnet-v3/issues/4162"/>.
+        /// Testing CreateDocumentClientExceptionAsync when media type is application/json and the content is not valid json
+        /// and has a content length that is zero after trim.
         /// </summary>
-        /// <returns></returns>
         [TestMethod]
-        [DataRow(@"This is the content of a test error message.")]
-        public async Task CreateDocumentClientExceptionValidJsonResponseFromGatewayTestAsync(string content)
+        [DataRow(@"")]
+        [DataRow(@"    ")]
+        public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsApplicationJsonAndContentIsNotValidJsonAndContentLengthIsZeroAsync(string contentMessage)
         {
             HttpResponseMessage responseMessage = new(statusCode: System.Net.HttpStatusCode.NotFound)
             {
@@ -81,26 +152,75 @@ public async Task CreateDocumentClientExceptionValidJsonResponseFromGatewayTestA
                     requestUri: @"https://pt_ac_test_uri.com/"),
                 Content = new StringContent(
                     content: JsonConvert.SerializeObject(
-                        value: new Error() { Code = HttpStatusCode.NotFound.ToString(), Message = content })),
+                        value: new Error() { Code = HttpStatusCode.NotFound.ToString(), Message = contentMessage })),
             };
 
-            responseMessage.Content.Headers.ContentType = new System.Net.Http.Headers.MediaTypeHeaderValue("application/json");
-
             IClientSideRequestStatistics requestStatistics = new ClientSideRequestStatisticsTraceDatum(
                 startTime: DateTime.UtcNow,
                 trace: NoOpTrace.Singleton);
 
             DocumentClientException documentClientException = await GatewayStoreClient.CreateDocumentClientExceptionAsync(
-                responseMessage: responseMessage,
-                requestStatistics: requestStatistics);
+                    responseMessage: responseMessage,
+                    requestStatistics: requestStatistics);
 
             Assert.IsNotNull(value: documentClientException);
             Assert.AreEqual(expected: HttpStatusCode.NotFound, actual: documentClientException.StatusCode);
-            Assert.IsTrue(condition: documentClientException.Message.Contains(content));
+            Assert.IsTrue(condition: documentClientException.Message.Contains("No response content from gateway."));
 
             Assert.IsNotNull(value: documentClientException.Error);
             Assert.AreEqual(expected: HttpStatusCode.NotFound.ToString(), actual: documentClientException.Error.Code);
-            Assert.AreEqual(expected: content, actual: documentClientException.Error.Message);
+            Assert.AreEqual(expected: "No response content from gateway.", actual: documentClientException.Error.Message);
+        }
+
+        /// <summary>
+        /// Testing CreateDocumentClientExceptionAsync when response message argument is null, then expects an argumentNullException.
+        /// </summary>
+        [TestMethod]
+        public async Task TestCreateDocumentClientExceptionWhenResponseMessageIsNullExpectsArgumentNullException()
+        {
+            IClientSideRequestStatistics requestStatistics = new ClientSideRequestStatisticsTraceDatum(
+                startTime: DateTime.UtcNow,
+                trace: NoOpTrace.Singleton);
+
+            ArgumentNullException argumentNullException = await Assert.ThrowsExceptionAsync<ArgumentNullException>(async () => await GatewayStoreClient.CreateDocumentClientExceptionAsync(
+                    responseMessage: default,
+                    requestStatistics: requestStatistics)
+            );
+
+            Assert.IsNotNull(argumentNullException);
+            Assert.AreEqual(expected: "Value cannot be null. (Parameter 'responseMessage')", actual: argumentNullException.Message);
+        }
+
+        /// <summary>
+        /// Testing CreateDocumentClientExceptionAsync when request statistics argument is null, then expects an argumentNullException.
+        /// </summary>
+        [TestMethod]
+        public async Task TestCreateDocumentClientExceptionWhenRequestStatisticsIsNullExpectsArgumentNullException()
+        {
+            HttpResponseMessage responseMessage = new(statusCode: System.Net.HttpStatusCode.NotFound)
+            {
+                RequestMessage = new HttpRequestMessage(
+                    method: HttpMethod.Get,
+                    requestUri: @"https://pt_ac_test_uri.com/"),
+                Content = new StringContent(
+                    content: JsonConvert.SerializeObject(
+                        value: new Error() { Code = HttpStatusCode.NotFound.ToString(), Message = "" })),
+            };
+
+            ArgumentNullException argumentNullException = await Assert.ThrowsExceptionAsync<ArgumentNullException>(async () => await GatewayStoreClient.CreateDocumentClientExceptionAsync(
+                    responseMessage: responseMessage,
+                    requestStatistics: default)
+            );
+
+            Assert.IsNotNull(argumentNullException);
+            Assert.AreEqual(expected: "Value cannot be null. (Parameter 'requestStatistics')", actual: argumentNullException.Message);
+        }
+
+        private static IClientSideRequestStatistics CreateClientSideRequestStatistics()
+        {
+            return new ClientSideRequestStatisticsTraceDatum(
+                startTime: DateTime.UtcNow,
+                trace: NoOpTrace.Singleton);
         }
     }
 }