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: Adds diagnostics to message for 408, 500, 503, 404/1002 scenarios #2719

Merged
merged 15 commits into from
Sep 10, 2021
Merged
Show file tree
Hide file tree
Changes from 8 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
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ namespace Microsoft.Azure.Cosmos
public class CosmosException : Exception
{
private readonly string stackTrace;
private readonly Lazy<string> lazyMessage;

internal CosmosException(
HttpStatusCode statusCode,
Expand All @@ -26,17 +27,21 @@ internal CosmosException(
ITrace trace,
Error error,
Exception innerException)
: base(CosmosException.GetMessageHelper(
statusCode,
headers,
message), innerException)
// The message is overridden. Base exception does not have CTOR for just innerException and the property is not virtual
: base(string.Empty, innerException)
{
this.ResponseBody = message;
this.stackTrace = stackTrace;
this.StatusCode = statusCode;
this.Headers = headers ?? new Headers();
this.Error = error;
this.Trace = trace;
this.Diagnostics = new CosmosTraceDiagnostics(this.Trace ?? NoOpTrace.Singleton);
this.lazyMessage = new Lazy<string>(() => GetMessageHelper(
j82w marked this conversation as resolved.
Show resolved Hide resolved
statusCode,
this.Headers,
this.ResponseBody,
this.Diagnostics));
}

/// <summary>
Expand All @@ -59,6 +64,7 @@ public CosmosException(
this.StatusCode = statusCode;
this.ResponseBody = message;
this.Trace = NoOpTrace.Singleton;
this.lazyMessage = new Lazy<string>(() => message);
this.Headers = new Headers()
{
SubStatusCode = (SubStatusCodes)subStatusCode,
Expand All @@ -71,6 +77,9 @@ public CosmosException(
}
}

/// <inheritdoc/>
public override string Message => this.lazyMessage.Value;

/// <summary>
/// The body of the cosmos response message as a string
/// </summary>
Expand Down Expand Up @@ -117,7 +126,7 @@ public CosmosException(
/// <summary>
/// Gets the diagnostics for the request
/// </summary>
public virtual CosmosDiagnostics Diagnostics => new CosmosTraceDiagnostics(this.Trace ?? NoOpTrace.Singleton);
public virtual CosmosDiagnostics Diagnostics { get; }

/// <inheritdoc/>
public override string StackTrace
Expand Down Expand Up @@ -189,32 +198,60 @@ internal ResponseMessage ToCosmosResponseMessage(RequestMessage request)
private static string GetMessageHelper(
HttpStatusCode statusCode,
Headers headers,
string responseBody)
string responseBody,
CosmosDiagnostics diagnostics)
{
StringBuilder stringBuilder = new StringBuilder();

CosmosException.AppendMessageWithoutDiagnostics(
stringBuilder,
statusCode,
headers,
responseBody);

// Include the diagnostics for exceptions where it is critical
// to root cause failures.
if (statusCode == HttpStatusCode.RequestTimeout
|| statusCode == HttpStatusCode.InternalServerError
|| statusCode == HttpStatusCode.ServiceUnavailable)
j82w marked this conversation as resolved.
Show resolved Hide resolved
{
stringBuilder.Append(" Diagnostics:");
j82w marked this conversation as resolved.
Show resolved Hide resolved
stringBuilder.Append(diagnostics.ToString());
}

return stringBuilder.ToString();
}

private static void AppendMessageWithoutDiagnostics(
StringBuilder stringBuilder,
HttpStatusCode statusCode,
Headers headers,
string responseBody)
{
stringBuilder.Append($"Response status code does not indicate success: ");
stringBuilder.Append($"{statusCode} ({(int)statusCode})");
stringBuilder.Append("; Substatus: ");
stringBuilder.Append(headers?.SubStatusCodeLiteral ?? "0" );
stringBuilder.Append(headers?.SubStatusCodeLiteral ?? "0");
stringBuilder.Append("; ActivityId: ");
stringBuilder.Append(headers?.ActivityId ?? string.Empty);
stringBuilder.Append("; Reason: (");
stringBuilder.Append(responseBody ?? string.Empty);
stringBuilder.Append(");");

return stringBuilder.ToString();
}

private string ToStringHelper(
StringBuilder stringBuilder)
StringBuilder stringBuilder)
{
if (stringBuilder == null)
{
throw new ArgumentNullException(nameof(stringBuilder));
}

stringBuilder.Append(this.Message);
CosmosException.AppendMessageWithoutDiagnostics(
stringBuilder,
this.StatusCode,
this.Headers,
this.ResponseBody);
stringBuilder.AppendLine();

if (this.InnerException != null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ namespace Microsoft.Azure.Cosmos
public class CosmosOperationCanceledException : OperationCanceledException
{
private readonly OperationCanceledException originalException;
private readonly Lazy<string> lazyMessage;

/// <summary>
/// Create an instance of CosmosOperationCanceledException
Expand All @@ -30,6 +31,7 @@ public CosmosOperationCanceledException(
{
this.originalException = originalException ?? throw new ArgumentNullException(nameof(originalException));
this.Diagnostics = diagnostics ?? throw new ArgumentNullException(nameof(diagnostics));
this.lazyMessage = this.CreateLazyMessage();
}

internal CosmosOperationCanceledException(
Expand All @@ -45,6 +47,7 @@ internal CosmosOperationCanceledException(

trace.AddDatum("Operation Cancelled Exception", originalException);
this.Diagnostics = new CosmosTraceDiagnostics(trace);
this.lazyMessage = this.CreateLazyMessage();
}

/// <inheritdoc/>
Expand All @@ -55,7 +58,7 @@ public override string Source
}

/// <inheritdoc/>
public override string Message => this.originalException.Message;
public override string Message => this.lazyMessage.Value;

/// <inheritdoc/>
public override string StackTrace => this.originalException.StackTrace;
Expand Down Expand Up @@ -86,5 +89,10 @@ public override string ToString()
{
return $"{this.originalException} {Environment.NewLine}CosmosDiagnostics: {this.Diagnostics}";
}

private Lazy<string> CreateLazyMessage()
{
return new Lazy<string>(() => $"{this.originalException.Message} {Environment.NewLine}CosmosDiagnostics: {this.Diagnostics}");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,9 @@ await container.CreateItemAsync<ToDoActivity>(
string diagnostics = ce.Diagnostics.ToString();
string toString = ce.ToString();
Assert.IsTrue(toString.Contains(diagnostics));
Assert.IsTrue(toString.Contains(message));
Assert.IsTrue(message.Contains(diagnostics));
string messageWithoutDiagnostics = message.Substring(0, message.IndexOf(Environment.NewLine)).Trim();
Assert.IsTrue(toString.Contains(messageWithoutDiagnostics));
}

try
Expand All @@ -115,13 +117,13 @@ await container.CreateItemAsync<ToDoActivity>(
}
catch (CosmosOperationCanceledException ce)
{
Assert.IsNotNull(ce);
string message = ce.Message;
string diagnostics = ce.Diagnostics.ToString();
Assert.IsTrue(diagnostics.Contains("The operation was canceled."));
string toString = ce.ToString();
Assert.IsTrue(toString.Contains(diagnostics));
Assert.IsTrue(toString.Contains(message));
Assert.IsTrue(message.Contains(diagnostics));
string messageWithoutDiagnostics = message.Substring(0, message.IndexOf(Environment.NewLine)).Trim();
Assert.IsTrue(toString.Contains(messageWithoutDiagnostics));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@
"MockedItemBenchmark.DeleteItemExists;[Type=OfTWithClientTelemetryEnabled]": 39840.0,
"MockedItemBenchmark.DeleteItemExists;[Type=OfTWithDiagnosticsToString]": 70924.0,
"MockedItemBenchmark.DeleteItemExists;[Type=Stream]": 29872.0,
"MockedItemBenchmark.DeleteItemNotExists;[Type=OfT]": 48270.0,
"MockedItemBenchmark.DeleteItemNotExists;[Type=OfTCustom]": 48264.0,
"MockedItemBenchmark.DeleteItemNotExists;[Type=OfTWithClientTelemetryEnabled]": 51146.0,
"MockedItemBenchmark.DeleteItemNotExists;[Type=OfTWithDiagnosticsToString]": 82144.0,
"MockedItemBenchmark.DeleteItemNotExists;[Type=Stream]": 43180.0,
"MockedItemBenchmark.DeleteItemNotExists;[Type=OfT]": 42786.0,
"MockedItemBenchmark.DeleteItemNotExists;[Type=OfTCustom]": "43144.0",
"MockedItemBenchmark.DeleteItemNotExists;[Type=OfTWithClientTelemetryEnabled]": 44694.0,
"MockedItemBenchmark.DeleteItemNotExists;[Type=OfTWithDiagnosticsToString]": 77632.0,
"MockedItemBenchmark.DeleteItemNotExists;[Type=Stream]": 37610.0,
"MockedItemBenchmark.ReadFeed;[Type=OfT]": 560692.0,
"MockedItemBenchmark.ReadFeed;[Type=OfTCustom]": 571334.0,
"MockedItemBenchmark.ReadFeed;[Type=OfTWithClientTelemetryEnabled]": 565196.0,
Expand All @@ -26,11 +26,11 @@
"MockedItemBenchmark.ReadItemExists;[Type=OfTWithClientTelemetryEnabled]": 44440.0,
"MockedItemBenchmark.ReadItemExists;[Type=OfTWithDiagnosticsToString]": 76298.0,
"MockedItemBenchmark.ReadItemExists;[Type=Stream]": 34464.0,
"MockedItemBenchmark.ReadItemNotExists;[Type=OfT]": 52826.0,
"MockedItemBenchmark.ReadItemNotExists;[Type=OfTCustom]": 52864.0,
"MockedItemBenchmark.ReadItemNotExists;[Type=OfTWithClientTelemetryEnabled]": 55754.0,
"MockedItemBenchmark.ReadItemNotExists;[Type=OfTWithDiagnosticsToString]": 87640.0,
"MockedItemBenchmark.ReadItemNotExists;[Type=Stream]": 47994.0,
"MockedItemBenchmark.ReadItemNotExists;[Type=OfT]": 47408.0,
"MockedItemBenchmark.ReadItemNotExists;[Type=OfTCustom]": 47406.0,
"MockedItemBenchmark.ReadItemNotExists;[Type=OfTWithClientTelemetryEnabled]": 49298.0,
"MockedItemBenchmark.ReadItemNotExists;[Type=OfTWithDiagnosticsToString]": 81450.0,
"MockedItemBenchmark.ReadItemNotExists;[Type=Stream]": 42246.0,
"MockedItemBenchmark.UpdateItem;[Type=OfT]": 45542.0,
"MockedItemBenchmark.UpdateItem;[Type=OfTCustom]": 45532.0,
"MockedItemBenchmark.UpdateItem;[Type=OfTWithClientTelemetryEnabled]": 48372.0,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2446,9 +2446,11 @@
"Attributes": [],
"MethodInfo": "Microsoft.Azure.Cosmos.CosmosDiagnostics Diagnostics;CanRead:True;CanWrite:False;Microsoft.Azure.Cosmos.CosmosDiagnostics get_Diagnostics();IsAbstract:False;IsStatic:False;IsVirtual:True;IsGenericMethod:False;IsConstructor:False;IsFinal:False;"
},
"Microsoft.Azure.Cosmos.CosmosDiagnostics get_Diagnostics()": {
"Microsoft.Azure.Cosmos.CosmosDiagnostics get_Diagnostics()[System.Runtime.CompilerServices.CompilerGeneratedAttribute()]": {
"Type": "Method",
"Attributes": [],
"Attributes": [
"CompilerGeneratedAttribute"
],
"MethodInfo": "Microsoft.Azure.Cosmos.CosmosDiagnostics get_Diagnostics();IsAbstract:False;IsStatic:False;IsVirtual:True;IsGenericMethod:False;IsConstructor:False;IsFinal:False;"
},
"Microsoft.Azure.Cosmos.Headers get_Headers()[System.Runtime.CompilerServices.CompilerGeneratedAttribute()]": {
Expand Down Expand Up @@ -2495,6 +2497,11 @@
"Attributes": [],
"MethodInfo": "System.String get_ActivityId();IsAbstract:False;IsStatic:False;IsVirtual:True;IsGenericMethod:False;IsConstructor:False;IsFinal:False;"
},
"System.String get_Message()": {
"Type": "Method",
"Attributes": [],
"MethodInfo": "System.String get_Message();IsAbstract:False;IsStatic:False;IsVirtual:True;IsGenericMethod:False;IsConstructor:False;IsFinal:False;"
},
"System.String get_ResponseBody()[System.Runtime.CompilerServices.CompilerGeneratedAttribute()]": {
"Type": "Method",
"Attributes": [
Expand All @@ -2507,6 +2514,11 @@
"Attributes": [],
"MethodInfo": "System.String get_StackTrace();IsAbstract:False;IsStatic:False;IsVirtual:True;IsGenericMethod:False;IsConstructor:False;IsFinal:False;"
},
"System.String Message": {
"Type": "Property",
"Attributes": [],
"MethodInfo": "System.String Message;CanRead:True;CanWrite:False;System.String get_Message();IsAbstract:False;IsStatic:False;IsVirtual:True;IsGenericMethod:False;IsConstructor:False;IsFinal:False;"
},
"System.String ResponseBody": {
"Type": "Property",
"Attributes": [],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,61 @@ public void VerifyHeaderAlwaysExists()
Assert.IsNotNull(cosmosException.Headers, "Header should always be created to avoid null refs caused by users always expecting it to be there");
}

[TestMethod]
public void VerifyDiagnosticsInTimeoutAndServerError()
{
ITrace trace = NoOpTrace.Singleton;
string diagnosticString = new Diagnostics.CosmosTraceDiagnostics(trace).ToString();

CosmosException cosmosException = new CosmosException(
statusCode: HttpStatusCode.RequestTimeout,
message: "Test",
stackTrace: null,
headers: null,
trace: trace,
error: null,
innerException: null);

Assert.IsTrue(cosmosException.Message.EndsWith(diagnosticString));
Assert.IsTrue(cosmosException.ToString().Contains(diagnosticString));

cosmosException = new CosmosException(
statusCode: HttpStatusCode.InternalServerError,
message: "Test",
stackTrace: null,
headers: null,
trace: trace,
error: null,
innerException: null);

Assert.IsTrue(cosmosException.Message.EndsWith(diagnosticString));
Assert.IsTrue(cosmosException.ToString().Contains(diagnosticString));

cosmosException = new CosmosException(
statusCode: HttpStatusCode.ServiceUnavailable,
message: "Test",
stackTrace: null,
headers: null,
trace: trace,
error: null,
innerException: null);

Assert.IsTrue(cosmosException.Message.EndsWith(diagnosticString));
Assert.IsTrue(cosmosException.ToString().Contains(diagnosticString));

cosmosException = new CosmosException(
statusCode: HttpStatusCode.NotFound,
message: "Test",
stackTrace: null,
headers: null,
trace: trace,
error: null,
innerException: null);

Assert.IsFalse(cosmosException.Message.Contains(diagnosticString));
Assert.IsTrue(cosmosException.ToString().Contains(diagnosticString));
}

[TestMethod]
public void VerifyNullHeaderLogic()
{
Expand Down Expand Up @@ -205,7 +260,7 @@ public void VerifyDocumentClientExceptionWithNullHeader()
public void VerifyDocumentClientExceptionToResponseMessage()
{
string errorMessage = "Test Exception!";
DocumentClientException dce = null;
DocumentClientException dce;
try
{
throw new DocumentClientException(
Expand All @@ -230,7 +285,6 @@ public void VerifyDocumentClientExceptionToResponseMessage()
public void VerifyTransportExceptionToResponseMessage()
{
string errorMessage = "Test Exception!";
DocumentClientException dce = null;
TransportException transportException = new TransportException(
errorCode: TransportErrorCode.ConnectionBroken,
innerException: null,
Expand All @@ -240,6 +294,7 @@ public void VerifyTransportExceptionToResponseMessage()
userPayload: true,
payloadSent: true);

DocumentClientException dce;
try
{
throw new ServiceUnavailableException(
Expand Down Expand Up @@ -379,6 +434,13 @@ private void ValidateExceptionInfo(
Assert.IsTrue(exception.ToString().Contains(message));
string expectedMessage = $"Response status code does not indicate success: {httpStatusCode} ({(int)httpStatusCode}); Substatus: {substatus}; ActivityId: {exception.ActivityId}; Reason: ({message});";

if(httpStatusCode == HttpStatusCode.RequestTimeout
|| httpStatusCode == HttpStatusCode.InternalServerError
|| httpStatusCode == HttpStatusCode.ServiceUnavailable)
{
expectedMessage += " Diagnostics:" + new Diagnostics.CosmosTraceDiagnostics(NoOpTrace.Singleton).ToString();
}

Assert.AreEqual(expectedMessage, exception.Message);

// Verify updating the header updates the exception info
Expand Down