From 2b3c0c95c96ba4a9a87ebce7c7e9fc1771c5f147 Mon Sep 17 00:00:00 2001 From: Christian Date: Mon, 6 Oct 2025 09:58:23 -0700 Subject: [PATCH 1/4] Checking if the edit has already occurred --- .../Api/EditFailedMessagesController.cs | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/ServiceControl/MessageFailures/Api/EditFailedMessagesController.cs b/src/ServiceControl/MessageFailures/Api/EditFailedMessagesController.cs index 9c0413cbac..be9cdcc044 100644 --- a/src/ServiceControl/MessageFailures/Api/EditFailedMessagesController.cs +++ b/src/ServiceControl/MessageFailures/Api/EditFailedMessagesController.cs @@ -35,6 +35,17 @@ public async Task Edit(string failedMessageId, [FromBody] EditMes return NotFound(); } + //HINT: This validation is the first one because we want to minimize the chance of two users concurrently execute an edit-retry. + var editManager = await store.CreateEditFailedMessageManager(); + var editId = await editManager.GetCurrentEditingMessageId(failedMessageId); + if (editId != null) + { + logger.LogWarning("Cannot edit message {FailedMessageId} because it has already been edited", failedMessageId); + // We return HTTP 200 even though the edit is being ignored. This is to keep the compatibility with older versions of ServicePulse. + // Newer ServicePulse versions (starting with x.x) will handle the response's payload accordingly. + return Ok(new { EditIgnored = true }); + } + var failedMessage = await store.ErrorBy(failedMessageId); if (failedMessage == null) @@ -45,8 +56,8 @@ public async Task Edit(string failedMessageId, [FromBody] EditMes //WARN /* - * failedMessage.ProcessingAttempts.Last() return the lat retry attempt. - * In theory between teh time someone edits a failed message and retry it someone else + * failedMessage.ProcessingAttempts.Last() return the last retry attempt. + * In theory between the time someone edits a failed message and retry it someone else * could have retried the same message without editing. If this is the case "Last()" is * not anymore the same message. * Instead of using Last() it's probably better to select the processing attempt by looking for @@ -74,7 +85,7 @@ await session.SendLocal(new EditAndSend NewHeaders = edit.MessageHeaders }); - return Accepted(); + return Accepted(new { EditIgnored = false }); } From 6a769acbdf669c991b2a9e993a1d5ebbf7110e10 Mon Sep 17 00:00:00 2001 From: Christian Date: Wed, 15 Oct 2025 08:27:11 -0700 Subject: [PATCH 2/4] Replacing an anonymous object with a typed response --- .../Api/EditFailedMessagesController.cs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/ServiceControl/MessageFailures/Api/EditFailedMessagesController.cs b/src/ServiceControl/MessageFailures/Api/EditFailedMessagesController.cs index be9cdcc044..b73814d8a2 100644 --- a/src/ServiceControl/MessageFailures/Api/EditFailedMessagesController.cs +++ b/src/ServiceControl/MessageFailures/Api/EditFailedMessagesController.cs @@ -27,7 +27,7 @@ public class EditFailedMessagesController( [Route("edit/{failedMessageId:required:minlength(1)}")] [HttpPost] - public async Task Edit(string failedMessageId, [FromBody] EditMessageModel edit) + public async Task> Edit(string failedMessageId, [FromBody] EditMessageModel edit) { if (!settings.AllowMessageEditing) { @@ -43,7 +43,7 @@ public async Task Edit(string failedMessageId, [FromBody] EditMes logger.LogWarning("Cannot edit message {FailedMessageId} because it has already been edited", failedMessageId); // We return HTTP 200 even though the edit is being ignored. This is to keep the compatibility with older versions of ServicePulse. // Newer ServicePulse versions (starting with x.x) will handle the response's payload accordingly. - return Ok(new { EditIgnored = true }); + return Ok(new EditRetryResponse { EditIgnored = true }); } var failedMessage = await store.ErrorBy(failedMessageId); @@ -85,7 +85,7 @@ await session.SendLocal(new EditAndSend NewHeaders = edit.MessageHeaders }); - return Accepted(new { EditIgnored = false }); + return Accepted(new EditRetryResponse { EditIgnored = false }); } @@ -148,4 +148,9 @@ public class EditMessageModel public Dictionary MessageHeaders { get; set; } } + + public class EditRetryResponse + { + public bool EditIgnored { get; set; } + } } \ No newline at end of file From b5019e61b50af47b324b09d9fefb03ad96d628b6 Mon Sep 17 00:00:00 2001 From: Christian Date: Wed, 15 Oct 2025 10:39:19 -0700 Subject: [PATCH 3/4] Apply suggestion from @cquirosj --- .../MessageFailures/Api/EditFailedMessagesController.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/ServiceControl/MessageFailures/Api/EditFailedMessagesController.cs b/src/ServiceControl/MessageFailures/Api/EditFailedMessagesController.cs index b73814d8a2..40a163c135 100644 --- a/src/ServiceControl/MessageFailures/Api/EditFailedMessagesController.cs +++ b/src/ServiceControl/MessageFailures/Api/EditFailedMessagesController.cs @@ -41,9 +41,8 @@ public async Task> Edit(string failedMessageId, if (editId != null) { logger.LogWarning("Cannot edit message {FailedMessageId} because it has already been edited", failedMessageId); - // We return HTTP 200 even though the edit is being ignored. This is to keep the compatibility with older versions of ServicePulse. - // Newer ServicePulse versions (starting with x.x) will handle the response's payload accordingly. - return Ok(new EditRetryResponse { EditIgnored = true }); + // We return HTTP 200 even though the edit is being ignored. This is to keep the compatibility with older versions of ServicePulse that don't handle the payload. + return Ok(new { EditIgnored = true }); } var failedMessage = await store.ErrorBy(failedMessageId); From 7d204bb8d6e31fbbfebd9e18239b6485affa82b0 Mon Sep 17 00:00:00 2001 From: Christian Date: Wed, 15 Oct 2025 11:00:16 -0700 Subject: [PATCH 4/4] Fixed return type --- .../MessageFailures/Api/EditFailedMessagesController.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ServiceControl/MessageFailures/Api/EditFailedMessagesController.cs b/src/ServiceControl/MessageFailures/Api/EditFailedMessagesController.cs index 40a163c135..533f2c8792 100644 --- a/src/ServiceControl/MessageFailures/Api/EditFailedMessagesController.cs +++ b/src/ServiceControl/MessageFailures/Api/EditFailedMessagesController.cs @@ -42,7 +42,7 @@ public async Task> Edit(string failedMessageId, { logger.LogWarning("Cannot edit message {FailedMessageId} because it has already been edited", failedMessageId); // We return HTTP 200 even though the edit is being ignored. This is to keep the compatibility with older versions of ServicePulse that don't handle the payload. - return Ok(new { EditIgnored = true }); + return Ok(new EditRetryResponse { EditIgnored = true }); } var failedMessage = await store.ErrorBy(failedMessageId);