Make stack traces readable #274

benaadams · 2017-10-25T00:38:26Z

Brings upstream the coreclr PRs which work on Exception.ToString to work on the StackFrames for a far more productive trouble-shooting experience.

"Hide post exception stack frames" dotnet/coreclr#14652
"Resolve iterators and async methods" stacktrace dotnet/coreclr#14655

Microsoft.AspNetCore.Diagnostics works on StackFrames rather than the .ToString() so it won't automatically fix with those PRs (except the "Show raw exception details" bit)

"Hide post exception stack frames" drops

   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() 
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) 
   at System.Runtime.CompilerServices.TaskAwaiter.ValidateEnd(Task task) 
   at System.Runtime.CompilerServices.TaskAwaiter.GetResult()

"Resolve iterators and async methods" converts

Program.<Iterator>d__4.MoveNext()

to the correct method signature

Program.Iterator(Int64 count, Int32 value)+MoveNext()

However it will need a recent build of coreclr to drop the Task/ExceptionDispatchInfo stuff as that's a new Attribute on the methods/Types.

/cc @DamianEdwards @davidfowl

benaadams · 2017-10-25T00:40:03Z

Changes the DeveloperExceptionPage to a more productive

Microsoft.AspNetCore.Mvc.Razor.RazorView.RenderPageCoreAsync(IRazorPage page, ViewContext context)
Microsoft.AspNetCore.Mvc.Razor.RazorView.RenderPageAsync(IRazorPage page, ViewContext context, bool invokeViewStarts)
Microsoft.AspNetCore.Mvc.Razor.RazorView.RenderAsync(ViewContext context)
Microsoft.AspNetCore.Mvc.ViewFeatures.ViewExecutor.ExecuteAsync(ViewContext viewContext, string contentType, Nullable<int> statusCode)
Microsoft.AspNetCore.Mvc.ViewFeatures.ViewExecutor.ExecuteAsync(ActionContext actionContext, IView view, ViewDataDictionary viewData, ITempDataDictionary tempData, string contentType, Nullable<int> statusCode)
Microsoft.AspNetCore.Mvc.ViewResult.ExecuteResultAsync(ActionContext context)
Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.InvokeResultAsync(IActionResult result)
Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.InvokeNextResultFilterAsync()
Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.Rethrow(ResultExecutedContext context)
Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.Next(ref State next, ref Scope scope, ref object state, ref bool isCompleted)
Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.InvokeNextResourceFilter()+MoveNext()
Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.Rethrow(ResourceExecutedContext context)
Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.Next(ref State next, ref Scope scope, ref object state, ref bool isCompleted)
Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.InvokeFilterPipelineAsync()
Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.InvokeAsync()
Microsoft.AspNetCore.Builder.RouterMiddleware.Invoke(HttpContext httpContext)
IdentityServer4.Hosting.IdentityServerMiddleware.Invoke(HttpContext context, IEndpointRouter router, IUserSession session)
Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
Microsoft.AspNetCore.Cors.Infrastructure.CorsMiddleware.Invoke(HttpContext context)
IdentityServer4.Hosting.BaseUrlMiddleware.Invoke(HttpContext context)
Microsoft.AspNetCore.Diagnostics.EntityFrameworkCore.MigrationsEndPointMiddleware.Invoke(HttpContext context)
Microsoft.AspNetCore.Diagnostics.EntityFrameworkCore.DatabaseErrorPageMiddleware.Invoke(HttpContext httpContext)
Microsoft.AspNetCore.Diagnostics.EntityFrameworkCore.DatabaseErrorPageMiddleware.Invoke(HttpContext httpContext)
Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.Invoke(HttpContext context) in DeveloperExceptionPageMiddleware.cs

from a less productive

