diff --git a/CHANGELOG.md b/CHANGELOG.md index 715e246151..3943097551 100644 --- a/CHANGELOG.md +++ b/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)) diff --git a/src/Sentry.Hangfire/SentryServerFilter.cs b/src/Sentry.Hangfire/SentryServerFilter.cs index c94c3fa07e..c4cb4f3551 100644 --- a/src/Sentry.Hangfire/SentryServerFilter.cs +++ b/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) diff --git a/test/Sentry.Hangfire.Tests/ServerFilterTests.cs b/test/Sentry.Hangfire.Tests/ServerFilterTests.cs new file mode 100644 index 0000000000..0ac1a1f8a5 --- /dev/null +++ b/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(); + storageConnection.GetJobParameter(jobId, SentryServerFilter.SentryMonitorSlugKey).Returns( + SerializationHelper.Serialize(monitorSlug) + ); + + var backgroundJob = new BackgroundJob(jobId, null, DateTime.Now); + var cancellationToken = Substitute.For(); + var performContext = new PerformContext( + null, + storageConnection, + backgroundJob, + cancellationToken + ); + var performingContext = new PerformingContext(performContext); + + var hub = Substitute.For(); + hub.CaptureCheckIn(monitorSlug, CheckInStatus.InProgress).Returns(SentryId.Create()); + + var logger = Substitute.For(); + 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); + } +}