Fixed duplicate key exception for Hangfire jobs with AutomaticRetry (#3631)

Author: jamescrosswell

CHANGELOG.md

@@ -8,6 +8,10 @@
   Sentry will reject all metrics sent after October 7, 2024.
   Learn more: https://sentry.zendesk.com/hc/en-us/articles/26369339769883-Upcoming-API-Changes-to-Metrics  ([#3619](https://github.com/getsentry/sentry-dotnet/pull/3619))
 
+## Fixes
+
+- Fixed duplicate key exception for Hangfire jobs with AutomaticRetry ([#3631](https://github.com/getsentry/sentry-dotnet/pull/3631))
+
 ### Features
 
 - Added a flag to options `DisableFileWrite` to allow users to opt-out of all file writing operations. Note that toggling this will affect features such as offline caching and auto-session tracking and release health as these rely on some file persistency  ([#3614](https://github.com/getsentry/sentry-dotnet/pull/3614))

src/Sentry.Hangfire/SentryServerFilter.cs

@@ -34,7 +34,11 @@ public void OnPerforming(PerformingContext context)
         }
 
         var checkInId = _hub.CaptureCheckIn(monitorSlug, CheckInStatus.InProgress);
-        context.Items.Add(SentryCheckInIdKey, checkInId);
+
+        // Note that we may be overwriting context.Items[SentryCheckInIdKey] here, which is intentional. If that happens
+        // then implicitly OnPerforming was called previously with the same context, but we never made it to OnPerformed
+        // This might happen if a Hangfire job failed at least once, with automatic retries configured.
+        context.Items[SentryCheckInIdKey] = checkInId;
     }
 
     public void OnPerformed(PerformedContext context)

test/Sentry.Hangfire.Tests/ServerFilterTests.cs

@@ -0,0 +1,52 @@
+using Hangfire;
+using Hangfire.Common;
+using Hangfire.Server;
+using Hangfire.Storage;
+
+namespace Sentry.Hangfire.Tests;
+
+public class ServerFilterTests
+{
+    [Fact]
+    public void OnPerforming_IsReentrant()
+    {
+        // Arrange
+        const string jobId = "test-id";
+        const string monitorSlug = "test-monitor-slug";
+
+        var storageConnection = Substitute.For<IStorageConnection>();
+        storageConnection.GetJobParameter(jobId, SentryServerFilter.SentryMonitorSlugKey).Returns(
+            SerializationHelper.Serialize(monitorSlug)
+            );
+
+        var backgroundJob = new BackgroundJob(jobId, null, DateTime.Now);
+        var cancellationToken = Substitute.For<IJobCancellationToken>();
+        var performContext = new PerformContext(
+            null,
+            storageConnection,
+            backgroundJob,
+            cancellationToken
+            );
+        var performingContext = new PerformingContext(performContext);
+
+        var hub = Substitute.For<IHub>();
+        hub.CaptureCheckIn(monitorSlug, CheckInStatus.InProgress).Returns(SentryId.Create());
+
+        var logger = Substitute.For<IDiagnosticLogger>();
+        var filter = new SentryServerFilter(hub, logger);
+
+        // Act
+        filter.OnPerforming(performingContext);
+
+        // Assert
+        performContext.Items.ContainsKey(SentryServerFilter.SentryCheckInIdKey).Should().BeTrue();
+        var firstKey = performingContext.Items[SentryServerFilter.SentryCheckInIdKey];
+
+        // Act
+        filter.OnPerforming(performingContext);
+
+        // Assert
+        performContext.Items.ContainsKey(SentryServerFilter.SentryCheckInIdKey).Should().BeTrue();
+        performingContext.Items[SentryServerFilter.SentryCheckInIdKey].Should().NotBeSameAs(firstKey);
+    }
+}