Exception in DeveloperProblemDetails.GetErrors() while executing the problem details middleware

[cremor]

I sometimes get the following exception in my server log:

An exception was thrown attempting to execute the problem details middleware.
System.BadImageFormatException: Invalid directory size.
   bei System.Reflection.PortableExecutable.PEReader.ReadDebugDirectory()
   bei Microsoft.Extensions.StackTrace.Sources.PortablePdbReader.GetPdbPath(String assemblyPath)
   bei Microsoft.Extensions.StackTrace.Sources.PortablePdbReader.GetMetadataReader(String assemblyPath)
   bei Microsoft.Extensions.StackTrace.Sources.PortablePdbReader.PopulateStackFrame(StackFrameInfo frameInfo, MethodBase method, Int32 IlOffset)
   bei Microsoft.Extensions.StackTrace.Sources.StackTraceHelper.GetFrames(Exception exception)
   bei Microsoft.Extensions.StackTrace.Sources.ExceptionDetailsProvider.<GetDetails>d__3.MoveNext()
   bei Hellang.Middleware.ProblemDetails.DeveloperProblemDetails.<GetErrors>d__4.MoveNext()
   bei System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
   bei System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   bei Hellang.Middleware.ProblemDetails.DeveloperProblemDetails..ctor(ExceptionProblemDetails problem, IEnumerable`1 details)
   bei Hellang.Middleware.ProblemDetails.ProblemDetailsMiddleware.GetDetails(HttpContext context, Exception error)

I'm not sure how to reproduce it, but from reading the stack trace I have an idea what might happen:

The top methods of the exception call stack indicate that it crashes while reading a PDB file. The problem I see here is that that class name indicates that it tries to read a portable PDB. But my application is running ASP.NET Core 2.2 on the full .NET Framework 4.7.1 since some used libraries are not available as .NET Core or .NET Standard builds. Those libraries also use non-portable PDB files. So I assume the crash happens because it tries to read a non-portable PDB file assuming it is a portable one.

I'm using ProblemDetails package version 3.1.0.

[khellang]

Hmm. That's weird. It's even mentioned in the source:

https://github.com/aspnet/AspNetCore/blob/c84e37f30def9ff0b2d12e877e3de6c283be6145/src/Shared/StackTrace/StackFrame/StackTraceHelper.cs#L59-L61

[khellang]

I think I might have to file an issue upstream for this...

[khellang]

It should support both portable and "full" PDBs, but the check for portable PDBs is below the GetPdbPath call:

https://github.com/aspnet/AspNetCore/blob/23596349092f69890c3efe5b8b9005856cc83db2/src/Shared/StackTrace/StackFrame/PortablePdbReader.cs#L74

[nguerrera]

Doesn't look like it's gotten to reading pdb yet, but throws while reading the dll to get the debug directory info.

I looked quickly at the code wondering if there was something letting go of the PE memory, but it looks ok.

The next thing I'd want to try would be to inspect the dll to see if it is in fact corrupted. Are you able to debug and determine what assemblyPath parameter is here?

Cc @tmat

[cremor]

Are you able to debug and determine what assemblyPath parameter is here?

Debugging should be no problem. But since I'm not sure how to reproduce this reliably, I might have to try for some time.

An idea about assembly "corruption": I'm using the Oracle.ManagedDataAccess library. This assembly is obfuscated. Maybe that causes problems?

[khellang]

I'm using the Oracle.ManagedDataAccess library. This assembly is obfuscated. Maybe that causes problems?

Aha! I knew I'd seen that somewhere when Googling for more info on this; coverlet-coverage/coverlet#189

The latest version of Oracle.ManagedDataAccess.dll (which is of course copied to the output directory for a full framework build) fails to decompile in the latest ILSpy (it raises an exception when it tries reading the debug header with an invalid length)

Smells like Oracle is doing something fishy (won't be the first time 😅)...

[cremor]

I can now confirm that this indeed happens if Oracle.ManagedDataAccess throws an exception. The assemblyPath parameter of the method Microsoft.Extensions.StackTrace.Sources.PortablePdbReader.GetPdbPath() is the "Oracle.ManagedDataAccess.dll" in my bin directory.

So I assume this means this is actually a bug in System.Reflection.PortableExecutable.PEReader.ReadDebugDirectory() and I have to file an issue in dotnet/corefx?
Or is the exception fine (since Oracle "broke" the assembly) and should be handled in Microsoft.Extensions.StackTrace.Sources.PortablePdbReader.GetPdbPath()? Then it would be an issue for aspnet/AspNetCore.

But I think this exception could also be handled better in ProblemDetailsMiddleware. Why does it fall back to returning a StatusCodeProblemDetails if an ExceptionProblemDetails is already available (see here)? If it would return the already available result in the catch block, the developer would at least still get the exception message and stack trace. It feels like this was the plan with commit 58619e1, at least according to the message?

[khellang]

So I assume this means this is actually a bug in System.Reflection.PortableExecutable.PEReader.ReadDebugDirectory() and I have to file an issue in dotnet/corefx?
Or is the exception fine (since Oracle "broke" the assembly) and should be handled in Microsoft.Extensions.StackTrace.Sources.PortablePdbReader.GetPdbPath()? Then it would be an issue for aspnet/AspNetCore.

Yeah, I'm thinking it's the latter. I.e. the PEReader.ReadDebugDirectory() can't read the binary and correctly throws a BadImageFormatException. Would you like to file an issue or should I?

Why does it fall back to returning a StatusCodeProblemDetails if an ExceptionProblemDetails is already available (see here)? If it would return the already available result in the catch block, the developer would at least still get the exception message and stack trace.

Hmm 🤔 I can't remember what my thinking was at the time. I think it was to avoid directly serializing an instance of ExceptionProblemDetails, which includes the Exception directly. Some exceptions might serialize easily, some might not.

I think it would be better to wrap it in a DeveloperProblemDetails and just pass Array.Empty<ExceptionDetails> to not include additional details. That would at least fill in the detail, title, instance and type fields:

Middleware/src/ProblemDetails/DeveloperProblemDetails.cs

Lines 16 to 23 in c454363

	Detail = problem.Detail ?? problem.Error.Message;
	Title = problem.Title ?? TypeNameHelper.GetTypeDisplayName(problem.Error.GetType());
	Instance = problem.Instance ?? GetHelpLink(problem.Error);
	if (!string.IsNullOrEmpty(problem.Type))
	{
	Type = problem.Type;
	}

[cremor]

Would you like to file an issue or should I?

Would be nice if you could do it. Otherwise I'll do it on Monday.

That would at least fill in the detail, title, instance and type fields:

So it wouldn't give the stack trace? I'd say that's the most important information about an exception 😉

[cremor]

@khellang I forgot about this yesterday. I wanted to start with a small sample now, but I already have problems getting the ExceptionDetailsProvider class available in my project. Is that MyGet dev feed really the "official" way to get this class?

[khellang]

Is that MyGet dev feed really the "official" way to get this class?

Yeah, the source packages aren't shipped to NuGet, so there's really no better way to get it 😏

[cremor]

I've now created the issue: dotnet/aspnetcore#14609

Did you already decide if you will also handle the exception in ProblemDetails?

[khellang]

I've now created the issue: dotnet/aspnetcore#14609

Nice! That's a really well written issue! ✨

Did you already decide if you will also handle the exception in ProblemDetails?

It really depends on how fast it's fixed upstream. Since it's just a source package, I can easily pull the latest from MyGet and ship a new version without making source changes to the middleware itself. I'll see if maybe I can fix it myself and send a PR upstream.

[cremor]

Seems like your PR was merged, nice.
But do you have any idea when new packages are created? If I see this correctly, the newest package is from November 2018, which was before the code was moved from the aspnet/Extensions repo to the aspnet/AspNetCore repo.

[khellang]

I've pushed 4.0.0-beta.1 to NuGet. Let me know if that fixes your issue 👍

[cremor]

Works fine, thanks!