diff --git a/.autover/changes/0529a49e-6b48-4eea-874b-1a5c2be8b029.json b/.autover/changes/0529a49e-6b48-4eea-874b-1a5c2be8b029.json new file mode 100644 index 00000000..f85b8f10 --- /dev/null +++ b/.autover/changes/0529a49e-6b48-4eea-874b-1a5c2be8b029.json @@ -0,0 +1,18 @@ +{ + "Projects": [ + { + "Name": "AWS.Messaging", + "Type": "Patch", + "ChangelogMessages": [ + "Fix issue with fifo when a message is failed to process the later messages are not retried" + ] + }, + { + "Name": "AWS.Messaging.Lambda", + "Type": "Patch", + "ChangelogMessages": [ + "Update AWS.Messaging dependency" + ] + } + ] +} \ No newline at end of file diff --git a/.autover/changes/cb641346-f9f6-414d-be09-90ea0b8366a7.json b/.autover/changes/cb641346-f9f6-414d-be09-90ea0b8366a7.json new file mode 100644 index 00000000..5932362e --- /dev/null +++ b/.autover/changes/cb641346-f9f6-414d-be09-90ea0b8366a7.json @@ -0,0 +1,25 @@ +{ + "Projects": [ + { + "Name": "AWS.Messaging", + "Type": "Patch", + "ChangelogMessages": [ + "Update User-Agent string" + ] + }, + { + "Name": "AWS.Messaging.Lambda", + "Type": "Patch", + "ChangelogMessages": [ + "Update project dependencies" + ] + }, + { + "Name": "AWS.Messaging.Telemetry.OpenTelemetry", + "Type": "Patch", + "ChangelogMessages": [ + "Update project dependencies" + ] + } + ] + } \ No newline at end of file diff --git a/.github/ISSUE_TEMPLATE/bug-report.yml b/.github/ISSUE_TEMPLATE/bug-report.yml index 63a0240b..d307d758 100644 --- a/.github/ISSUE_TEMPLATE/bug-report.yml +++ b/.github/ISSUE_TEMPLATE/bug-report.yml @@ -12,6 +12,14 @@ body: description: What is the problem? A clear and concise description of the bug. validations: required: true + - type: checkboxes + id: regression + attributes: + label: Regression Issue + description: What is a regression? If it worked in a previous version but doesn't in the latest version, it's considered a regression. In this case, please provide specific version number in the report. + options: + - label: Select this option if this issue appears to be a regression. + required: false - type: textarea id: expected attributes: diff --git a/.github/workflows/BuildandTest.yml b/.github/workflows/BuildandTest.yml index bdb3700a..9e6c3de7 100644 --- a/.github/workflows/BuildandTest.yml +++ b/.github/workflows/BuildandTest.yml @@ -15,6 +15,10 @@ jobs: uses: actions/setup-dotnet@4d6c8fcf3c8f7a60068d26b594648e99df24cee3 # pinning V4 with: dotnet-version: 6.0.x + - name: Setup .NET Core 8.0 + uses: actions/setup-dotnet@4d6c8fcf3c8f7a60068d26b594648e99df24cee3 # pinning V4 + with: + dotnet-version: 8.0.x - name: Restore dependencies run: dotnet restore - name: Build diff --git a/.github/workflows/issue-regression-labeler.yml b/.github/workflows/issue-regression-labeler.yml new file mode 100644 index 00000000..bd000719 --- /dev/null +++ b/.github/workflows/issue-regression-labeler.yml @@ -0,0 +1,32 @@ +# Apply potential regression label on issues +name: issue-regression-label +on: + issues: + types: [opened, edited] +jobs: + add-regression-label: + runs-on: ubuntu-latest + permissions: + issues: write + steps: + - name: Fetch template body + id: check_regression + uses: actions/github-script@v7 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + TEMPLATE_BODY: ${{ github.event.issue.body }} + with: + script: | + const regressionPattern = /\[x\] Select this option if this issue appears to be a regression\./i; + const template = `${process.env.TEMPLATE_BODY}` + const match = regressionPattern.test(template); + core.setOutput('is_regression', match); + - name: Manage regression label + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + if [ "${{ steps.check_regression.outputs.is_regression }}" == "true" ]; then + gh issue edit ${{ github.event.issue.number }} --add-label "potential-regression" -R ${{ github.repository }} + else + gh issue edit ${{ github.event.issue.number }} --remove-label "potential-regression" -R ${{ github.repository }} + fi diff --git a/src/AWS.Messaging/Services/DefaultMessageManager.cs b/src/AWS.Messaging/Services/DefaultMessageManager.cs index bb15127b..a4c106a9 100644 --- a/src/AWS.Messaging/Services/DefaultMessageManager.cs +++ b/src/AWS.Messaging/Services/DefaultMessageManager.cs @@ -133,12 +133,19 @@ public async Task ProcessMessageGroupAsync(List message var remaining = messageGroup.Count; // Sequentially process each message within the group - foreach (var message in messageGroup) + for(var i = 0; i < messageGroup.Count; i++) { + var message = messageGroup[i]; + var isSuccessful = await InvokeHandler(message.Envelope, message.Mapping, token); if (!isSuccessful) { - // If the handler invocation fails for any message, skip processing subsequent messages in the group. + // If the handler invocation fails for any message, report failure for the rest in the group so they are reported back to Lambda + // as not failure that should be retried. Otherwise Lambda will think the messages were successfully processed and delete the messages. + for (var unprocessedIndex = i + 1; unprocessedIndex < messageGroup.Count; unprocessedIndex++) + { + await _sqsMessageCommunication.ReportMessageFailureAsync(messageGroup[unprocessedIndex].Envelope, token); + } _logger.LogError("Handler invocation failed for a message belonging to message group '{GroupdId}' having message ID '{MessageID}'. Skipping processing of {Remaining} messages from the same group.", groupId, message.Envelope.Id, remaining); break; } diff --git a/test/AWS.Messaging.UnitTests/LambdaTests.cs b/test/AWS.Messaging.UnitTests/LambdaTests.cs index 9dfed8c7..dc5fb913 100644 --- a/test/AWS.Messaging.UnitTests/LambdaTests.cs +++ b/test/AWS.Messaging.UnitTests/LambdaTests.cs @@ -105,8 +105,13 @@ public async Task MessageHandlerReturnsFailedStatusBatchResponse() Id = "failed-message-1", ReturnFailedStatus = true }; + var secondMessage = new SimulatedMessage + { + Id = "second-message-1", + ReturnFailedStatus = false + }; - var tuple = await ExecuteWithBatchResponse(new SimulatedMessage[] { failedMessage }); + var tuple = await ExecuteWithBatchResponse(new SimulatedMessage[] { failedMessage, secondMessage }); _mockSqs!.Verify( expression: x => x.ChangeMessageVisibilityAsync(It.IsAny(), It.IsAny()), times: Times.Never); @@ -114,6 +119,30 @@ public async Task MessageHandlerReturnsFailedStatusBatchResponse() Assert.Equal("1", tuple.batchResponse.BatchItemFailures[0].ItemIdentifier); } + [Fact] + public async Task MessageHandlerReturnsFailedStatusBatchResponseFifo() + { + var failedMessage = new SimulatedMessage + { + Id = "failed-message-1", + ReturnFailedStatus = true, + MessageGroupId = "group1" + }; + var secondMessage = new SimulatedMessage + { + Id = "second-message-1", + ReturnFailedStatus = false, + MessageGroupId = "group1" + }; + + var tuple = await ExecuteWithBatchResponse(new SimulatedMessage[] { failedMessage, secondMessage }, isFifoQueue: true); + _mockSqs!.Verify( + expression: x => x.ChangeMessageVisibilityAsync(It.IsAny(), It.IsAny()), + times: Times.Never); + Assert.Equal(2, tuple.batchResponse.BatchItemFailures.Count); + Assert.Equal("1", tuple.batchResponse.BatchItemFailures[0].ItemIdentifier); + } + [Fact] public async Task MessageHandlerResetsVisibilityWhenFailedStatusBatchResponse() { @@ -304,13 +333,14 @@ private async Task Execute(SimulatedMessage[] messages, int maxNumberOfC SimulatedMessage[] messages, int maxNumberOfConcurrentMessages = 1, bool deleteMessagesWhenCompleted = false, - int? visibilityTimeoutForBatchFailures = default) + int? visibilityTimeoutForBatchFailures = default, + bool isFifoQueue = false) { var provider = CreateServiceProvider( maxNumberOfConcurrentMessages: maxNumberOfConcurrentMessages, deleteMessagesWhenCompleted: deleteMessagesWhenCompleted, - visibilityTimeoutForBatchFailures: visibilityTimeoutForBatchFailures); - var sqsEvent = await CreateLambdaEvent(provider, messages); + visibilityTimeoutForBatchFailures: visibilityTimeoutForBatchFailures, isFifoQueue: isFifoQueue); + var sqsEvent = await CreateLambdaEvent(provider, messages, isFifoQueue: isFifoQueue); var logger = new TestLambdaLogger(); var context = new TestLambdaContext()