aws / aws/aws-dotnet-messaging
No backoff/retry for delete
- Dominant language
- C#
- Stars
- 143
- Forks
- 27
- Avg merge
- 1d 18h
- Merged PRs (30d)
- 4
Description
### Describe the bug
On a successful handler invocation, the `delete` operation can still fail due to throttling (or other HTTP error). This results in an otherwise successful message being reprocessed/sent to the DLQ.
```
Failed to delete message 00000000-0000-0000-0000-000000000000 from queue https://sqs.ap-southeast-2.amazonaws.com/XXXX/XXX.fifo: Request is throttled
```
### Regression Issue
- [ ] Select this option if this issue appears to be a regression.
### Expected Behavior
Use the `IBackoffHandler` in `SQSMessagePoller.DeleteMessagesAsync` to retry delete on a non-fatal exception.
### Current Behavior
When API requests are being throttled/other HTTP exception, the request fails and the message is sent back to the queue to be processed again even though the handler has successfully completed invocation.
### Reproduction Steps
Exceed the throttle cap on a SQS queue.
'delete' will start to fail, resulting in the same message being processed multiple times and eventually may be redirected to the DLQ.
```
Failed to delete message 00000000-0000-0000-0000-000000000000 from queue https://sqs.ap-southeast-2.amazonaws.com/XXXX/XXX.fifo: Request is throttled
```
### Possible Solution
Wrap `DeleteMessageBatchAsync()` in a retry block. Optionally handle exceptions that are not fatal to the poller, but are fatal to the message (eg. already deleted, etc0.
```cs
internal class SQSMessagePoller : IMessagePoller, ISQSMessageCommunication
{
public async Task DeleteMessagesAsync(IEnumerable messages, CancellationToken token = default)
{
...
// From:
var response = await _sqsClient.DeleteMessageBatchAsync(request, token);
// To:
var response =
await _backoffHandler.BackoffAsync(async () =>
{
DeleteMessageBatchResponse deleteMessageBatchResponse;
try
{
deleteMessageBatchResponse = await _sqsClient.DeleteMessageBatchAsync(request, token);
}
catch (TaskCanceledException)
{
_logger.LogTrace("Cancellation requested while deleting messages, probably due to shutdown. Returning empty response");
return new DeleteMessageBatchResponse();
}
return deleteMessageBatchResponse;
},
// supply retry exception check instead of base configuration so that exceptions fatal to delete will also avoid a retry
// ie. message not found/already deleted should not be retried or stop the SQS poller
ex => _configuration.IsExceptionFatal(ex) || IsDeletedFatalException(ex),
token);
...
}
}
```
### Additional Information/Context
_No response_
### AWS.Messaging (or related) package versions
Issue confirmed in source.
### Targeted .NET Platform
net8.0
### Operating System and version
n/a
Contributor guide
Research direction
Start in SQSMessagePoller.DeleteMessagesAsync and inspect the existing IBackoffHandler usage, DeleteMessageBatchAsync call, and fatal-exception configuration. Confirm how cancellation and already-deleted messages are handled, then verify that non-fatal delete failures retry without stopping the poller and that a successful handler invocation is not reprocessed because of throttling.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- aws, csharp
- Domain
- backend, cloud
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 64/100