Exception in DeveloperProblemDetails.GetErrors() while executing the problem details middleware about middleware HOT 16 CLOSED

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.

from middleware.

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

from middleware.

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

from middleware.

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

from middleware.

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

from middleware.

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?

from middleware.

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 😅)...

from middleware.

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?

from middleware.

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:

from middleware.

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 😉

from middleware.

@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?

from middleware.

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 😏

from middleware.

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

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

from middleware.

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.

from middleware.

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

from middleware.

Works fine, thanks!

from middleware.