GatewayClientStore: Fixes an issue with dealing with invalid JSON HTTP responses

Microsoft.Azure.Cosmos/src/GatewayStoreClient.cs

@@ -9,6 +9,7 @@ namespace Microsoft.Azure.Cosmos
     using System.Diagnostics;
     using System.Diagnostics.CodeAnalysis;
     using System.IO;
+    using System.Linq;
     using System.Net.Http;
     using System.Net.Http.Headers;
     using System.Text;
@@ -19,6 +20,7 @@ namespace Microsoft.Azure.Cosmos
     using Microsoft.Azure.Documents;
     using Microsoft.Azure.Documents.Collections;
     using Newtonsoft.Json;
+    using Newtonsoft.Json.Linq;
 
     internal class GatewayStoreClient : TransportClient
     {
@@ -132,26 +134,66 @@ 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));
+            }
 
-            string resourceLink = responseMessage.RequestMessage.RequestUri.LocalPath;
-            if (!PathsHelper.TryParsePathSegments(resourceLink, out isFeed, out resourceTypeString, out resourceIdOrFullName, out isNameBased))
+            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.
+
+            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
             {
                 Stream readStream = await responseMessage.Content.ReadAsStreamAsync();
                 Error error = Documents.Resource.LoadFrom<Error>(readStream);
+
+                // need to rethink dropping the check for media type "application/json".
+
+                if (responseMessage.Content?.Headers?.ContentLength == 0 ||
+                    error.Message.Trim().Length == 0)
+                {
+                    error = new Error
+                    {
+                        Code = responseMessage.StatusCode.ToString(),
+                        Message = "No response content from gateway."
+                    };
+                }
+
                 return new DocumentClientException(
                     error,
                     responseMessage.Headers,
@@ -162,15 +204,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)
@@ -182,7 +224,6 @@ internal static async Task<DocumentClientException> CreateDocumentClientExceptio
                     }
                 }
 
-                String message = await responseMessage.Content.ReadAsStringAsync();
                 return new DocumentClientException(
                     message: context.ToString(),
                     innerException: null,

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

@@ -0,0 +1,226 @@
+//------------------------------------------------------------
+// Copyright (c) Microsoft Corporation.  All rights reserved.
+//------------------------------------------------------------
+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;
+    using Microsoft.Azure.Documents;
+    using Microsoft.VisualStudio.TestTools.UnitTesting;
+    using Newtonsoft.Json;
+
+    /// <summary>
+    /// Tests for <see cref="GatewayStoreClient"/>.
+    /// </summary>
+    [TestClass]
+    public class GatewayStoreClientTests
+    {
+        /// <summary>
+        /// 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>
+        [TestMethod]
+        [DataRow(@"<!DOCTYPE html><html><body></body></html>")]
+        [DataRow(@"   <!DOCTYPE html><html><body></body></html>")]
+        [DataRow(@"<!DOCTYPE html><html><body></body></html>   ")]
+        [DataRow(@"   <!DOCTYPE html><html><body></body></html>   ")]
+        [DataRow(@"ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890")]
+        [DataRow(@"   ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890")]
+        [DataRow(@"ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890   ")]
+        [DataRow(@"   ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890   ")]
+        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(
+                    mediaType: "application/json",
+                    encoding: Encoding.UTF8,
+                    content: JsonConvert.SerializeObject(
+                        value: new Error() { Code = HttpStatusCode.NotFound.ToString(), Message = contentMessage })),
+            };
+
+            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(contentMessage));
+
+            Assert.IsNotNull(value: documentClientException.Error);
+            Assert.AreEqual(expected: HttpStatusCode.NotFound.ToString(), actual: documentClientException.Error.Code);
+            Assert.AreEqual(expected: contentMessage, 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 zero after trim.
+        /// </summary>
+        [TestMethod]
+        [DataRow(@"")]
+        [DataRow(@"    ")]
+        public async Task TestCreateDocumentClientExceptionWhenMediaTypeIsApplicationJsonAndContentIsNotValidJsonAndContentLengthIsZeroAsync(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: JsonConvert.SerializeObject(
+                        value: new Error() { Code = HttpStatusCode.NotFound.ToString(), Message = contentMessage })),
+            };
+
+            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 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);
+        }
+    }
+}