Closed alesdvorakcz closed 5 years ago
@alesdvorakcz This would happen in case the SDK got disposed.
Could you please clarify how do you initialize the SDK? Is it via SentrySdk.Init
? If so, how do you handle the IDisposable
returned?
Or do you use UseSentry
on the WebHostBuilder
like the samples?
Did you register ISentryClient
or IHub
with an Autofac module or something? It could be an issue with the lifetime used in the registration.
It would be great if you could have a small reproducible code to make it easier to troubleshoot.
I dont use Static API.
This is my WebHostBuilder:
WebHost.CreateDefaultBuilder(args)
.ConfigureAppConfiguration((context, builder) =>
{
builder
.SetBasePath(context.HostingEnvironment.ContentRootPath)
.AddJsonFile("appsettings.json", optional: false, reloadOnChange: true)
.AddJsonFile($"appsettings.{context.HostingEnvironment.EnvironmentName}.json", optional: true)
.AddEnvironmentVariables();
})
.ConfigureServices(services =>
{
services.AddSingleton<IStartup, Startup>();
})
.UseSentry()
.Build()
.Run();
In config I have only
{
...
"Sentry": {
"Dns": "..."
}
}
Here is my Startup.ConfigureServices where I integrate Autofac:
public IServiceProvider ConfigureServices(IServiceCollection services)
{
var containerBuilder = new ContainerBuilder(); //AutoFac ContainerBuilder
services.Configure<AzureBlobConfig>(Configuration.GetSection("AzureBlob"));
ConfigureIdentityServer(services);
RegisterDbContext(services);
RegisterMvc(services);
RegisterSwagger(services);
IoCHelper.ConfigureContainer(containerBuilder);
containerBuilder.Populate(services);
Container = containerBuilder.Build();
return Container.Resolve<IServiceProvider>();
}
I dont register a client, hub or anything by my own so I dont set any lifetimes to Sentry.
@alesdvorakcz I'm not able to reproduce this issue. I added Autofac to one of the samples:
https://github.com/getsentry/sentry-dotnet/commit/590cf891983a9c4025aa2446c978d64a8f21f2a4
Running it I see:
trce: Sentry.AspNetCore.SentryMiddleware[0]
Sending event 'Sentry.SentryEvent' to Sentry.
info: Sentry.AspNetCore.SentryMiddleware[0]
Event '5259a129-ff0e-4b44-b55a-5dc71e3055e9' queued.
Could you please create a small reproducible repo? I could check that out and debug through.
I will try to locate problem more specifically and send you small repo. Thanks
Can confirm this bug. We are using AspNetCore 2.1 with Ninject. We integrated Ninject following this tutorial. I'm quite sure there is a correlation between the custom DI solution and this error. We also just used the extension method for the WebHostBuilder.
fail: Sentry.ISentryClient[0]
Failure to capture event: System.ObjectDisposedException: Cannot access a disposed object.
Object name: 'SentryClient'.
at Sentry.SentryClient.CaptureEvent(SentryEvent event, Scope scope) in C:\projects\sentry-dotnet\src\Sentry\SentryClient.cs:line 77
at Sentry.Internal.Hub.CaptureEvent(SentryEvent evt, Scope scope) in C:\projects\sentry-dotnet\src\Sentry\Internal\Hub.cs:line 107
14:21:41.425 [1] ERROR Sentry.ISentryClient - Failure to capture event: System.ObjectDisposedException: Cannot access a disposed object.
Object name: 'SentryClient'.
at Sentry.SentryClient.CaptureEvent(SentryEvent event, Scope scope) in C:\projects\sentry-dotnet\src\Sentry\SentryClient.cs:line 77
at Sentry.Internal.Hub.CaptureEvent(SentryEvent evt, Scope scope) in C:\projects\sentry-dotnet\src\Sentry\Internal\Hub.cs:line 107
14:21:41.431 [1] ERROR Sentry.ISentryClient - Failure to capture event: System.ObjectDisposedException: Cannot access a disposed object.
Object name: 'SentryClient'.
at Sentry.SentryClient.CaptureEvent(SentryEvent event, Scope scope) in C:\projects\sentry-dotnet\src\Sentry\SentryClient.cs:line 77
at Sentry.Internal.Hub.CaptureEvent(SentryEvent evt, Scope scope) in C:\projects\sentry-dotnet\src\Sentry\Internal\Hub.cs:line 107
@chris579 it seems to be related then to using a third party DI container which wasn't tested. I've made a branch testing Autofac and could not reproduce the issue.
Could you please create a small reproducible repo? It would really help pinpoint the issue and fix it.
It seems IHub
or ISentryClient
gets resolved from a scope which gets disposed.
This shouldn't be the case since it's registered in the root container as Singleton. It should only be disposed when shutting down the app.
I'll try to create a reproduction sample but the repro I created is working fine. I'll further investigate this issue by taking the Sentry source into my project and debug the client the next days.
I tried to made small repo to isolate problem but it works fine there... maybe it has some connection with other asp services I use (EF Core, ...). I will try to find problem and then I will post it.
Just chiming in to say we also see it with .NET Core 2.0 and Sentry 1.0.0, with exactly the same kind of traces as the initial post, and also simply using UseSentry
with a simple Dsn
definition in appsettings.json
.
A wrinkle is that it only happens when using a dependency injected logger, e.g. passed to a controller using FooController(ILogger<FooController> logger)
.
When doing Services.GetRequiredService<ILogger<FooController>>().LogError(ex, "test")
, everything works as expected, but you then lose all request context.
@prencher I've pushed this branch: https://github.com/getsentry/sentry-dotnet/tree/repro/autofac
Where I plugged in Autofac.
In HomeController.cs
a ILogger<>
is also injected and works as expected.
I can't reproduce this error. It's affecting different setups as we can see in this thread (Ninject, Autofac) but somehow I still can't get to repro the problem in order to fix it.
Would it be possible for someone who can repro this issue to strip off from the solution whatever can't be made public and share the code so we can debug through it?
Just to note that here: we are using the Log4net AspNetCore Integration. I'll try to add logging to my repro attempt, maybe this is causing the bug.
@bruno-garcia For us, we don't use autofac, just standard .NET Core 2.0 with ASP.NET Core 2.0.9, and no other logging providers except for the standard console & debug ones.
When enabling Sentry debug, I can see two separate cases of it starting up, and then one shutting itself down and disposing, which seems to be the one used in the controller DI loggers.
I'm actually traveling until mid next week, but I'll try to create a minimal reproduction when back if necessary.
When enabling Sentry debug, I can see two separate cases of it starting up, and then one shutting itself down and disposing, which seems to be the one used in the controller DI loggers.
Can confirm that, did the same observations when stepping through the code.
I managed to tackle the issue down to Swagger (which is absurdly strange) but I'm still unable to reproduce it in a separate project.
Would it be possible for someone who can repro this issue to strip off from the solution whatever can't be made public and share the code so we can debug through it?
For me this is not possible. Our project is way too big to be able to strip out business relevant parts.
When enabling Sentry debug, I can see two separate cases of it starting up, and then one shutting itself down and disposing, which seems to be the one used in the controller DI loggers.
@chris579 @prencher the two initializations are due to ASP.NET Core creating two containers. It creates an intermediary which gets disposed. There's an issue raised but it's by design: aspnet/Hosting#1499
Also, Damian Edwards mentions it yesterday in the [community standup].(https://www.youtube.com/watch?v=-b59KvkWBUo&feature=youtu.be&t=1798)
For us what this means is that: If an exception happens when the app is starting, depending where it happens the SDK will capture that error and report to Sentry. If no error happens during that first container's lifetime, it gets disposed while disposing the SDK and a new initialization happens after with the final container which exists for the lifetime of the app.
This is the behaviour also in the samples in this repo which don't reproduce the error reported in this issue.
That said I still haven't been able to reproduce the issue. Still don't understand how it happens either.
We also use Swagger, but disabling it does not fix the issue for us, so I think that's a red herring.
I'm having this same issue. For me, it is a difference between running on Linux vs Windows. I develop on Windows but pushed to Linux. As far as I can tell, everything else is the same between the two deployments but the Linux deployment is throwing this same Object Disposed error.
UPDATE: Welp, scratch that - It's no longer giving me issues for some reason on my Linux system after restarting the process.
Can confirm problem is related to Swagger (I am not sure if this is only problem but was the first one I was able to reproduce in small repo) https://github.com/alesdvorakcz/sentry-test
Last commit (added swagger) started to make issues. Before it was working fine.
Hope it helps
@alesdvorakcz I added Swagger
in the commit linked above and still couldn't repro.
I've tried your repro (thank you very much for it) and I could find the culprit:
var provider = services.BuildServiceProvider().GetRequiredService<IApiVersionDescriptionProvider>();
This builds a container which re-initializes the SDK which disposes the previously initialized instance.
The disposed instance is still around in the real container, used by the application where the client created by this intermediary container can only be accessed through: SentrySdk
The reason my attempt to reproduce the issue with swagger didn't work is that I don't build a container like in your sample.
Is this the documented way to add things to SwaggerDoc
? Creating a container like could have side effects (like in this case). Also, note that the container should be disposed (unrelated to this issue though).
Regardless if this is the documented way (IMHO Swashbuckle should expose an overload that passes the ServiceProvider
as an argument to the callback instead) we need to find a work around to this.
Is this the documented way to add things to
SwaggerDoc
? Creating a container like could have side effects (like in this case). Also, note that the container should be disposed (unrelated to this issue though).
Yes, it is. It even is provided as an example from Microsoft for API Versioning with Swashbuckle.
Regardless if this is the documented way (IMHO Swashbuckle should expose an overload that passes the ServiceProvider as an argument to the callback instead) we need to find a work around to this.
Yep. Swashbuckle really needs a solution that doesn't require to build a ServiceProvider
during initialization.
My temporary workaround is to create a property ServiceProvider
in Startup
and assign it at the top of Configure()
.
ServiceProvider = app.ApplicationServices;
The initialization of swagger is happening afterwards then and no additional container is build.
@chris579 glad to hear there's a work around!
This problem is back for me. I do not have Swagger installed, but I am deploying to a Linux platform. For me, that is the difference between Sentry functioning or not functioning properly (works fine on Windows).
I was thinking I might be able to debug this by setting a breakpoint at the Dispose method of the Sentry client and tracing back the reason for the call.
Any insight?
@ryno1234 either you or one of your dependencies is likely building an intermediary container like:
services.BuildServiceProvider()
Details are described here: https://github.com/getsentry/sentry-dotnet/issues/103#issuecomment-435819704
@bruno-garcia , you're absolutely correct. I myself am doing that in my startup code. Odd that sometimes Sentry works fine and other times it does not. Perhaps due to some race condition....
At any rate, do you have a suggested work-around? I have services being registered during startup that I need access to within other startup Func<>
/ Action<>
bodies i.e.
string connectionString = this.Configuration.GetValue<string>("ConnectionString");
// other service configurations go here
services.AddDbContextPool<MyDbContext>(
options => options.UseMySql(connectionString)
);
// We need to use the service provider to access the DB context a little further down in the startup code, so build a service provider now.
var intermediateServiceProvider = services.BuildServiceProvider();
// Add authentication services
services.AddOpenIdConnect("AuthProvider", options =>
{
// Abbreviated....
options.Events = new OpenIdConnectEvents
{
OnTicketReceived = (context) => {
var dbContext = intermediateServiceProvider.GetRequiredService<MyDbContext>();
// Do stuff with DB context.
}
};
}
@ryno1234 my workaround should apply to your case too. Your access on the DbContext should happen after init. There is a race condition occurring that results in the manner that it’s working sometimes.
I‘m quite unsure why Sentry is being disposed when a new ServiceProvider is built. It’s not uncommon (especially for EntityFrameworkCore scenarios) that it is being built additionally to access the DI. IMO there is no reason why the Sentry Client has to behave like a singleton across different ServiceProviders.
@chris579 some integrations are not built with DI in mind and need access to the SentrySdk
static class. When a Hub
instance is first resolved, it creates the instance (initializes the SDK) and binds it to the SentrySdk
static class so that the IHub
or ISentryClient
you get injected somewhere is the same being accessed through SentrySdk
.
This makes sure that breadcrumbs and other things added to the scope are shared by the static context (SentrySdk.AddBreadcrumb
) and also via DI like the SentryMiddleware
's instance of Hub
.
One example is Hangfire
which I haven't had time to build an integration yet but would need to rely on SentrySdk
.
This does not explain why the old sentry client is disposed then. Whatever, this issue has to be resolved by Sentry itself as a lot of common cases are building the ServiceProvider manually to acces the DI.
That is indeed the issue, we do this at the end of our ConfigureServices
call: Services = services.BuildServiceProvider();
, which we have to use a in a few places where we can't currently do DI. This seems like a shortcoming of Sentry, not something that should require a workaround.
@prencher as I mentioned above, we'll need to work around this issue in the SDK itself.
The proposed work around in this thread for you is only until we ship a new version which no longer requires you to work around this issue.
One way I'm thinking we can work around it is simply bind Hub
to SentrySdk
in the SentryMiddleware
. In that case even if multiple containers are built, the last one to resolve the application pipeline is made sure to be the one bound to SentrySdk
.
Ok thanks to you I am using workaround now and it works. My second question related to my sample repo is why exception sended by Microsoft.Extensions.Logging.ILogger.LogError() is not sent to sentry? I have debug mode turned on and I can see only unhandled exception is sent to Sentry but exception catched in BaseController and logged by ILooger is not. Is that correct behavior or not?
By default, if you are not using a logging library that removes our provider, such as Serilog, LogError
sends event unless you configure MinimumEventLevel to something else.
There's a PR open for Serilog support already. It will be merged soon.
Can you please check that repo? https://github.com/alesdvorakcz/sentry-test I am not using anything more than .UseSentry() extension method and buildin logger. No extra configuration is provided
Any traction for this issue being fixed in 1.0.1? This is unfortunately severely limiting the context we get from Sentry, because only the logging integration works, not the ASP.NET part.
Release 1.1 just went out and this bug was not fixed yet. Apologies!
I hope to take care of this sooner rather than later but besides the smaller fixes and features from 1.1 I've been busy with other things outside the .NET SDK.
A late Christmas miracle! Took a while but finally released a package with a fix for this issue.
The SDK will no longer hold on to the last Hub
it creates via the DI container.
Instead, it will use an accessor (Func<IHub>
) so if your app or some framework requires building containers after the SDK is initialized, the new Hub will just take over. That is, so any integration you have enabled (say ASP.NET Core and Serilog, for example) will share state.
One example: crumbs added via the Serilog integration are sent out if the ASP.NET Core integration captures an exception, or vice-versa.
The release is version 1.1.2-beta
. Please give it a try and let give some feedback if there's anything wrong.
https://www.nuget.org/packages/Sentry.AspNetCore/1.1.2-beta
Thanks everyone who helped tracking the error and @alesdvorakcz for giving the repro!
Just wanted to leave a quick followup now that I had some cycles to go back and upgrade our main service using the new SDK: Works great! Thanks so much.
@precher thanks for being so patient for this fix. This one took a while, unfortunately.
Hi, I'm running into this same issue for a WinForms app. Is there a solution for WinForms?
Hi I tried sentry in my asp.net core app. When I turn on Debug mode I can see this exception:
I dont think I have some special setup. Its dotnet core 2.1 web api with EF core and Autofac DI container. Unfortunately I can post here whole project but if you need more info don't hesitate to ask.