From 67aa23edecca0fffce4708086b9188332e3d59dc Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Thu, 26 Sep 2024 14:59:27 +1200 Subject: [PATCH 1/2] Fixed problem in Sentry.Hangfire when jobs with automatic retry failed --- src/Sentry.Hangfire/SentryServerFilter.cs | 6 ++- .../ServerFilterTests.cs | 52 +++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 test/Sentry.Hangfire.Tests/ServerFilterTests.cs 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); + } +} From bcdf18134d5e5a116f458198cd7679fdf701b338 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Thu, 26 Sep 2024 15:04:59 +1200 Subject: [PATCH 2/2] Update CHANGELOG.md --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 13630824ea..914c959097 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)) + ### Dependencies - Bump Native SDK from v0.7.9 to v0.7.10 ([#3623](https://github.com/getsentry/sentry-dotnet/pull/3623))