sentry-dotnet icon indicating copy to clipboard operation
sentry-dotnet copied to clipboard

When enabling attaching the request body Sentry throws endless errors

Open Skyppid opened this issue 1 year ago • 2 comments

Package

Sentry

.NET Flavor

.NET

.NET Version

8.x

OS

Linux

SDK Version

4.10.2

Self-Hosted Sentry Version

No response

Steps to Reproduce

We have an API which has both a REST API as well as several background processes which work via Kafka or a background services. In order to better understand issues which are caused by requests from our frontend, we enabled request body capturing. From what I found on the internet this line should be enough to do so:

builder.WebHost.UseSentry(o =>
{
    o.Dsn = "...";
    o.MaxRequestBodySize = RequestSize.Always;
}

Now since we have background processes as well, it seems that Sentry is ignorantly trying to capture the body without checking it's situation. Obviously background services and triggered parts do not have any HttpContext or other features of these. Whenever Sentry tries to create events, it instead fails with the following logs spamming our log files:

[ERROR] | Failed invoking event handler. | System.ObjectDisposedException: IFeatureCollection has been disposed. Object name: 'Collection'. at void Microsoft.AspNetCore.Http.Features.FeatureReferences<TCache>.ThrowContextDisposed() at void Sentry.AspNetCore.ScopeExtensions.Populate(Scope scope, HttpContext context, SentryAspNetCoreOptions options) at void Sentry.AspNetCore.SentryMiddleware.PopulateScope(HttpContext context, Scope scope) at void Sentry.Scope.Evaluate()

Expected Result

No errors from Sentry. If there's noting to capture or no context available it should gracefully ignore that.

Actual Result

Half our logs are Sentry logs telling us that the feature collection was disposed.

Skyppid avatar Sep 19 '24 08:09 Skyppid

Hi @Skyppid , thanks for getting in touch. Are you able to provide a small app that demonstrates this issue reliably? That would definitely help identify and resolve any problems more quickly.

jamescrosswell avatar Sep 19 '24 09:09 jamescrosswell

If there's noting to capture or no context available it should gracefully ignore that.

That's the plan!

There are not that many places where we try accessing the context: We've got the TraceIdentifier https://github.com/getsentry/sentry-dotnet/blob/c892db48962c9160777eb51f723fd206991ebc8c/src/Sentry.AspNetCore/ScopeExtensions.cs#L36

our own ISentryUserFactory https://github.com/getsentry/sentry-dotnet/blob/c892db48962c9160777eb51f723fd206991ebc8c/src/Sentry.AspNetCore/ScopeExtensions.cs#L44

and since we don't have SetBody https://github.com/getsentry/sentry-dotnet/blob/c892db48962c9160777eb51f723fd206991ebc8c/src/Sentry.AspNetCore/ScopeExtensions.cs#L55

in the stacktrace it's not that or https://github.com/getsentry/sentry-dotnet/blob/c892db48962c9160777eb51f723fd206991ebc8c/src/Sentry.AspNetCore/ScopeExtensions.cs#L76 either.

Can we check for context.Response.HasStarted to avoid accessing disposed or inaccessible context features?

bitsandfoxes avatar Sep 20 '24 16:09 bitsandfoxes

Closing this ticket as, without a reproduction, there's not much we can do here.

Feel free to reopen the ticket with instructions on how we can reproduce, if you still believe this is a problem.

Here's what I tried (small modification of the Sentry.Samples.AspNetCore.Basic sample from the repo):

using Sentry.Extensibility;

var builder = WebApplication.CreateBuilder(args);

builder.WebHost.UseSentry(options =>
{
#if !SENTRY_DSN_DEFINED_IN_ENV
    // A DSN is required. You can set here in code, or you can set it in the SENTRY_DSN environment variable.
    // See https://docs.sentry.io/product/sentry-basics/dsn-explainer/
    options.Dsn = SamplesShared.Dsn;
#endif

    // Enable Sentry performance monitoring
    options.TracesSampleRate = 1.0;

#if DEBUG
    // Log debug information about the Sentry SDK
    options.Debug = true;
    options.MaxRequestBodySize = RequestSize.Always;
#endif
});

// Register the background service
builder.Services.AddHostedService<SampleBackgroundService>();

var app = builder.Build();

// An example ASP.NET Core endpoint that throws an exception when serving a request to path: /throw
app.MapGet("/throw/{message?}", context =>
{
    var exceptionMessage = context.GetRouteValue("message") as string;

    var log = context.RequestServices.GetRequiredService<ILoggerFactory>()
        .CreateLogger<Program>();

    log.LogInformation("Handling some request...");

    var hub = context.RequestServices.GetRequiredService<IHub>();
    hub.ConfigureScope(s =>
    {
        // More data can be added to the scope like this:
        s.SetTag("Sample", "ASP.NET Core"); // indexed by Sentry
        s.SetExtra("Extra!", "Some extra information");
    });

    log.LogInformation("Logging info...");
    log.LogWarning("Logging some warning!");

    // The following exception will be captured by the SDK and the event
    // will include the Log messages and any custom scope modifications
    // as exemplified above.
    throw new Exception(
        exceptionMessage ?? "An exception thrown from the ASP.NET Core pipeline");
});

// Demonstrates how to add tracing in custom middleware
app.Use(async (context, next) =>
{
    var span = SentrySdk.StartSpan("CustomMiddlewareSpan", "middleware");
    try
    {
        var log = context.RequestServices.GetRequiredService<ILoggerFactory>()
            .CreateLogger<Program>();

        log.LogInformation("Just chilling for a bit...");
        await Task.Delay(TimeSpan.FromMilliseconds(50)); // Simulate some work
        span.Finish();
    }
    catch (Exception e)
    {
        span.Finish(e);
        throw;
    }
    await next();
});

app.Run();

// Add this class at the end of the file
public class SampleBackgroundService : BackgroundService
{
    private readonly ILogger<SampleBackgroundService> _logger;

    public SampleBackgroundService(ILogger<SampleBackgroundService> logger)
    {
        _logger = logger;
    }

    protected override async Task ExecuteAsync(CancellationToken stoppingToken)
    {
        while (!stoppingToken.IsCancellationRequested)
        {
            try
            {
                await Task.Delay(5000, stoppingToken);
                throw new InvalidOperationException("Background service test exception for Sentry.");
            }
            catch (Exception ex)
            {
                _logger.LogError(ex, "Exception in background service");
                SentrySdk.CaptureException(ex);
            }
        }
    }
}

jamescrosswell avatar Jul 01 '25 03:07 jamescrosswell