Microsoft.AspNetCore.Mvc.Razor.RazorView+<RenderPageCoreAsync>d__16.MoveNext()
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() in ExceptionServicesCommon.cs
System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.ValidateEnd(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.GetResult() in TaskAwaiter.cs
Microsoft.AspNetCore.Mvc.Razor.RazorView+<RenderPageAsync>d__15.MoveNext()
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() in ExceptionServicesCommon.cs
System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.ValidateEnd(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.GetResult() in TaskAwaiter.cs
Microsoft.AspNetCore.Mvc.Razor.RazorView+<RenderAsync>d__14.MoveNext()
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() in ExceptionServicesCommon.cs
System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.ValidateEnd(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.GetResult() in TaskAwaiter.cs
Microsoft.AspNetCore.Mvc.ViewFeatures.ViewExecutor+<ExecuteAsync>d__22.MoveNext()
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() in ExceptionServicesCommon.cs
System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.ValidateEnd(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.GetResult() in TaskAwaiter.cs
Microsoft.AspNetCore.Mvc.ViewFeatures.ViewExecutor+<ExecuteAsync>d__21.MoveNext()
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() in ExceptionServicesCommon.cs
System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.ValidateEnd(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.GetResult() in TaskAwaiter.cs
Microsoft.AspNetCore.Mvc.ViewResult+<ExecuteResultAsync>d__26.MoveNext()
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() in ExceptionServicesCommon.cs
System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.ValidateEnd(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.GetResult() in TaskAwaiter.cs
Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker+<InvokeResultAsync>d__19.MoveNext()
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() in ExceptionServicesCommon.cs
System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.ValidateEnd(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.GetResult() in TaskAwaiter.cs
Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker+<InvokeNextResultFilterAsync>d__24.MoveNext()
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() in ExceptionServicesCommon.cs
Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.Rethrow(ResultExecutedContext context)
Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.Next(ref State next, ref Scope scope, ref object state, ref bool isCompleted)
Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker+<InvokeNextResourceFilter>d__22.MoveNext()
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() in ExceptionServicesCommon.cs
Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.Rethrow(ResourceExecutedContext context)
Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.Next(ref State next, ref Scope scope, ref object state, ref bool isCompleted)
Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker+<InvokeFilterPipelineAsync>d__17.MoveNext()
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() in ExceptionServicesCommon.cs
System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.ValidateEnd(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.GetResult() in TaskAwaiter.cs
Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker+<InvokeAsync>d__15.MoveNext()
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() in ExceptionServicesCommon.cs
System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.ValidateEnd(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.GetResult() in TaskAwaiter.cs
Microsoft.AspNetCore.Builder.RouterMiddleware+<Invoke>d__4.MoveNext()
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() in ExceptionServicesCommon.cs
System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.ValidateEnd(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.GetResult() in TaskAwaiter.cs
IdentityServer4.Hosting.IdentityServerMiddleware+<Invoke>d__4.MoveNext()
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() in ExceptionServicesCommon.cs
System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.ValidateEnd(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.GetResult() in TaskAwaiter.cs
Microsoft.AspNetCore.Authentication.AuthenticationMiddleware+<Invoke>d__6.MoveNext()
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() in ExceptionServicesCommon.cs
System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.ValidateEnd(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.GetResult() in TaskAwaiter.cs
Microsoft.AspNetCore.Cors.Infrastructure.CorsMiddleware+<Invoke>d__7.MoveNext()
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() in ExceptionServicesCommon.cs
System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.ValidateEnd(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.GetResult() in TaskAwaiter.cs
IdentityServer4.Hosting.BaseUrlMiddleware+<Invoke>d__3.MoveNext()
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() in ExceptionServicesCommon.cs
System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.ValidateEnd(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.GetResult() in TaskAwaiter.cs
Microsoft.AspNetCore.Diagnostics.EntityFrameworkCore.MigrationsEndPointMiddleware+<Invoke>d__4.MoveNext()
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() in ExceptionServicesCommon.cs
System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.ValidateEnd(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.GetResult() in TaskAwaiter.cs
Microsoft.AspNetCore.Diagnostics.EntityFrameworkCore.DatabaseErrorPageMiddleware+<Invoke>d__6.MoveNext()
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() in ExceptionServicesCommon.cs
Microsoft.AspNetCore.Diagnostics.EntityFrameworkCore.DatabaseErrorPageMiddleware+<Invoke>d__6.MoveNext()
System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() in ExceptionServicesCommon.cs
System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.ValidateEnd(Task task) in TaskAwaiter.cs
System.Runtime.CompilerServices.TaskAwaiter.GetResult() in TaskAwaiter.cs
Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware+<Invoke>d__7.MoveNext() in DeveloperExceptionPageMiddleware.cs

benaadams · 2017-10-25T03:23:45Z

Incorporated feedback from coreclr

pranavkm

Do the tests need to wait for a new build of CoreCLR to be consumed?

pranavkm · 2017-10-25T18:07:13Z

shared/Microsoft.Extensions.StackTrace.Sources/StackFrame/StackTraceHelper.cs

+            foreach (var attibute in method.CustomAttributes)
+            {
+                // internal Attribute, match on name
+                if (attibute.AttributeType.Name == "StackTraceHiddenAttribute")


Should we look up the full name? i.e. System.Diagnostics.StackTraceHiddenAttribute. Also (string.Equals(StringComparison.Ordinal)

operator ==(String a, String b) is Ordinal?

operator ==(String a, String b) => Equals(String a, String b) => EqualsHelper(a, b);

Which is the same function String.Equals(String a, String b, StringComparison.Ordinal) ends up calling: https://github.com/dotnet/coreclr/blob/master/src/mscorlib/src/System/String.Comparison.cs#L904-L952

Should we look up the full name? i.e. System.Diagnostics.StackTraceHiddenAttribute.

Can't add your own then :) Also much longer string to match on and if there was a second one its intent would likely be the same?

Will aim to get it exposed via api review; then can refer to the type directly; and everyone can use it for this purpose (and revisit the matching as can be more efficient then)

Huh, always thought == did InvariantCulture. Nevermind then

pranavkm · 2017-10-25T18:11:46Z

shared/Microsoft.Extensions.StackTrace.Sources/StackFrame/StackTraceHelper.cs

+
+            foreach (var candidateMethod in methods)
+            {
+                var attributes = candidateMethod.GetCustomAttributes<StateMachineAttribute>();


StateMachineAttribute has AllowMultiple=false. Would GetCustomAttribute suffice?

In the coreclr PR they said there is nothing actually preventing multiple attributes being in the il dotnet/coreclr#14655 (comment); so is hardening it against failure.

pranavkm · 2017-10-25T18:15:39Z

Responding to my previous comment seeing

However it will need a recent build of coreclr to drop the Task/ExceptionDispatchInfo stuff as that's a new Attribute on the methods/Types.

Yup.

benaadams · 2017-10-25T18:33:36Z

Do the tests need to wait for a new build of CoreCLR to be consumed?

Statemachine output should change with this PR; the Task noise will need a newer CoreCLR

benaadams · 2017-10-25T18:37:22Z

e.g. with a new build of Microsoft.AspNetCore.Diagnostics with this shared src change will see

Microsoft.AspNetCore.Mvc.Razor.RazorView.RenderAsync(ViewContext context)

instead of

Microsoft.AspNetCore.Mvc.Razor.RazorView+<RenderAsync>d__14.MoveNext()

on the diagnostics error page

benaadams · 2017-10-26T10:31:56Z

@pranavkm added fallbacks for earlier runtimes

pranavkm · 2017-10-26T14:16:17Z

Would this warrant some tests now?

pranavkm · 2017-10-26T14:17:34Z

shared/Microsoft.Extensions.StackTrace.Sources/StackFrame/StackTraceHelper.cs

+                return false;
+            }
+            else if (type == typeof(TaskAwaiter) ||
+                     type == typeof(TaskAwaiter<>) ||


We don't try to align things like this in our code (harder to maintain). Could you let the editor naturally tab these?

pranavkm · 2017-10-26T14:20:10Z

shared/Microsoft.Extensions.StackTrace.Sources/StackFrame/StackTraceHelper.cs

+                    case "HandleNonSuccessAndDebuggerNotification":
+                    case "ThrowForNonSuccess":
+                    case "ValidateEnd":
+                    case "GetResult":


We have a couple of places in Mvc where we manually unwrap tasks using GetAwaiter().GetResult(). What does the stack trace look like in these cases?

Added to test; Method(string value) unwraps this way, output is

System.Collections.Generic.List<T>+Enumerator.MoveNextRare() Microsoft.Extensions.Internal.StackTraceHelperTest.Iterator()+MoveNext() string.Join(string separator, IEnumerable<string> values) Microsoft.Extensions.Internal.StackTraceHelperTest+GenericClass<T>.GenericMethod<V>(ref V value) Microsoft.Extensions.Internal.StackTraceHelperTest.MethodAsync(int value) Microsoft.Extensions.Internal.StackTraceHelperTest.MethodAsync<TValue>(TValue value) Microsoft.Extensions.Internal.StackTraceHelperTest.Method(string value) Microsoft.Extensions.Internal.StackTraceHelperTest.StackTraceHelper_ProducesReadableOutput()

the .ToString() on the Exception is

at System.ThrowHelper.ThrowInvalidOperationException_InvalidOperation_EnumFailedVersion() at System.Collections.Generic.List`1.Enumerator.MoveNextRare() at Microsoft.Extensions.Internal.StackTraceHelperTest.<Iterator>d__5.MoveNext() at System.String.Join(String separator, IEnumerable`1 values) at Microsoft.Extensions.Internal.StackTraceHelperTest.GenericClass`1.GenericMethod[V](V& value) at Microsoft.Extensions.Internal.StackTraceHelperTest.<MethodAsync>d__2.MoveNext() --- End of stack trace from previous location where exception was thrown at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult() at Microsoft.Extensions.Internal.StackTraceHelperTest.<MethodAsync>d__3`1.MoveNext() --- End of stack trace from previous location where exception was thrown at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult() at Microsoft.Extensions.Internal.StackTraceHelperTest.Method(String value) at Microsoft.Extensions.Internal.StackTraceHelperTest.StackTraceHelper_ProducesReadableOutput()

benaadams · 2017-10-27T15:39:26Z

Add test and fixed the generic type resolution issues for types they highlighted

e.g. List<T> would show up as List

Eilon

This looks really cool, a long time overdue!

Eilon · 2017-10-31T17:22:28Z

shared/Microsoft.Extensions.StackTrace.Sources/StackFrame/StackTraceHelper.cs

+                        return false;
+                }
+            }
+            else if (type.FullName == "System.ThrowHelper")


This type is a real type that shows up in real source code, no? Shouldn't it be left in the call stack?

I totally agree removing the "junk" that doesn't show up in anyone's source code (as in, not even in .NET's source code), but I'm less certain about legitimate method calls.

This type is a real type that shows up in real source code, no? Shouldn't it be left in the call stack?

Its just a type to move the throw out of the generic code to reduce instantiation code bloat. So it essentially turns the call stack back into looking like an inline throw (as its just an implementation detail)

e.g. from

System.InvalidOperationException: Collection was modified; enumeration operation may not execute. at System.ThrowHelper.ThrowInvalidOperationException_InvalidOperation_EnumFailedVersion() at System.Collections.Generic.List`1.Enumerator.MoveNextRare()

to

System.InvalidOperationException: Collection was modified; enumeration operation may not execute. at System.Collections.Generic.List`1.Enumerator.MoveNextRare()

It will also be suppressed in future Exception.ToString() dotnet/coreclr#14652 so its just matching ASP.NET's stack frame processing to match that.

Also they will also get hidden when running on latest 2.1.x as it checking for the StackTraceHiddenAttribute so is just bringing Framework inline

Yeah I know ThrowHelper is cruft, but it's explicit cruft that an engineer chose to put in their code. I personally strongly dislike using such helpers because it does affect the stack trace and can potentially inhibit debugging tools from doing their jobs. That is, where did this ArgNullException really come from?

But, I don't see why the ThrowHelper needs to be special cased here. Won't https://github.com/dotnet/coreclr/pull/14652/files#diff-49215540f67fdd24553e4844693948b4R45 resolve it on the new runtime? I'm not sure I like the idea of ASP.NET Core checking for an odd implementation detail of .NET (and especially one which I 100% disagree with 😄).

Removed the removal of ThrowHelper

I personally strongly dislike using such helpers because it does affect the stack trace...

Slides 19-22 😉

Yeah inlining is a real problem in some cases. I just don't like working around "the system's" limitations. You want exceptions to be thrown from where the error happens, but you don't want to because then it won't inline, so you have to write helpers for it instead. So I say go fix .NET! 😄

Eilon · 2017-10-31T17:24:41Z

@pranavkm assigning to you to track & merge when ready.

Eilon

Looks great, thanks @benaadams !

@pranavkm can you take another look to review, then merge?

pranavkm · 2017-11-02T16:39:07Z

Looks like one of the test is broken on AppVeyor:

[xUnit.net 00:00:01.3468453]       Assert.Equal() Failure
[xUnit.net 00:00:01.3469024]       Expected: List<String> ["System.Collections.Generic.List<T>+Enumerator.Move"..., "Microsoft.Extensions.Internal.StackTraceHelperTest"..., "string.Join(string separator, IEnumerable<string> "..., "Microsoft.Extensions.Internal.StackTraceHelperTest"..., "Microsoft.Extensions.Internal.StackTraceHelperTest"..., ...]
[xUnit.net 00:00:01.3470183]       Actual:   WhereEnumerableIterator<String> ["System.ThrowHelper.ThrowInvalidOperationException("..., "System.Collections.Generic.List<T>+Enumerator.Move"..., "Microsoft.Extensions.Internal.StackTraceHelperTest"..., "string.Join(string separator, IEnumerable<string> "..., "Microsoft.Extensions.Internal.StackTraceHelperTest"..., ...]

Also, could you rebase on dev? I've had very limited success using GitHub's UI to resolve merge conflicts and there's one in your PR.

benaadams · 2017-11-07T22:20:15Z

Also, could you rebase on dev?

You're trolling me with these merge clashes; nothing for months then 3 come along at once... 😉

pranavkm · 2017-11-07T22:23:30Z

@benaadams I'm working on getting it sorted. By the way, I'm considering removing the use of List<T> in the stack trace and simulating it with a test type with a StackTraceHidden attribute. It simplifies the expectations without having to be concerned about the runtime the test is targeting.

benaadams · 2017-11-07T22:31:13Z

Have almost rebased; want me to hold off?

pranavkm · 2017-11-07T22:31:47Z

Yes, please 😄

benaadams · 2017-11-08T00:22:59Z

Closing as superseded by #285

Also, add above root check to CreateFileChangeToken

dnfclas added the cla-already-signed label Oct 25, 2017

Make stack traces readable

5542fb7

benaadams force-pushed the readable-stacktraces branch from 13147b2 to 5542fb7 Compare October 25, 2017 08:08

pranavkm reviewed Oct 25, 2017

View reviewed changes

benaadams mentioned this pull request Oct 26, 2017

Resolve iterators and async methods in stacktrace dotnet/coreclr#14655

Merged

Add fallbacks for earlier runtimes

3385225

pranavkm reviewed Oct 26, 2017

View reviewed changes

benaadams force-pushed the readable-stacktraces branch from a759659 to a505819 Compare October 27, 2017 08:45

Add test + generic resolution fixes

1a76154

benaadams force-pushed the readable-stacktraces branch from cdf0fdb to 1a76154 Compare October 27, 2017 09:17

benaadams mentioned this pull request Oct 27, 2017

[Discussion] Method linking dotnet/roslyn#22857

Open

Eilon requested changes Oct 31, 2017

View reviewed changes

Eilon assigned pranavkm Oct 31, 2017

Don't auto exclude ThrowHelper

bd6c742

Eilon approved these changes Nov 1, 2017

View reviewed changes

halter73 mentioned this pull request Nov 7, 2017

Razor View NullReferenceException Stacktraces are very unhelpful dotnet/efcore#10221

Closed

strich mentioned this pull request Nov 7, 2017

Razor View NullReferenceException Stacktraces are very unhelpful aspnet/Razor#1757

Closed

benaadams closed this Nov 8, 2017

benaadams deleted the readable-stacktraces branch November 8, 2017 00:23

natemcmaster pushed a commit that referenced this pull request Nov 5, 2018

Fix: PollingFileChangeToken wasn't watching actual files (#274)

49de885

Also, add above root check to CreateFileChangeToken

dotnet locked as resolved and limited conversation to collaborators May 30, 2023

Make stack traces readable #274

Make stack traces readable #274

Conversation

benaadams commented Oct 25, 2017 • edited

benaadams commented Oct 25, 2017 • edited

benaadams commented Oct 25, 2017

pranavkm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benaadams Oct 25, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pranavkm commented Oct 25, 2017

benaadams commented Oct 25, 2017

benaadams commented Oct 25, 2017

benaadams commented Oct 26, 2017

pranavkm commented Oct 26, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pranavkm Oct 26, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benaadams commented Oct 27, 2017

Eilon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benaadams Oct 31, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Eilon commented Oct 31, 2017

Eilon left a comment

Choose a reason for hiding this comment

pranavkm commented Nov 2, 2017

benaadams commented Nov 7, 2017

pranavkm commented Nov 7, 2017

benaadams commented Nov 7, 2017

pranavkm commented Nov 7, 2017

benaadams commented Nov 8, 2017

benaadams commented Oct 25, 2017 •

edited

benaadams commented Oct 25, 2017 •

edited

benaadams Oct 25, 2017 •

edited

pranavkm Oct 26, 2017 •

edited

benaadams Oct 31, 2017 •

edited