Ben.Demystifier icon indicating copy to clipboard operation
Ben.Demystifier copied to clipboard

Demystify is still run twice on InnerException of an AggregateException

Open fubar-coder opened this issue 7 years ago • 1 comments

When an AggregateException is found, Demystify is run twice in the InnerException. The problematic source code can be seen here in ExceptionExtensions.cs.

According to the source, the InnerException is always the first of the InnerExceptions. You can find the code in lines 77 and 150.

fubar-coder avatar Jan 12 '18 10:01 fubar-coder

Interesting observation.

I agree the source certainly has things that way, and it "makes sense". I suspect it's not in the contract of AggregateException though.

I wonder if it'd be acceptable to change


                if (exception is AggregateException aggEx)
                {
                    foreach (var ex in EnumerableIList.Create(aggEx.InnerExceptions))
                    {
                        ex.Demystify();
                    }
                }

                exception.InnerException?.Demystify();

To something more like

                var innerException = exception.InnerException;
                innerException?.Demystify();

                if (exception is AggregateException aggEx)
                {
                    foreach (var ex in EnumerableIList.Create(aggEx.InnerExceptions))
                    {
                        if (!Object.ReferenceEquals(ex, innerException)) {
                            ex.Demystify();
                        }
                    }
                }

That handles the case described here and any weird descendant of AggregateException that doesn't honour the implicit contract about the first of the inner exceptions being the inner exception.

Note: I haven't read through the Demystifier codebase. I'm just an interested user who likes to read GitHub issues. That said, I don't think Demystifier creates new Exception instances (ie, it mutates them in-place), particularly since the results of the calls to .Demystify() aren't stored anywhere so object references should be the same.

IanYates avatar Jan 14 '18 11:01 IanYates