Skip to content

Handle exceptions thrown by PEReader #14612

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 8, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ private RequestDelegate BuildErrorPageApplication(Exception exception)
{
var exceptionDetailProvider = new ExceptionDetailsProvider(
HostingEnvironment.ContentRootFileProvider,
Logger,
sourceCodeLineCount: 6);

model.ErrorDetails = exceptionDetailProvider.GetDetails(exception);
Expand Down
1 change: 1 addition & 0 deletions src/Hosting/Hosting/src/Internal/WebHost.cs
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,7 @@ private RequestDelegate BuildApplication()
{
var exceptionDetailProvider = new ExceptionDetailsProvider(
hostingEnv.ContentRootFileProvider,
logger,
sourceCodeLineCount: 6);

model.ErrorDetails = exceptionDetailProvider.GetDetails(ex);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public DeveloperExceptionPageMiddleware(
_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())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public void DisplaysSourceCodeLines_ForAbsolutePaths(string absoluteFilePath)
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,
Expand All @@ -90,7 +90,7 @@ public void DisplaysSourceCodeLines_ForRelativePaths(string relativePath)
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,
Expand All @@ -116,7 +116,7 @@ public void DisplaysSourceCodeLines_ForRelativeEmbeddedPaths(string relativePath
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,
Expand Down Expand Up @@ -259,7 +259,8 @@ public void DisplaysSourceCodeLines_PreAndPostErrorLine(ErrorData errorData)
// Act
var exceptionDetailProvider = new ExceptionDetailsProvider(
new PhysicalFileProvider(Directory.GetCurrentDirectory()),
sourceCodeLineCount: 6);
logger: null,
sourceCodeLineCount: 6);

exceptionDetailProvider.ReadFrameContent(
stackFrame,
Expand Down
1 change: 1 addition & 0 deletions src/Servers/IIS/IIS/src/StartupHook.cs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ public static void Initialize()

var exceptionDetailProvider = new ExceptionDetailsProvider(
new PhysicalFileProvider(contentRoot),
logger: null,
sourceCodeLineCount: 6);

// The startup hook is only present when detailed errors are allowed, so
Expand Down
27 changes: 21 additions & 6 deletions src/Shared/StackTrace/ExceptionDetails/ExceptionDetailsProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,20 @@
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;
}

Expand All @@ -30,15 +33,27 @@ public IEnumerable<ExceptionDetails> GetDetails(Exception exception)
yield return new ExceptionDetails
{
Error = ex,
StackFrames = StackTraceHelper.GetFrames(ex)
.Select(frame => GetStackFrameSourceCodeInfo(
frame.MethodDisplayInfo.ToString(),
frame.FilePath,
frame.LineNumber))
StackFrames = GetStackFrames(ex),
};
}
}

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
Expand Down
26 changes: 26 additions & 0 deletions src/Shared/StackTrace/ExceptionDetails/LoggerExtensions.cs
Original file line number Diff line number Diff line change
@@ -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);
}
}
}
31 changes: 27 additions & 4 deletions src/Shared/StackTrace/StackFrame/StackTraceHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -32,9 +33,12 @@ public static IList<StackFrameInfo> GetFrames(Exception exception)

if (stackFrames == null)
{
error = default;
return frames;
}

List<Exception> exceptions = null;

for (var i = 0; i < stackFrames.Length; i++)
{
var frame = stackFrames[i];
Expand All @@ -56,14 +60,33 @@ public static IList<StackFrameInfo> GetFrames(Exception exception)

if (string.IsNullOrEmpty(stackFrame.FilePath))
{
// .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());
try
{
// .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;
}
}
Expand Down
26 changes: 13 additions & 13 deletions src/Shared/test/Shared.Tests/StackTraceHelperTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public void StackTraceHelper_IncludesLineNumbersForFiles()
}

// Act
var stackFrames = StackTraceHelper.GetFrames(exception);
var stackFrames = StackTraceHelper.GetFrames(exception, out _);

// Assert
Assert.Collection(stackFrames,
Expand All @@ -55,7 +55,7 @@ public void StackTraceHelper_PrettyPrintsStackTraceForGenericMethods()
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();
Expand All @@ -69,7 +69,7 @@ public void StackTraceHelper_PrettyPrintsStackTraceForMethodsWithOutParameters()
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();
Expand All @@ -83,7 +83,7 @@ public void StackTraceHelper_PrettyPrintsStackTraceForMethodsWithGenericOutParam
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();
Expand All @@ -98,7 +98,7 @@ public void StackTraceHelper_PrettyPrintsStackTraceForMethodsWithRefParameters()
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();
Expand All @@ -113,7 +113,7 @@ public void StackTraceHelper_PrettyPrintsStackTraceForMethodsWithGenericRefParam
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();
Expand All @@ -128,7 +128,7 @@ public void StackTraceHelper_PrettyPrintsStackTraceForMethodsWithNullableParamet
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();
Expand All @@ -142,7 +142,7 @@ public void StackTraceHelper_PrettyPrintsStackTraceForMethodsOnGenericTypes()
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();
Expand Down Expand Up @@ -175,7 +175,7 @@ public void StackTraceHelper_ProducesReadableOutput()
}

// Act
var stackFrames = StackTraceHelper.GetFrames(exception);
var stackFrames = StackTraceHelper.GetFrames(exception, out _);
var methodNames = stackFrames.Select(stackFrame => stackFrame.MethodDisplayInfo.ToString()).ToArray();

// Assert
Expand All @@ -189,7 +189,7 @@ public void StackTraceHelper_DoesNotIncludeInstanceMethodsOnTypesWithStackTraceH
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();
Expand All @@ -204,7 +204,7 @@ public void StackTraceHelper_DoesNotIncludeStaticMethodsOnTypesWithStackTraceHid
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();
Expand All @@ -219,7 +219,7 @@ public void StackTraceHelper_DoesNotIncludeMethodsWithStackTraceHiddenAttribute(
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();
Expand All @@ -237,7 +237,7 @@ public void GetFrames_DoesNotFailForDynamicallyGeneratedAssemblies()
var exception = Record.Exception(action);

// Act
var frames = StackTraceHelper.GetFrames(exception).ToArray();
var frames = StackTraceHelper.GetFrames(exception, out _).ToArray();

// Assert
var frame = frames[0];
Expand Down