aws / aws/aws-dotnet-messaging

No backoff/retry for delete

Open
#319 6 comments 0 reactions 0 assignees View on GitHub
bug p1 queued
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.