Handle exceptions thrown by PEReader (#14612)

* Catch and log exceptions from PortablePdbReader.PopulateStackFrame

src/Hosting/Hosting/src/GenericHost/GenericWebHostedService.cs

@@ -173,6 +173,7 @@ namespace Microsoft.AspNetCore.Hosting
             {
                 var exceptionDetailProvider = new ExceptionDetailsProvider(
                     HostingEnvironment.ContentRootFileProvider,
+                    Logger,
                     sourceCodeLineCount: 6);
 
                 model.ErrorDetails = exceptionDetailProvider.GetDetails(exception);

src/Hosting/Hosting/src/Internal/WebHost.cs

@@ -270,6 +270,7 @@ namespace Microsoft.AspNetCore.Hosting
                 {
                     var exceptionDetailProvider = new ExceptionDetailsProvider(
                         hostingEnv.ContentRootFileProvider,
+                        logger,
                         sourceCodeLineCount: 6);
 
                     model.ErrorDetails = exceptionDetailProvider.GetDetails(ex);

src/Middleware/Diagnostics/src/DeveloperExceptionPage/DeveloperExceptionPageMiddleware.cs

@@ -72,7 +72,7 @@ namespace Microsoft.AspNetCore.Diagnostics
             _logger = loggerFactory.CreateLogger<DeveloperExceptionPageMiddleware>();
             _fileProvider = _options.FileProvider ?? hostingEnvironment.ContentRootFileProvider;
             _diagnosticSource = diagnosticSource;
-            _exceptionDetailsProvider = new ExceptionDetailsProvider(_fileProvider, _options.SourceCodeLineCount);
+            _exceptionDetailsProvider = new ExceptionDetailsProvider(_fileProvider, _logger, _options.SourceCodeLineCount);
             _exceptionHandler = DisplayException;
 
             foreach (var filter in filters.Reverse())

src/Middleware/Diagnostics/test/UnitTests/ExceptionDetailsProviderTest.cs

@@ -66,7 +66,7 @@ namespace Microsoft.Extensions.Internal
             using (var provider = new PhysicalFileProvider(rootPath))
             {
                 // Act
-                var exceptionDetailProvider = new ExceptionDetailsProvider(provider, sourceCodeLineCount: 6);
+                var exceptionDetailProvider = new ExceptionDetailsProvider(provider, logger: null, sourceCodeLineCount: 6);
                 var stackFrame = exceptionDetailProvider.GetStackFrameSourceCodeInfo(
                     "func1",
                     absoluteFilePath,
@@ -90,7 +90,7 @@ namespace Microsoft.Extensions.Internal
             using (var provider = new PhysicalFileProvider(rootPath))
             {
                 // Act
-                var exceptionDetailProvider = new ExceptionDetailsProvider(provider, sourceCodeLineCount: 6);
+                var exceptionDetailProvider = new ExceptionDetailsProvider(provider, logger: null, sourceCodeLineCount: 6);
                 var stackFrame = exceptionDetailProvider.GetStackFrameSourceCodeInfo(
                     "func1",
                     relativePath,
@@ -116,7 +116,7 @@ namespace Microsoft.Extensions.Internal
                 baseNamespace: $"{typeof(ExceptionDetailsProviderTest).GetTypeInfo().Assembly.GetName().Name}.Resources");
 
             // Act
-            var exceptionDetailProvider = new ExceptionDetailsProvider(provider, sourceCodeLineCount: 6);
+            var exceptionDetailProvider = new ExceptionDetailsProvider(provider, logger: null, sourceCodeLineCount: 6);
             var stackFrame = exceptionDetailProvider.GetStackFrameSourceCodeInfo(
                 "func1",
                 relativePath,
@@ -259,7 +259,8 @@ namespace Microsoft.Extensions.Internal
             // Act
             var exceptionDetailProvider = new ExceptionDetailsProvider(
                 new PhysicalFileProvider(Directory.GetCurrentDirectory()),
-               sourceCodeLineCount: 6);
+                logger: null,
+                sourceCodeLineCount: 6);
 
             exceptionDetailProvider.ReadFrameContent(
                 stackFrame,

src/Servers/IIS/IIS/src/StartupHook.cs

@@ -81,6 +81,7 @@ internal class StartupHook
 
             var exceptionDetailProvider = new ExceptionDetailsProvider(
                 new PhysicalFileProvider(contentRoot),
+                logger: null,
                 sourceCodeLineCount: 6);
 
             // The startup hook is only present when detailed errors are allowed, so

src/Shared/StackTrace/ExceptionDetails/ExceptionDetailsProvider.cs

@@ -7,17 +7,20 @@ using System.IO;
 using System.Linq;
 using System.Reflection;
 using Microsoft.Extensions.FileProviders;
+using Microsoft.Extensions.Logging;
 
 namespace Microsoft.Extensions.StackTrace.Sources
 {
     internal class ExceptionDetailsProvider
     {
         private readonly IFileProvider _fileProvider;
+        private readonly ILogger _logger;
         private readonly int _sourceCodeLineCount;
 
-        public ExceptionDetailsProvider(IFileProvider fileProvider, int sourceCodeLineCount)
+        public ExceptionDetailsProvider(IFileProvider fileProvider, ILogger logger, int sourceCodeLineCount)
         {
             _fileProvider = fileProvider;
+            _logger = logger;
             _sourceCodeLineCount = sourceCodeLineCount;
         }
 
@@ -30,15 +33,27 @@ namespace Microsoft.Extensions.StackTrace.Sources
                 yield return new ExceptionDetails
                 {
                     Error = ex,
-                    StackFrames = StackTraceHelper.GetFrames(ex)
+                    StackFrames = GetStackFrames(ex),
-                            .Select(frame => GetStackFrameSourceCodeInfo(
-                                frame.MethodDisplayInfo.ToString(),
-                                frame.FilePath,
-                                frame.LineNumber))
                 };
             }
         }
 
+        private IEnumerable<StackFrameSourceCodeInfo> GetStackFrames(Exception original)
+        {
+            var stackFrames = StackTraceHelper.GetFrames(original, out var exception)
+                .Select(frame => GetStackFrameSourceCodeInfo(
+                    frame.MethodDisplayInfo.ToString(),
+                    frame.FilePath,
+                    frame.LineNumber));
+
+            if (exception != null)
+            {
+                _logger?.FailedToReadStackTraceInfo(exception);
+            }
+
+            return stackFrames;
+        }
+
         private static IEnumerable<Exception> FlattenAndReverseExceptionTree(Exception ex)
         {
             // ReflectionTypeLoadException is special because the details are in

src/Shared/StackTrace/ExceptionDetails/LoggerExtensions.cs

@@ -0,0 +1,26 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+using Microsoft.Extensions.Logging;
+
+namespace Microsoft.Extensions.StackTrace.Sources
+{
+    internal static class LoggerExtensions
+    {
+        private static readonly Action<ILogger, Exception> _failedToReadStackFrameInfo;
+
+        static LoggerExtensions()
+        {
+            _failedToReadStackFrameInfo = LoggerMessage.Define(
+                logLevel: LogLevel.Debug,
+                eventId: new EventId(0, "FailedToReadStackTraceInfo"),
+                formatString: "Failed to read stack trace information for exception.");
+        }
+
+        public static void FailedToReadStackTraceInfo(this ILogger logger, Exception exception)
+        {
+            _failedToReadStackFrameInfo(logger, exception);
+        }
+    }
+}

src/Shared/StackTrace/StackFrame/StackTraceHelper.cs

@@ -15,12 +15,13 @@ namespace Microsoft.Extensions.StackTrace.Sources
 {
     internal class StackTraceHelper
     {
-        public static IList<StackFrameInfo> GetFrames(Exception exception)
+        public static IList<StackFrameInfo> GetFrames(Exception exception, out AggregateException error)
         {
             var frames = new List<StackFrameInfo>();
 
             if (exception == null)
             {
+                error = default;
                 return frames;
             }
 
@@ -32,9 +33,12 @@ namespace Microsoft.Extensions.StackTrace.Sources
 
                 if (stackFrames == null)
                 {
+                    error = default;
                     return frames;
                 }
 
+                List<Exception> exceptions = null;
+
                 for (var i = 0; i < stackFrames.Length; i++)
                 {
                     var frame = stackFrames[i];
@@ -56,14 +60,33 @@ namespace Microsoft.Extensions.StackTrace.Sources
 
                     if (string.IsNullOrEmpty(stackFrame.FilePath))
                     {
-                        // .NET Framework and older versions of mono don't support portable PDBs
+                        try
-                        // so we read it manually to get file name and line information
+                        {
-                        portablePdbReader.PopulateStackFrame(stackFrame, method, frame.GetILOffset());
+                            // .NET Framework and older versions of mono don't support portable PDBs
+                            // so we read it manually to get file name and line information
+                            portablePdbReader.PopulateStackFrame(stackFrame, method, frame.GetILOffset());
+                        }
+                        catch (Exception ex)
+                        {
+                            if (exceptions is null)
+                            {
+                                exceptions = new List<Exception>();
+                            }
+
+                            exceptions.Add(ex);
+                        }
                     }
 
                     frames.Add(stackFrame);
                 }
 
+                if (exceptions != null)
+                {
+                    error = new AggregateException(exceptions);
+                    return frames;
+                }
+
+                error = default;
                 return frames;
             }
         }

src/Shared/test/Shared.Tests/StackTraceHelperTest.cs

@@ -33,7 +33,7 @@ namespace Microsoft.Extensions.Internal
             }
 
             // Act
-            var stackFrames = StackTraceHelper.GetFrames(exception);
+            var stackFrames = StackTraceHelper.GetFrames(exception, out _);
 
             // Assert
             Assert.Collection(stackFrames,
@@ -55,7 +55,7 @@ namespace Microsoft.Extensions.Internal
             var exception = Record.Exception(() => GenericMethod<string>(null));
 
             // Act
-            var stackFrames = StackTraceHelper.GetFrames(exception);
+            var stackFrames = StackTraceHelper.GetFrames(exception, out _);
 
             // Assert
             var methods = stackFrames.Select(frame => frame.MethodDisplayInfo.ToString()).ToArray();
@@ -69,7 +69,7 @@ namespace Microsoft.Extensions.Internal
             var exception = Record.Exception(() => MethodWithOutParameter(out var value));
 
             // Act
-            var stackFrames = StackTraceHelper.GetFrames(exception);
+            var stackFrames = StackTraceHelper.GetFrames(exception, out _);
 
             // Assert
             var methods = stackFrames.Select(frame => frame.MethodDisplayInfo.ToString()).ToArray();
@@ -83,7 +83,7 @@ namespace Microsoft.Extensions.Internal
             var exception = Record.Exception(() => MethodWithGenericOutParameter("Test", out int value));
 
             // Act
-            var stackFrames = StackTraceHelper.GetFrames(exception);
+            var stackFrames = StackTraceHelper.GetFrames(exception, out _);
 
             // Assert
             var methods = stackFrames.Select(frame => frame.MethodDisplayInfo.ToString()).ToArray();
@@ -98,7 +98,7 @@ namespace Microsoft.Extensions.Internal
             var exception = Record.Exception(() => MethodWithRefParameter(ref value));
 
             // Act
-            var stackFrames = StackTraceHelper.GetFrames(exception);
+            var stackFrames = StackTraceHelper.GetFrames(exception, out _);
 
             // Assert
             var methods = stackFrames.Select(frame => frame.MethodDisplayInfo.ToString()).ToArray();
@@ -113,7 +113,7 @@ namespace Microsoft.Extensions.Internal
             var exception = Record.Exception(() => MethodWithGenericRefParameter(ref value));
 
             // Act
-            var stackFrames = StackTraceHelper.GetFrames(exception);
+            var stackFrames = StackTraceHelper.GetFrames(exception, out _);
 
             // Assert
             var methods = stackFrames.Select(frame => frame.MethodDisplayInfo.ToString()).ToArray();
@@ -128,7 +128,7 @@ namespace Microsoft.Extensions.Internal
             var exception = Record.Exception(() => MethodWithNullableParameter(value));
 
             // Act
-            var stackFrames = StackTraceHelper.GetFrames(exception);
+            var stackFrames = StackTraceHelper.GetFrames(exception, out _);
 
             // Assert
             var methods = stackFrames.Select(frame => frame.MethodDisplayInfo.ToString()).ToArray();
@@ -142,7 +142,7 @@ namespace Microsoft.Extensions.Internal
             var exception = Record.Exception(() => new GenericClass<int>().Throw(0));
 
             // Act
-            var stackFrames = StackTraceHelper.GetFrames(exception);
+            var stackFrames = StackTraceHelper.GetFrames(exception, out _);
 
             // Assert
             var methods = stackFrames.Select(frame => frame.MethodDisplayInfo.ToString()).ToArray();
@@ -175,7 +175,7 @@ namespace Microsoft.Extensions.Internal
             }
 
             // Act
-            var stackFrames = StackTraceHelper.GetFrames(exception);
+            var stackFrames = StackTraceHelper.GetFrames(exception, out _);
             var methodNames = stackFrames.Select(stackFrame => stackFrame.MethodDisplayInfo.ToString()).ToArray();
 
             // Assert
@@ -189,7 +189,7 @@ namespace Microsoft.Extensions.Internal
             var exception = Record.Exception(() => InvokeMethodOnTypeWithStackTraceHiddenAttribute());
 
             // Act
-            var stackFrames = StackTraceHelper.GetFrames(exception);
+            var stackFrames = StackTraceHelper.GetFrames(exception, out _);
 
             // Assert
             var methods = stackFrames.Select(frame => frame.MethodDisplayInfo.ToString()).ToArray();
@@ -204,7 +204,7 @@ namespace Microsoft.Extensions.Internal
             var exception = Record.Exception(() => InvokeStaticMethodOnTypeWithStackTraceHiddenAttribute());
 
             // Act
-            var stackFrames = StackTraceHelper.GetFrames(exception);
+            var stackFrames = StackTraceHelper.GetFrames(exception, out _);
 
             // Assert
             var methods = stackFrames.Select(frame => frame.MethodDisplayInfo.ToString()).ToArray();
@@ -219,7 +219,7 @@ namespace Microsoft.Extensions.Internal
             var exception = Record.Exception(() => new TypeWithMethodWithStackTraceHiddenAttribute().Throw());
 
             // Act
-            var stackFrames = StackTraceHelper.GetFrames(exception);
+            var stackFrames = StackTraceHelper.GetFrames(exception, out _);
 
             // Assert
             var methods = stackFrames.Select(frame => frame.MethodDisplayInfo.ToString()).ToArray();
@@ -237,7 +237,7 @@ namespace Microsoft.Extensions.Internal
             var exception = Record.Exception(action);
 
             // Act
-            var frames = StackTraceHelper.GetFrames(exception).ToArray();
+            var frames = StackTraceHelper.GetFrames(exception, out _).ToArray();
 
             // Assert
             var frame = frames[0];