Show nested exceptions more clearly by danmoseley · Pull Request #25045 · dotnet/coreclr

danmoseley · 2019-06-09T21:45:37Z

Possible fix for https://github.com/dotnet/corefx/issues/37816

Changes made

Put messages from nested inner exceptions on their own line. This is to avoid the 600 char+ lines.
Format inner exceptions in a central place. This also fixes a bug in AggregateException where it would output one inner exception twice.
Indent output from inner exceptions. This is to make it clear which contain which.
Remove dividing markers for compactness. This is because indent makes them unnecessary.

Did not change

Existing message content. This means that AggregateException's message still aggregates all interior messages to whatever length. (I did tweak ArgumentException message to be a single line though)
Existing ordering of outer->inner messages followed by inner->outer stacks. In the deepest part of the indentation the innermost exception message and stack are still together.

Examples

Should change this from #37815 into more like this which removes duplication and draws the eye to the innermost exception, which in this case is the root cause.

Also this from #38293 to this. Again, half as long and it's easy to see which stack goes with which message.

Output of this test program was this is now this.

Thoughts?

src/System.Private.CoreLib/shared/System/AggregateException.cs

stephentoub · 2019-06-10T01:30:25Z

Can you share an example of what this looks like when one or more of the exceptions have been propagated multiple times via ExceptionDispatchInfo.Throw?

stephentoub · 2019-06-10T01:33:32Z

src/System.Private.CoreLib/shared/System/AggregateException.cs

-                text.Append(m_innerExceptions[i].ToString());
-                text.Append("<---");
-                text.AppendLine();
+                sb.Append(base.InnerExceptionToString(ex));


Having removed all of the separators, what does an AggregateException now look like?

See example linked in top post. I can also add one with an AggE wrapping an AggE.

src/System.Private.CoreLib/shared/System/Exception.cs

danmoseley · 2019-06-10T03:36:10Z

@stephentoub

Can you share an example of what this looks like when one or more of the exceptions have been propagated multiple times via ExceptionDispatchInfo.Throw?

As you saw, I didn't change that banner:

test program
before
after

eg

         at _1.Program.F(String type, String msg)
         at _1.Program.E(String type, String msg)
         at _1.Program.D(String type, String msg)
      --- End of stack trace from previous location where exception was thrown ---
         at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw(Exception source)
         at _1.Program.C(String type, String msg)
         at _1.Program.B(String type, String msg)

It is a bit distracting from the stack itself, though. I wonder whether the text is necessary or useful. Would this be an improvement?

         at _1.Program.F(String type, String msg)
         at _1.Program.E(String type, String msg)
         at _1.Program.D(String type, String msg)
      --------------------------
         at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw(Exception source)
         at _1.Program.C(String type, String msg)
         at _1.Program.B(String type, String msg)

Or, is the ExceptionDispatchInfo.Throw sufficient, and divider not useful? We already skip it in the async case apparently (along with other shenanigans)

         at _1.Program.F(String type, String msg)
         at _1.Program.E(String type, String msg)
         at _1.Program.D(String type, String msg)
         at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw(Exception source)
         at _1.Program.C(String type, String msg)
         at _1.Program.B(String type, String msg)

Edit: pushed up the change to remove the boundary for discussion sake.
new result

jkotas · 2019-06-10T04:53:49Z

src/System.Private.CoreLib/Resources/Strings.resx

  <data name="EventSource_VarArgsParameterMismatch" xml:space="preserve">
    <value>The parameters to the Event method do not match the parameters to the WriteEvent method. This may cause the event to be displayed incorrectly.</value>
  </data>
-  <data name="Exception_EndOfInnerExceptionStack" xml:space="preserve">


It is not unusual for folks to have parser of the Exception.ToString message - to reformat it or to identify root cause. Here is an example found by a quick github search:

https://github.com/pieceofsummer/Hangfire.StackTrace/blob/45ced0e988bee23f86d71c2fa60997b9210f677a/src/Hangfire.StackTrace/FailedStateRenderer.cs#L146

This change is likely going to break these ... that may be ok, but we do not have much time to react to feedback in 3.0.

