getsentry / sentry-dotnet

Sentry SDK for .NET
https://docs.sentry.io/platforms/dotnet
MIT License
579 stars 206 forks source link

Crash in release mode when using the Logging extensions only #3327

Closed softlion closed 4 months ago

softlion commented 4 months ago

Package

Sentry.Extensions.Logging

.NET Flavor

.NET

.NET Version

8.0.0

OS

iOS

SDK Version

4.4.0

Self-Hosted Sentry Version

No response

Steps to Reproduce

In MauiProgram.cs:

builder
  .UseMauiApp<App>()
  .UseSentry();

builder.Services.Configure<SentryMauiOptions>(o =>
        {
            o.Dsn = "...";
            o.AutoSessionTracking = true;
            o.DisableDiagnosticSourceIntegration();
            o.DisableSystemDiagnosticsMetricsIntegration();

#if !DEBUG
            o.Environment = "production";
#else
            o.Environment = "debug";
#endif

            o.AddExceptionFilterForType<OperationCanceledException>();
            o.MinimumEventLevel = LogLevel.Warning;
        });

builder.Logging
  .SetMinimumLevel(LogLevel.Trace)
  .AddConsoleLogger(_ => {})
  .AddSentry()
  .ConfigureContainer(new AutofacServiceProviderFactory(), builder =>
  {
  });

builder.Services.TryAdd(ServiceDescriptor.Singleton(typeof(ILogger), typeof(Logger<MauiApp>)));

var mauiApp = builder.Build();

mauiApp.Services.GetRequiredService<ILogger>()
  .LogWarning("Starting App (test sentry)");

Expected Result

no crash in release mode (ie: app on testflight)

Actual Result

crash in release mode:

https://perceptivesol.sentry.io/issues/5244249140/?environment=production&query=&referrer=issue-stream&statsPeriod=7d&stream_index=0

when that work in debug mode:

https://perceptivesol.sentry.io/issues/5243314824/?environment=production&environment=debug&query=&referrer=issue-stream&statsPeriod=7d&stream_index=1

If builder.Logging.AddSentry(); is removed, the app works perfectly and crash reports are visible on sentry.io

When builder.Logging.AddSentry(); is there, the app works perfectly in debug mode, but crashes in release mode (ie: when it is on testflight).
The crash report is visible on sentry.io, which means only the Logging extension is an issue.

Note: this app uses Autofac as the default iOC container.
That's what those lines do:

  .ConfigureContainer(new AutofacServiceProviderFactory(), builder =>
  {
  });
jamescrosswell commented 4 months ago

Thanks for the report @softlion. This is probably a duplicate of:

softlion commented 4 months ago

Thanks for the report @softlion. This is probably a duplicate of:

That does not seem the same at all to me.
Key differences:

softlion commented 4 months ago

Here is the full stack trace:

Autofac.Core.Activators.Reflection.NoConstructorsFoundException No constructors on type 'Microsoft.Extensions.Logging.Configuration.LoggingConfiguration' can be found with the constructor finder 'Autofac.Core.Activators.Reflection.DefaultConstructorFinder'.

