From 1e9c56663c95d29a744e2b1a16bf661b6a2e364d Mon Sep 17 00:00:00 2001 From: j82w Date: Tue, 12 Jan 2021 09:15:54 -0800 Subject: [PATCH] CosmosException: Fixes substatuscode to get the correct value instead of 0 when it is not in the enum. (#2110) The document client exception that is thrown internally will return default value if the SubStatusCode is not defined in the enum. This changes the logic to parse and use the int from the headers so the exception always shows the correct value. This allows the SDK to properly handle unexpected or newer substatuscodes that it is not aware of. --- Microsoft.Azure.Cosmos/src/Headers/Headers.cs | 9 +++- .../CosmosExceptionFactory.cs | 25 +++-------- .../CosmosItemTests.cs | 43 +++++++++++++++++++ .../Utils/HttpHandlerHelper.cs | 34 +++++++++++++++ .../CosmosExceptionTests.cs | 13 +----- 5 files changed, 91 insertions(+), 33 deletions(-) create mode 100644 Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Utils/HttpHandlerHelper.cs diff --git a/Microsoft.Azure.Cosmos/src/Headers/Headers.cs b/Microsoft.Azure.Cosmos/src/Headers/Headers.cs index d6483e862e..9369621c04 100644 --- a/Microsoft.Azure.Cosmos/src/Headers/Headers.cs +++ b/Microsoft.Azure.Cosmos/src/Headers/Headers.cs @@ -284,8 +284,7 @@ public virtual string Get(string headerName) /// True or false if the header name existed in the header collection. public virtual bool TryGetValue(string headerName, out string value) { - value = this.CosmosMessageHeaders.Get(headerName); - return value != null; + return this.CosmosMessageHeaders.TryGetValue(headerName, out value); } /// @@ -359,6 +358,12 @@ internal string[] GetValues(string key) internal CosmosMessageHeadersInternal CosmosMessageHeaders { get; } + internal static int GetIntValueOrDefault(string value) + { + int.TryParse(value, out int number); + return number; + } + internal static SubStatusCodes GetSubStatusCodes(string value) { if (uint.TryParse(value, NumberStyles.Integer, CultureInfo.InvariantCulture, out uint nSubStatus)) diff --git a/Microsoft.Azure.Cosmos/src/Resource/CosmosExceptions/CosmosExceptionFactory.cs b/Microsoft.Azure.Cosmos/src/Resource/CosmosExceptions/CosmosExceptionFactory.cs index 1f4065e19f..55287a0e32 100644 --- a/Microsoft.Azure.Cosmos/src/Resource/CosmosExceptions/CosmosExceptionFactory.cs +++ b/Microsoft.Azure.Cosmos/src/Resource/CosmosExceptions/CosmosExceptionFactory.cs @@ -15,22 +15,7 @@ internal static CosmosException Create( DocumentClientException dce, CosmosDiagnosticsContext diagnosticsContext) { - Headers headers = new Headers(); - if (dce.Headers != null) - { - foreach (string key in dce.Headers) - { - string value = dce.Headers[key]; - if (value == null) - { - throw new ArgumentNullException( - message: $"{nameof(key)}: {key};", - innerException: dce); - } - - headers.Add(key, value); - } - } + Headers headers = dce.Headers == null ? new Headers() : new Headers(dce.Headers); HttpStatusCode httpStatusCode; if (dce.StatusCode.HasValue) @@ -48,7 +33,7 @@ internal static CosmosException Create( return CosmosExceptionFactory.Create( httpStatusCode, - (int)dce.GetSubStatus(), + Headers.GetIntValueOrDefault(headers.SubStatusCodeLiteral), dce.Message, dce.StackTrace, dce.ActivityId, @@ -108,7 +93,7 @@ internal static CosmosException Create( return CosmosExceptionFactory.Create( responseMessage.StatusCode, - (int)responseMessage.Headers.SubStatusCode, + Headers.GetIntValueOrDefault(responseMessage.Headers.SubStatusCodeLiteral), errorMessage, responseMessage?.CosmosException?.StackTrace, responseMessage.Headers.ActivityId, @@ -144,7 +129,7 @@ internal static CosmosException Create( return CosmosExceptionFactory.Create( statusCode: documentServiceResponse.StatusCode, - subStatusCode: (int)responseHeaders.SubStatusCode, + subStatusCode: Headers.GetIntValueOrDefault(responseHeaders.SubStatusCodeLiteral), message: errorMessage, stackTrace: null, activityId: responseHeaders.ActivityId, @@ -175,7 +160,7 @@ internal static CosmosException Create( return CosmosExceptionFactory.Create( statusCode: storeResponse.StatusCode, - subStatusCode: (int)headers.SubStatusCode, + subStatusCode: Headers.GetIntValueOrDefault(headers.SubStatusCodeLiteral), message: errorMessage, stackTrace: null, activityId: headers.ActivityId, diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/CosmosItemTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/CosmosItemTests.cs index 63927b7835..d69db3bc2f 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/CosmosItemTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/CosmosItemTests.cs @@ -12,6 +12,8 @@ namespace Microsoft.Azure.Cosmos.SDK.EmulatorTests using System.IO; using System.Linq; using System.Net; + using System.Net.Http; + using System.Net.Http.Headers; using System.Runtime.Serialization; using System.Text; using System.Threading; @@ -131,6 +133,47 @@ public async Task CreateDropItemTest() Assert.IsNotNull(deleteResponse); } + [TestMethod] + public async Task NegativeCreateItemTest() + { + HttpClientHandlerHelper httpHandler = new HttpClientHandlerHelper(); + HttpClient httpClient = new HttpClient(httpHandler); + using CosmosClient client = TestCommon.CreateCosmosClient(x => x.WithHttpClientFactory(() => httpClient)); + + httpHandler.RequestCallBack = (request, cancellation) => + { + if(request.Method == HttpMethod.Get && + request.RequestUri.AbsolutePath == "//addresses/") + { + HttpResponseMessage result = new HttpResponseMessage(HttpStatusCode.Forbidden); + + // Add a substatus code that is not part of the enum. + // This ensures that if the backend adds a enum the status code is not lost. + result.Headers.Add(WFConstants.BackendHeaders.SubStatus, 999999.ToString(CultureInfo.InvariantCulture)); + string payload = JsonConvert.SerializeObject(new Error() { Message = "test message" }); + result.Content = new StringContent(payload, Encoding.UTF8, "application/json"); + return Task.FromResult(result); + } + + return null; + }; + + try + { + ToDoActivity testItem = ToDoActivity.CreateRandomToDoActivity(); + await client.GetContainer(this.database.Id, this.Container.Id).CreateItemAsync(item: testItem); + Assert.Fail("Request should throw exception."); + } + catch(CosmosException ce) when (ce.StatusCode == HttpStatusCode.Forbidden) + { + Assert.AreEqual(999999, ce.SubStatusCode); + string exception = ce.ToString(); + Assert.IsTrue(exception.StartsWith("Microsoft.Azure.Cosmos.CosmosException : Response status code does not indicate success: Forbidden (403); Substatus: 999999; ")); + string diagnostics = ce.Diagnostics.ToString(); + Assert.IsTrue(diagnostics.Contains("\"SubStatusCode\":999999")); + } + } + [TestMethod] public async Task NegativeCreateDropItemTest() { diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Utils/HttpHandlerHelper.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Utils/HttpHandlerHelper.cs new file mode 100644 index 0000000000..86e593eae9 --- /dev/null +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/Utils/HttpHandlerHelper.cs @@ -0,0 +1,34 @@ +//------------------------------------------------------------ +// Copyright (c) Microsoft Corporation. All rights reserved. +//------------------------------------------------------------ + +namespace Microsoft.Azure.Cosmos.SDK.EmulatorTests +{ + using System; + using System.Net.Http; + using System.Threading; + using System.Threading.Tasks; + + public class HttpClientHandlerHelper : DelegatingHandler + { + public HttpClientHandlerHelper() : base(new HttpClientHandler()) + { + } + + public Func> RequestCallBack { get; set; } + + protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + { + if(this.RequestCallBack != null) + { + Task response = this.RequestCallBack(request, cancellationToken); + if(response != null) + { + return response; + } + } + + return base.SendAsync(request, cancellationToken); + } + } +} diff --git a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/CosmosExceptionTests.cs b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/CosmosExceptionTests.cs index 15129ec710..92d331f48b 100644 --- a/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/CosmosExceptionTests.cs +++ b/Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/CosmosExceptionTests.cs @@ -97,17 +97,8 @@ public void VerifyDocumentClientExceptionWithNullHeader() string headerValue = "Test" + Guid.NewGuid(); dce.Headers.Add(headerValue, null); - try - { - ResponseMessage responseMessage = dce.ToCosmosResponseMessage(null); - Assert.Fail("Should throw exception"); - } - catch (ArgumentNullException ane) - { - Assert.IsTrue(ane.ToString().Contains(headerValue)); - Assert.IsTrue(ane.ToString().Contains(errorMessage)); - Assert.IsTrue(ane.InnerException is DocumentClientException); - } + ResponseMessage responseMessage = dce.ToCosmosResponseMessage(null); + Assert.IsNull(responseMessage.Headers.Get(headerValue)); } [TestMethod]