benaadams / Ben.Demystifier

High performance understanding for stack traces (Make error logs more productive)
Apache License 2.0
2.75k stars 118 forks source link

Serialized exceptions not correctly Demystified #152

Open enemaerke opened 3 years ago

enemaerke commented 3 years ago

Exceptions going through the serialization constructor stores the original stacktrace in a private _stackTraceString field (see https://referencesource.microsoft.com/#mscorlib/system/exception.cs,93). This original stacktrace is tested for and returned from the StackTrace property-getter in Exception (see https://referencesource.microsoft.com/#mscorlib/system/exception.cs,352 )

From my observations, Demystifying an exception after serialization will not capture and Demystify this original stacktrace but will instead return a string that is the exception type and the exception message, no stacktrace included.

This is a quick repro (cheating a little bit with the reflection to avoid a serialization pass in this code):

var originalStackTrace = @"   at Some.Service.TestCode(Guid id) in C:\path\class.cs:line 28
   at Some.Service.Start(Guid id) in C:\path\class.cs:line 22
";
var exception = Activator.CreateInstance<NullReferenceException>();
SetPrivateFieldWithReflection(exception, "_message", "Object reference not set to an instance of an object.");
SetPrivateFieldWithReflection(exception, "_stackTraceString", originalStackTrace);

// verify that the Demystified string cuts off the stacktrace but the original StackTrace property retains it
Assert.Equal($"System.NullReferenceException: Object reference not set to an instance of an object.{Environment.NewLine}", exception.ToStringDemystified());
Assert.Equal(originalStackTrace, exception.StackTrace);
enemaerke commented 3 years ago

Working a bit more with this in our context we are hitting this scenario (in a microservice setup):

So we need a way to determine this. We could attempt to determine the number of stackframes, since they are 0 when having crossed the serialization boundary but that would require us to perform either new StackTrace(exception).FrameCount or even new EnhancedStackTrace(exception).FrameCount and thus potentially paying the penalty of obtaining a stacktrace twice. A way to avoid this would be to allow the EnhancedStackTrace to also be demystified. So essentially:

var enhancedStackTrace = new EnhancedStackTrace(exc);
if (enhancedStackTrace.FrameCound > 0) {
  return enhancedStackTrace.ToStringDemystified(exc);   //proposed new method
}

return exc.StackTrace;

This new method hanging off of the EnhancedStackTrace would essentially enable the ToStringDemystified to skip new'ing up another instance of EnhancedStackTrace and saving a stackframe capture. Would there be interest in such an extension?