An exception was thrown while activating Microsoft.Extensions.Logging.Logger1[[Microsoft.Maui.Hosting.MauiApp, Microsoft.Maui, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]] -> Microsoft.Extensions.Logging.LoggerFactory -> λ:Microsoft.Extensions.Logging.ILoggerProvider[] -> Sentry.Extensions.Logging.SentryLoggerProvider -> Microsoft.Extensions.Options.UnnamedOptionsManager1[[Sentry.Extensions.Logging.SentryLoggingOptions, Sentry.Extensions.Logging, Version=4.4.0.0, Culture=neutral, PublicKeyToken=fba2ec45388e2af0]] -> Microsoft.Extensions.Options.OptionsFactory1[[Sentry.Extensions.Logging.SentryLoggingOptions, Sentry.Extensions.Logging, Version=4.4.0.0, Culture=neutral, PublicKeyToken=fba2ec45388e2af0]] -> λ:Microsoft.Extensions.Options.IConfigureOptions1[[Sentry.Extensions.Logging.SentryLoggingOptions, Sentry.Extensions.Logging, Version=4.4.0.0, Culture=neutral, PublicKeyToken=fba2ec45388e2af0]][] -> Sentry.Extensions.Logging.SentryLoggingOptionsSetup -> Microsoft.Extensions.Logging.Configuration.LoggerProviderConfiguration`1[[Sentry.Extensions.Logging.SentryLoggerProvider, Sentry.Extensions.Logging, Version=4.4.0.0, Culture=neutral, PublicKeyToken=fba2ec45388e2af0]] -> Microsoft.Extensions.Logging.Configuration.LoggerProviderConfigurationFactory -> λ:Microsoft.Extensions.Logging.Configuration.LoggingConfiguration[].

I would like to add

[DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(Microsoft.Extensions.Logging.Configuration.LoggingConfiguration))]

But that don't work as LoggingConfiguration is internal.

It may be a bug in the Microsoft.Extensions.Logging.Configuration nuget, as that also happens with the SimpleConsoleLogger (but does not happen with the ConsoleLogger).

A workaround for you may be to NOT use Microsoft.Extensions.Logging.Configuration when it's not needed.

jamescrosswell commented 4 months ago

That does not seem the same at all to me.

You're right - my mistake... we'll try to reproduce.

softlion commented 4 months ago

I switched to seq logging instead of sentry as a temporary workaround. That is working. Maybe because the seq nuget is very simple and does not use the ms logging configuration nuget.

softlion commented 4 months ago

What's curious is, the warnings logs are now logged to sentry too, even if I did not add the sentry logging call to MauiApp.cs

jamescrosswell commented 4 months ago

Hi @softlion, I took another look at this but it's not exactly easy to reproduce. Publishing an app to TestFlight would require setting up provisioning profiles etc.

I wondered if it might be quicker to get you to share the exception details and stack trace that you're seeing or any logs that get captured when you're running this app in TestFlight?

jamescrosswell commented 4 months ago

Apologies, I see you provided details above.

I think I must always be looking into this issue when my brain is fried 🤯 (that or my brain is permanently fried).

jamescrosswell commented 4 months ago

OK so now I'm actually caught up and looking at the exception details that you provided (ages ago, apologies once again)... I'm not sure if this is a Sentry thing or an Autofac thing.

The error appears to be coming from Autofac when it attempts to create an instance of Microsoft.Extensions.Logging.Configuration.LoggingConfiguration:

Autofac.Core.Activators.Reflection.NoConstructorsFoundException No constructors on type 'Microsoft.Extensions.Logging.Configuration.LoggingConfiguration' can be found with the constructor finder 'Autofac.Core.Activators.Reflection.DefaultConstructorFinder'.

And I take it from the workaround that you were looking into, this is related to trimming when building the application in release mode.

Have you tried asking the Autofac folks for suggestions? Perhaps they've already seen this before?

By the way, Sentry should hook up logging in the call to MauiAppBuilder.UseSentry: https://github.com/getsentry/sentry-dotnet/blob/feabf090918b551d8e7c8c24d27c752881623c37/src/Sentry.Maui/SentryMauiAppBuilderExtensions.cs#L56

So I don't think you need the call to ILoggingBuilder.AddSentry (where you configure Autofac as the DI container).

softlion commented 4 months ago

By the way, Sentry should hook up logging in the call to MauiAppBuilder.UseSentry:

Maybe you missed it, but I wrote that it is both logging and reporting crashes when using UseSentry.

When I added Logging.AddSentry() it stopped working in release mode. At that time I did not know that UseSentry() already hooks the logging.

That is curious that logging to sentry is working even when I don't follow your doc for it.

jamescrosswell commented 4 months ago

Maybe you missed it, but I wrote that it is both logging and reporting crashes when using UseSentry.

I tried using the following code to try to reproduce on a simulator (iOS 17.2) and on a physical device, but the app seems to work fine in both cases (running "Release" configuration, not "Debug"):

    public static MauiApp CreateMauiApp()
    {
        var builder = MauiApp.CreateBuilder();
        builder.ConfigureContainer(new AutofacServiceProviderFactory(), autofacBuilder => { });
        builder
            .UseMauiApp<App>()
            .UseSentry(options =>
            {
                // ...
            })
            .ConfigureFonts(fonts =>
            {
                fonts.AddFont("OpenSans-Regular.ttf", "OpenSansRegular");
                fonts.AddFont("OpenSans-Semibold.ttf", "OpenSansSemibold");
            });

        builder.Logging
            .SetMinimumLevel(LogLevel.Trace)
            .AddSentry();

        return builder.Build();
    }

Is that what you see as well? So you only get this issue when running the application in TestFlight?

Without being able to reproduce the problem, it's hard to guess at what might be causing it.

Have you tried without Autofac? Does your app crash in release mode when using Microsoft's DependencyInjection?

softlion commented 4 months ago

Yes.
You can't repro in release mode on simulators. Simulator builds are not constraint.
As of releases, it's only full release that have the issue.

Anyway, I did not call that anymore:

       builder.Logging
            .SetMinimumLevel(LogLevel.Trace)
            .AddSentry();

And I still get sentry logging on sentry's dashboard (ie: ILogger messages). Is that expected ? What's the use of builder.Logging.AddSentry(); then ?

jamescrosswell commented 4 months ago

And I still get sentry logging on sentry's dashboard (ie: ILogger messages). Is that expected ? What's the use of builder.Logging.AddSentry(); then ?

Hi @softlion,

Yes that's what you'd expect. Sentry's MAUI integration (Sentry.Maui) is built on top of the logging integration (Sentry.Extensions.Logging). You should use the MauiAppBuilder.UseSentry extension to instrument MAUI applications as this adds various hooks and event binders automatically for you, in addition to wiring up a sink for ILogger.

The ILogger.AddSentry extension method offers a lower level integration with Sentry that you'd typically use if you weren't using any integrations that extended Sentry.Extensions.Logging. For example, if you had a basic console application and you wanted to capture log events as breadcrumbs/exceptions, you could use Sentry.Extensions.Logging.

So did removing the call to ILogger.AddSentry resolve your issue?

jamescrosswell commented 4 months ago

Closing this for the time being. @softlion feel free to reopen if you think there is still an issue.