Hangfire job failures result in SentryCheckInIdKey error

[vrelk-net]

Package

Other

.NET Flavor

.NET

.NET Version

8.0.8

OS

Windows

SDK Version

4.11.0

Self-Hosted Sentry Version

24.9.0

Steps to Reproduce

1. Create a hangfire background job. In my case it's hourly with 5 retries.
2. Make said job fail repeatedly

Relevant function:

    [Hangfire.Queue("redacted")]
    [Hangfire.AutomaticRetry(Attempts = 5)]
    [Hangfire.DisableConcurrentExecution(timeoutInSeconds: 2 * 60)]
    [SentryMonitorSlug("redacted-auth-refresh")]
    public async Task DoAuthRefreshAsync(CancellationToken token)
    {
        var result = await this.apiClient.RefreshAuthAsync();

        if (result.IsError)
        {
            throw new Exception("TASK - Redacted: AuthRefresh: Failed to refresh auth. " + result.FirstError.Code + " - " + result.FirstError.Description);
        }
        else
        {
            Log.Information("TASK - Redacted: AuthRefresh: Redacted auth token checked and refreshed if needed.");
        }
    }

Expected Result

A hangfire issue for the actual exception that was thrown. (Which actually happens the first time an exception occurs)

Actual Result

The screenshot of the latest report in Sentry.

Since I'm unable to share the actual sentry URL, This is the JSON for one of the many instances of this error.
https://gist.github.com/vrelk-net/e6175fda5ef90839e1a149769b2db14a

When the first event comes in, the report is actually helpful. After it continues, the original issue gets buried.
These breadcrumbs come from one of the first few events to come in.

[jamescrosswell]

Hi @vrelk-net ,

I'll try to summarise what I've understood:

• You have a hangfire job that fails repeatedly.
• You get a useful error report when the exception is first reported
• The error is then reported again, and again... ad infinitum
  • These subsequent errors become less and less useful because the breadcrumbs are just filled with this one exception

Is that correct? And is it all the breadcrumbs that are not useful (everything else about the subsequent error reports, like the stack trace etc. are still OK)?

The behaviour to create a breadcrumb for any exceptions is something that was added in version 4.11.0.

The thinking was that if you had a block of code where some stuff happened, an exception occurred, then some other stuff happened and another exception occurred, the first exception might be relevant to (and have even caused) the second exception... and I think often/usually, that would be true.

In this particular case, since it's a recurring exception, I can see all of those breadcrumbs aren't that useful... Potentially before adding a breadcrumb for an exception, we could check to see if a breadcrumb for the exact same exception already exists on the scope and, if so, skip it. I think we'd need to use the stack trace as a fingerprint to identify the exception - so as long as the stack trace was always the same, this might be possible.

[vrelk-net]

That is correct, though it seems there is more to it as I will explain.

Just for a little context, the task is to keep an oAuth session active as you have to "prime" it the first time using your login credentials, which is kinda dumb if you need to use it for automation.
I just did that for this task, leaving it with a good, active session. I then ran another scheduled task that would use this session, but has also been failing because the session was bad (resulting in failed jobs). This task (not a retry), which should have run without a problem, failed due to the issue this bug report is about.

So it appears that this is also preventing otherwise good jobs from running once the issue becomes present. Without knowing how it works on the Sentry side, it seems like it can only have 1 check-in active at a time, but due to the failure, it never ends the previous one, preventing new ones from starting. Bit of irony that the crash logger is the one causing the error.

[jamescrosswell]

This task (not a retry), which should have run without a problem, failed due to the issue this bug report is about.

Ah, I see... so the real issue is the An item with the same key has already been added. Key: SentryCheckInIdKey exception being thrown by the dictionary. The stack trace says the exception is thrown in SentryServerFilter on line 25 but I think that's erroneous. The only place SentryCheckInIdKey is added to a dictionary is on line 37:

sentry-dotnet/src/Sentry.Hangfire/SentryServerFilter.cs

Lines 36 to 37 in 8766994

	var checkInId = _hub.CaptureCheckIn(monitorSlug, CheckInStatus.InProgress);
	context.Items.Add(SentryCheckInIdKey, checkInId);

I've made a PR with what I think is a sensible fix for this:
#3631

I'll create a separate issue to track the creation of a bunch of not very useful breadcrumbs for repeating jobs. We may be able to come up with a workaround for that as well.