CosmosException : Fixes exception messages to remove JSON formatting (#…

…2303) * CosmosException : Fixes exception messages by removing escape character * Update benchmark results * Update Microsoft.Azure.Cosmos/src/Resource/CosmosExceptions/CosmosExceptionFactory.cs Co-authored-by: j82w <j82w@users.noreply.github.com> * Code review Changes - 1 * Code Review Changes - 2 Co-authored-by: Sourabh Jain <sourabhjain@microsoft.com> Co-authored-by: j82w <j82w@users.noreply.github.com>
Azure · Mar 18, 2021 · 6c4648b · 6c4648b
1 parent 79b257e
commit 6c4648b
Show file tree

Hide file tree

Showing 4 changed files with 113 additions and 49 deletions.
diff --git a/Microsoft.Azure.Cosmos/src/Resource/CosmosExceptions/CosmosExceptionFactory.cs b/Microsoft.Azure.Cosmos/src/Resource/CosmosExceptions/CosmosExceptionFactory.cs
@@ -5,10 +5,15 @@
 namespace Microsoft.Azure.Cosmos.Resource.CosmosExceptions
 {
     using System;
+    using System.Collections.Generic;
     using System.IO;
     using System.Net;
+    using System.Text;
+    using System.Text.RegularExpressions;
     using Microsoft.Azure.Cosmos.Tracing;
     using Microsoft.Azure.Documents;
+    using Newtonsoft.Json;
+    using Newtonsoft.Json.Linq;
 
     internal static class CosmosExceptionFactory
     {
@@ -154,24 +159,36 @@ internal static (Error, string) GetErrorFromStream(
                 if (content != null
                && content.CanRead)
                 {
-                    try
+                    using (StreamReader streamReader = new StreamReader(content))
                     {
-                        Error error = Documents.Resource.LoadFrom<Error>(content);
-                        if (error != null)
+                        string errorContent = streamReader.ReadToEnd();
+                        try
+                        {
+                            JObject errorObj = JObject.Parse(errorContent);
+                            Error error = errorObj.ToObject<Error>();
+                            if (error != null)
+                            {
+                                StringBuilder message = new StringBuilder();
+                                foreach (var err in errorObj)
+                                {
+                                    message
+                                        .Append(Environment.NewLine)
+                                        .Append(err.Key)
+                                        .Append(" : ")
+                                        .Append(err.Value);
+                                }
+                                message.Append(Environment.NewLine);
+                                // Error format is not consistent across modes
+                                return (error, message.ToString());
+                            }
+                        }
+                        catch (Newtonsoft.Json.JsonReaderException)
                         {
-                            // Error format is not consistent across modes
-                            return (error, error.ToString());
                         }
-                    }
-                    catch (Newtonsoft.Json.JsonReaderException)
-                    {
-                    }
 
-                    // Content is not Json
-                    content.Position = 0;
-                    using (StreamReader streamReader = new StreamReader(content))
-                    {
-                        return (null, streamReader.ReadToEnd());
+                        // Content is not Json
+                        content.Position = 0;
+                        return (null, errorContent);
                     }
                 }
 

diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/CosmosItemTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/CosmosItemTests.cs
@@ -1448,8 +1448,9 @@ public async Task ItemRequestOptionAccessConditionTest()
                 Assert.AreEqual(HttpStatusCode.PreconditionFailed, e.StatusCode, e.Message);
                 Assert.AreNotEqual(e.ActivityId, Guid.Empty);
                 Assert.IsTrue(e.RequestCharge > 0);
-                Assert.AreEqual($"{{{Environment.NewLine}  \"Errors\": [{Environment.NewLine}    \"One of the specified pre-condition is not met\"{Environment.NewLine}  ]{Environment.NewLine}}}", e.ResponseBody);
-                string expectedMessage = $"Response status code does not indicate success: PreconditionFailed (412); Substatus: 0; ActivityId: {e.ActivityId}; Reason: ({{{Environment.NewLine}  \"Errors\": [{Environment.NewLine}    \"One of the specified pre-condition is not met\"{Environment.NewLine}  ]{Environment.NewLine}}});";
+                string expectedResponseBody = $"{Environment.NewLine}Errors : [{Environment.NewLine}  \"One of the specified pre-condition is not met\"{Environment.NewLine}]{Environment.NewLine}";
+                Assert.AreEqual(expectedResponseBody, e.ResponseBody);
+                string expectedMessage = $"Response status code does not indicate success: PreconditionFailed (412); Substatus: 0; ActivityId: {e.ActivityId}; Reason: ({expectedResponseBody});";
                 Assert.AreEqual(expectedMessage, e.Message);
             }
             finally
@@ -1517,6 +1518,8 @@ await containerInternal.PatchItemAsync<ToDoActivity>(
             {
                 Assert.AreEqual(HttpStatusCode.NotFound, ex.StatusCode);
                 Assert.IsTrue(ex.Message.Contains("Resource Not Found"));
+                Assert.IsTrue(ex.Message.Contains("https://aka.ms/cosmosdb-tsg-not-found"));
+
             }
 
             // adding a child when parent / ancestor does not exist - 400 BadRequest response

diff --git a/...ure.Cosmos/tests/Microsoft.Azure.Cosmos.Performance.Tests/Contracts/BenchmarkResults.json b/...ure.Cosmos/tests/Microsoft.Azure.Cosmos.Performance.Tests/Contracts/BenchmarkResults.json
@@ -1,36 +1,36 @@
 {
-  "MasterKeyAuthorizationBenchmark.CreateSignatureGeneration;": 542.0,
-  "MasterKeyAuthorizationBenchmark.ReadSignatureGeneration;": 538.0,
-  "MockedItemBenchmark.CreateItem;[Type=OfT]": 45296.0,
-  "MockedItemBenchmark.CreateItem;[Type=OfTCustom]": 45306.0,
-  "MockedItemBenchmark.CreateItem;[Type=OfTWithDiagnosticsToString]": 72264.0,
-  "MockedItemBenchmark.CreateItem;[Type=Stream]": 29790.0,
-  "MockedItemBenchmark.DeleteItemExists;[Type=OfT]": 37180.0,
-  "MockedItemBenchmark.DeleteItemExists;[Type=OfTCustom]": 37150.0,
-  "MockedItemBenchmark.DeleteItemExists;[Type=OfTWithDiagnosticsToString]": 63448.0,
-  "MockedItemBenchmark.DeleteItemExists;[Type=Stream]": 29808.0,
-  "MockedItemBenchmark.DeleteItemNotExists;[Type=OfT]": 52258.0,
-  "MockedItemBenchmark.DeleteItemNotExists;[Type=OfTCustom]": 52254.0,
-  "MockedItemBenchmark.DeleteItemNotExists;[Type=OfTWithDiagnosticsToString]": 78552.0,
-  "MockedItemBenchmark.DeleteItemNotExists;[Type=Stream]": 47156.0,
-  "MockedItemBenchmark.ReadFeed;[Type=OfT]": 571066.0,
-  "MockedItemBenchmark.ReadFeed;[Type=OfTCustom]": 571062.0,
-  "MockedItemBenchmark.ReadFeed;[Type=OfTWithDiagnosticsToString]": 610228.0,
-  "MockedItemBenchmark.ReadFeed;[Type=Stream]": 42748.0,
-  "MockedItemBenchmark.ReadItemExists;[Type=OfT]": 42034.0,
-  "MockedItemBenchmark.ReadItemExists;[Type=OfTCustom]": 42040.0,
-  "MockedItemBenchmark.ReadItemExists;[Type=OfTWithDiagnosticsToString]": 67784.0,
-  "MockedItemBenchmark.ReadItemExists;[Type=Stream]": 34694.0,
-  "MockedItemBenchmark.ReadItemNotExists;[Type=OfT]": 57138.0,
-  "MockedItemBenchmark.ReadItemNotExists;[Type=OfTCustom]": 57142.0,
-  "MockedItemBenchmark.ReadItemNotExists;[Type=OfTWithDiagnosticsToString]": 82856.0,
-  "MockedItemBenchmark.ReadItemNotExists;[Type=Stream]": 52034.0,
+  "MasterKeyAuthorizationBenchmark.CreateSignatureGeneration;": 534.0,
+  "MasterKeyAuthorizationBenchmark.ReadSignatureGeneration;": 532.0,
+  "MockedItemBenchmark.CreateItem;[Type=OfT]": 45302.0,
+  "MockedItemBenchmark.CreateItem;[Type=OfTCustom]": 45320.0,
+  "MockedItemBenchmark.CreateItem;[Type=OfTWithDiagnosticsToString]": 72242.0,
+  "MockedItemBenchmark.CreateItem;[Type=Stream]": 29766.0,
+  "MockedItemBenchmark.DeleteItemExists;[Type=OfT]": 37154.0,
+  "MockedItemBenchmark.DeleteItemExists;[Type=OfTCustom]": 37154.0,
+  "MockedItemBenchmark.DeleteItemExists;[Type=OfTWithDiagnosticsToString]": 63450.0,
+  "MockedItemBenchmark.DeleteItemExists;[Type=Stream]": 30382.0,
+  "MockedItemBenchmark.DeleteItemNotExists;[Type=OfT]": 48216.0,
+  "MockedItemBenchmark.DeleteItemNotExists;[Type=OfTCustom]": 48216.0,
+  "MockedItemBenchmark.DeleteItemNotExists;[Type=OfTWithDiagnosticsToString]": 74512.0,
+  "MockedItemBenchmark.DeleteItemNotExists;[Type=Stream]": 43118.0,
+  "MockedItemBenchmark.ReadFeed;[Type=OfT]": 571684.0,
+  "MockedItemBenchmark.ReadFeed;[Type=OfTCustom]": 571048.0,
+  "MockedItemBenchmark.ReadFeed;[Type=OfTWithDiagnosticsToString]": 611762.0,
+  "MockedItemBenchmark.ReadFeed;[Type=Stream]": 42734.0,
+  "MockedItemBenchmark.ReadItemExists;[Type=OfT]": 42032.0,
+  "MockedItemBenchmark.ReadItemExists;[Type=OfTCustom]": 41870.0,
+  "MockedItemBenchmark.ReadItemExists;[Type=OfTWithDiagnosticsToString]": 67766.0,
+  "MockedItemBenchmark.ReadItemExists;[Type=Stream]": 34688.0,
+  "MockedItemBenchmark.ReadItemNotExists;[Type=OfT]": 53082.0,
+  "MockedItemBenchmark.ReadItemNotExists;[Type=OfTCustom]": 53100.0,
+  "MockedItemBenchmark.ReadItemNotExists;[Type=OfTWithDiagnosticsToString]": 78810.0,
+  "MockedItemBenchmark.ReadItemNotExists;[Type=Stream]": 47994.0,
   "MockedItemBenchmark.UpdateItem;[Type=OfT]": 45542.0,
-  "MockedItemBenchmark.UpdateItem;[Type=OfTCustom]": 45554.0,
-  "MockedItemBenchmark.UpdateItem;[Type=OfTWithDiagnosticsToString]": 72604.0,
-  "MockedItemBenchmark.UpdateItem;[Type=Stream]": 30038.0,
-  "MockedItemBenchmark.UpsertItem;[Type=OfT]": 45458.0,
-  "MockedItemBenchmark.UpsertItem;[Type=OfTCustom]": 45448.0,
-  "MockedItemBenchmark.UpsertItem;[Type=OfTWithDiagnosticsToString]": 72394.0,
-  "MockedItemBenchmark.UpsertItem;[Type=Stream]": 29958.0
+  "MockedItemBenchmark.UpdateItem;[Type=OfTCustom]": 45532.0,
+  "MockedItemBenchmark.UpdateItem;[Type=OfTWithDiagnosticsToString]": 72504.0,
+  "MockedItemBenchmark.UpdateItem;[Type=Stream]": 30026.0,
+  "MockedItemBenchmark.UpsertItem;[Type=OfT]": 45448.0,
+  "MockedItemBenchmark.UpsertItem;[Type=OfTCustom]": 45458.0,
+  "MockedItemBenchmark.UpsertItem;[Type=OfTWithDiagnosticsToString]": 72378.0,
+  "MockedItemBenchmark.UpsertItem;[Type=Stream]": 29934.0
 }
diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/CosmosExceptionTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/CosmosExceptionTests.cs
@@ -15,6 +15,7 @@ namespace Microsoft.Azure.Cosmos
     using Microsoft.VisualStudio.TestTools.UnitTesting;
     using Moq;
     using Newtonsoft.Json;
+    using Newtonsoft.Json.Linq;
 
     [TestClass]
     public class CosmosExceptionTests
@@ -140,6 +141,49 @@ public void EnsureSuccessStatusCode_ThrowsOnFailure_ContainsJsonBody()
             }
         }
 
+        [TestMethod]
+        public void EnsureSuccessStatusCode_ThrowsOnFailure_ContainsComplexJsonBody()
+        {
+            JObject error = new JObject
+            {
+                { "Code", "code" },
+                { "Message", "TestContent" },
+                { "Error", new JArray { "msg1", "msg2" }},
+                { "Link", "https://www.demolink.com" },
+                { "Path", "/demo/path" },
+                { "EscapedPath", @"/demo/path/with/escape/character" }
+            };
+
+            string testContent = JsonConvert.SerializeObject(error);
+
+            using (MemoryStream memoryStream = new MemoryStream())
+            {
+                StreamWriter sw = new StreamWriter(memoryStream);
+                sw.Write(testContent);
+                sw.Flush();
+                memoryStream.Seek(0, SeekOrigin.Begin);
+
+                ResponseMessage responseMessage = new ResponseMessage(HttpStatusCode.NotFound) { Content = memoryStream };
+                try
+                {
+                    responseMessage.EnsureSuccessStatusCode();
+                    Assert.Fail("Should have thrown");
+                }
+                catch (CosmosException exception)
+                {
+                    Assert.IsTrue(exception.Message.Contains("code"));
+                    Assert.IsTrue(exception.Message.Contains("TestContent"));
+                    Assert.IsTrue(exception.Message.Contains("msg1"));
+                    Assert.IsTrue(exception.Message.Contains("msg2"));
+                    Assert.IsTrue(exception.Message.Contains("https://www.demolink.com"));
+                    Assert.IsTrue(exception.Message.Contains("/demo/path"));
+                    Assert.IsTrue(exception.Message.Contains("/demo/path/with/escape/character"));
+                    Assert.IsFalse(exception.Message.Contains("}"));
+                    Assert.IsFalse(exception.Message.Contains("{"));
+                }
+            }
+        }
+
         [TestMethod]
         public void VerifyDocumentClientExceptionWithNullHeader()
         {