Cant we just enable that through configuration. In that way, people can start preparing for it, like it was done with tiered JIT.

We have used AppContext switches less than a dozen times in .NET Core so far. If it is a temporary thing we take out after 3.0 then I do not object. I can use a switch to hide most of this just for a few weeks. Of course that means that few if anyone "benefits" from this in 3.0.

Another option is that I keep the changes to put messages on their own lines, and to avoid duplicating the first inner exception in the AggregateException, but defer the change to indent and defer removing the two dividers.

thoughts?

I moved part into #25185 for 3.0, and will pick this up again when master is open for .NET 5.

danmoseley · 2019-06-20T01:53:14Z

I'll merge and finish this later for 5.0

danmoseley added 2 commits June 9, 2019 14:17

Change ArgumentException message to one line

4cb6fba

Extract common code to Exception

3572733

danmoseley requested review from jkotas and stephentoub June 9, 2019 21:48

jkotas reviewed Jun 9, 2019

View reviewed changes

src/System.Private.CoreLib/shared/System/AggregateException.cs Outdated Show resolved Hide resolved

stephentoub reviewed Jun 10, 2019

View reviewed changes

src/System.Private.CoreLib/shared/System/Exception.cs Outdated Show resolved Hide resolved

stephentoub reviewed Jun 10, 2019

View reviewed changes

src/System.Private.CoreLib/shared/System/Exception.cs Outdated Show resolved Hide resolved

jkotas reviewed Jun 10, 2019

View reviewed changes

danmoseley added 3 commits June 15, 2019 11:58

Prefix inner exception string

c32b26a

Attempt to clean up AE

b0dd864

Remove EDI boundary?

85fd408

danmoseley force-pushed the exformat branch from 5db0063 to 85fd408 Compare June 15, 2019 19:06

danmoseley mentioned this pull request Jun 15, 2019

Exception format cleanup subset #25185

Merged

danmoseley closed this Jun 20, 2019

danmoseley mentioned this pull request Oct 21, 2019

Clarify inner exception and aggregate exception ToString() #27333

Closed

danmoseley deleted the exformat branch June 30, 2020 23:53

danmoseley mentioned this pull request Jun 30, 2020

AggregateException.ToString() prints terrible message dotnet/runtime#28695

Closed

danmoseley mentioned this pull request Feb 13, 2024

VM: Include generic instantiations in stack frames dotnet/runtime#96440

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show nested exceptions more clearly#25045

Show nested exceptions more clearly#25045
danmoseley wants to merge 5 commits intodotnet:masterfrom
danmoseley:exformat

danmoseley commented Jun 9, 2019 •

edited

Loading

Uh oh!

Uh oh!

stephentoub commented Jun 10, 2019 •

edited

Loading

Uh oh!

stephentoub Jun 10, 2019

Uh oh!

danmoseley Jun 10, 2019

Uh oh!

Uh oh!

Uh oh!

danmoseley commented Jun 10, 2019 •

edited

Loading

Uh oh!

jkotas Jun 10, 2019

Uh oh!

redknightlois Jun 10, 2019

Uh oh!

danmoseley Jun 10, 2019 •

edited

Loading

Uh oh!

danmoseley Jun 15, 2019

Uh oh!

danmoseley commented Jun 20, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

danmoseley commented Jun 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes made

Did not change

Examples

Uh oh!

Uh oh!

stephentoub commented Jun 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stephentoub Jun 10, 2019

Choose a reason for hiding this comment

Uh oh!

danmoseley Jun 10, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

danmoseley commented Jun 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jkotas Jun 10, 2019

Choose a reason for hiding this comment

Uh oh!

redknightlois Jun 10, 2019

Choose a reason for hiding this comment

Uh oh!

danmoseley Jun 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danmoseley Jun 15, 2019

Choose a reason for hiding this comment

Uh oh!

danmoseley commented Jun 20, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

danmoseley commented Jun 9, 2019 •

edited

Loading

stephentoub commented Jun 10, 2019 •

edited

Loading

danmoseley commented Jun 10, 2019 •

edited

Loading

danmoseley Jun 10, 2019 •

edited

Loading