Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CosmosException: Fixes substatuscode to get the correct value instead of 0 when it is not in the enum. #2110

Merged
merged 5 commits into from
Jan 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions Microsoft.Azure.Cosmos/src/Headers/Headers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -284,8 +284,7 @@ public virtual string Get(string headerName)
/// <returns>True or false if the header name existed in the header collection.</returns>
public virtual bool TryGetValue(string headerName, out string value)
{
value = this.CosmosMessageHeaders.Get(headerName);
return value != null;
return this.CosmosMessageHeaders.TryGetValue(headerName, out value);
}

/// <summary>
Expand Down Expand Up @@ -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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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,
Expand Down Expand Up @@ -108,7 +93,7 @@ internal static CosmosException Create(

return CosmosExceptionFactory.Create(
responseMessage.StatusCode,
(int)responseMessage.Headers.SubStatusCode,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On a happy path (not exception), the ResponseMessage.Headers.SubStatusCode also has the same issue, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, but that is a internal field. If you are programming against the enum it makes since that it would use the default if you gave it value that wasn't supported by the enum.

Headers.GetIntValueOrDefault(responseMessage.Headers.SubStatusCodeLiteral),
errorMessage,
responseMessage?.CosmosException?.StackTrace,
responseMessage.Headers.ActivityId,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<ToDoActivity>(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()
{
Expand Down
Original file line number Diff line number Diff line change
@@ -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<HttpRequestMessage, CancellationToken, Task<HttpResponseMessage>> RequestCallBack { get; set; }

protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
{
if(this.RequestCallBack != null)
{
Task<HttpResponseMessage> response = this.RequestCallBack(request, cancellationToken);
if(response != null)
{
return response;
}
}

return base.SendAsync(request, cancellationToken);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down