Exceptions captured through `Extensions.Logging` marked as `handled = true`
bitsandfoxes opened this issue · 9 comments
An easy repro is Sentry.Samples.AspNetCore.Blazor.Wasm
:
The sample
throws null
but the error gets captured through Sentry.Extensions.Logging
which calls _hub.CaptureException
which ends up in here getting marked as handled
sentry-dotnet/src/Sentry/Internal/MainExceptionProcessor.cs
Lines 177 to 182 in 721b411
This seems like a bit of an edge case - it's pretty unusual to throw null
. Maybe we leave the logic for the SDK as it is and update the sample to throw an exception instead?
Edit: Turns out throw null
works as a shortcut to throw a NullReferenceException. TIL.
It's not about the throw null
but about our logging integration capturing an exception it receives from the logs and the processor assuming if exception has stacktrace then handled
.
Since we're fairly sure that this is most likely relevant in a WASM context, figuring out how to properly set up global exception handling might make this problem go away on its own. MS Docs
Tricky. When an unhandled exception occurs in WASM... the framework logs these. The logging itself is taken care of by various source generated helper functions like this one for unhandled exceptions that occur during rendering.
Potentially, we could take advantage of the fact that the LogMessageAttribute (and the corresponding source generator that is using this marker attribute) is setting an EventId... in the case of unhandled rendering errors it's 100:
private static partial class Log
{
[LoggerMessage(100, LogLevel.Critical, "Unhandled exception rendering component: {Message}", EventName = "ExceptionRenderingComponent")]
public static partial void UnhandledExceptionRenderingComponent(ILogger logger, string message, Exception exception);
}
There are various places where an error may occur though and rummaging around in the source code, it doesn't look like this pattern is used consistently to throw errors 😞 .
Exceptions for event handlers, for example, appear to be thrown from EventCallbackWorkItem.InvokeAsync but I'm not yet seeing how any exceptions thrown in those tasks result in messages being logged to the default logger.
Potential (partial) fix here: c6ccc52. It will identify the exception from our Sample project but I'm sure there are all kinds of other scenarios where the EventId.Name
might be something else. Probably need to do a bit more testing, trying to generate errors from the various other places where an error may occur (linked above).
we could pass some scope to the logger that the sdk understands as setting the event as unhandled. like logError(...)
or set something in the scope instead?
Potential (partial) fix here: c6ccc52
those strings are guaranteed to be unhanlded/framework captured exceptions ?
feels fragile that way, but if we're not the ones calling logException i don't see another way than using the logger id
feels fragile that way, but if we're not the ones calling logException i don't see another way than using the logger id
Yeah LogException gets called by runtime (e.g. here)... we have no control over it.
Fixed in #3386