aws / aws-sdk

Landing page for the AWS SDKs on GitHub
https://aws.amazon.com/tools/
Other
68 stars 12 forks source link

SQS sendMessageBatch is missing `Failed` array in the response (v2.1500) #657

Closed gersmann closed 7 months ago

gersmann commented 7 months ago

Describe the bug

SendMessageBatch does not contain Failed: [] anymore in case of successful sends:

sqs.sendMessageBatch(params, function (err, result) {
    if (err) return cb(err);

    failedMessages = failedMessages.concat(result.Failed.map(entryId));

    if (endIndex < messages.length) {
      return producer._sendBatch(failedMessages, messages, endIndex, cb);
    }

    cb(createError(failedMessages));

Gives us:

TypeError: Cannot read properties of undefined (reading 'map')

Expected Behavior

According to the SendMessageBatch docs and the AWS SDK documentation, Failed is a required property:

SendMessageBatch API Docs

Example:

HTTP/1.1 200 OK
x-amzn-RequestId: <requestId>
Content-Length: <PayloadSizeBytes>
Date: <Date>
Content-Type: application/x-amz-json-1.0
{
   "Failed": [],
    "Successful": [
        {
            "Id": "test_msg_001",
            "MD5OfMessageBody": "0e024d309850c78cba5eabbeff7cae71",
            "MessageId": "f4eb349f-cd33-4bc4-bdc2-e557c900d41d"
        },
        {
            "Id": "test_msg_002",
            "MD5OfMessageAttributes": "8ef4d60dbc8efda9f260e1dfd09d29f3",
            "MD5OfMessageBody": "27118326006d3829667a400ad23d5d98",
            "MessageId": "1dcfcd50-5a67-45ae-ae4c-1c152b5effb9"
        }
    ]
}

Current Behavior

Failed is missing from the response object (in eu-central-1 as of 7am UTC)

Reproduction Steps

Perform sendMessageBatch using the aws-js-sdk in eu-central-1.

Possible Solution

Apparently this is handled at the SDK level now in for instance the Ruby SDK: https://github.com/aws/aws-sdk-ruby/issues/2957

Additional Information/Context

No response

SDK version used

2.1500

Environment details (OS name and version, etc.)

Ubuntu 20.04

gersmann commented 7 months ago

To reproduce:

const sqs = new AWS.SQS()
sqs.sendMessageBatch(
    {
        QueueUrl: 'https://sqs.eu-central-1.amazonaws.com/1234567890123/some-queue',
        Entries: [{ Id: '123123', MessageBody: JSON.stringify({ foo: 'bar' }) }],
    },
    (err, data) => {
        console.log(data)
    }
)

produces:

{
  Successful: [
    {
      Id: '123123',
      MessageId: 'b0f22153-f08c-4c0a-868e-796f7618bac4',
      MD5OfMessageBody: '9bb58f26192e4ba00f01e2e7b136bbd8'
    }
  ]
}

But the clients sqs.d.ts has those types:

  export interface SendMessageBatchResult {
    /**
     * A list of  SendMessageBatchResultEntry  items.
     */
    Successful: SendMessageBatchResultEntryList;
    /**
     * A list of  BatchResultErrorEntry  items with error details about each message that can't be enqueued.
     */
    Failed: BatchResultErrorEntryList;
  }

  export type BatchResultErrorEntryList = BatchResultErrorEntry[];

  export interface BatchResultErrorEntry {
    /**
     * The Id of an entry in a batch request.
     */
    Id: String;
    /**
     * Specifies whether the error happened due to the caller of the batch API action.
     */
    SenderFault: Boolean;
    /**
     * An error code representing why the action failed on this entry.
     */
    Code: String;
    /**
     * A message explaining why the action failed on this entry.
     */
    Message?: String;
  }

in which Failed is not optional.

ambiguo commented 7 months ago

Same error using sqs-producer:1.6.3

kenkku commented 7 months ago

This was probably introduced in 2.1491.0.

feature: SQS: This release enables customers to call SQS using AWS JSON-1.0 protocol.

As suggested in aws/aws-sdk-js#4523, we downgraded to 2.1490, which resolved this problem.

gabrsar commented 7 months ago

Having same issue here. with sqs-producer:2.2.0.

Noezor commented 7 months ago

Somewhat incorrectly redirected AWS support to https://github.com/aws/aws-sdk-js/issues/4523 where there was an answer at https://github.com/aws/aws-sdk-js/issues/4523#issuecomment-1832771957 . I am raising the sendMessageBatch issue there.

RanVaknin commented 7 months ago

Hello everyone on the thread,

Recently the SQS service moved from XML wire protocol to a more modern JSON RPC protocol. Since the SQS service changed the protocol, the SDK's SQS Client now sends the requests in JSON format: https://docs.aws.amazon.com/AWSSimpleQueueService/latest/SQSDeveloperGuide/sqs-json-faqs.html

I ran sendMessageBatch and indeed after 2.149.0 I'm not able to see a Failed: [] in the response. At first I thought the SDK simply does not deserialize that field since the value is an empty array, but after inspecting the raw response sent from the SQS server and it seems like the Failed field is not sent at all in the response.

Just to make sure its not SDK specific, I tested it with the Go SDK which has a much more robust wire logger and got the same result:

package main

import (
    "github.com/aws/aws-sdk-go/aws"
    "github.com/aws/aws-sdk-go/aws/session"
    "github.com/aws/aws-sdk-go/service/sqs"
)

func main() {

    sess := session.Must(session.NewSession(&aws.Config{
        Region:   aws.String("us-east-1"),
        LogLevel: aws.LogLevel(aws.LogDebugWithHTTPBody),
    }))

    client := sqs.New(sess)

    _, err := client.SendMessageBatch(&sqs.SendMessageBatchInput{
        QueueUrl: aws.String("https://sqs.us-east-1.amazonaws.com/REDACTED/test-queue"),
        Entries: []*sqs.SendMessageBatchRequestEntry{
            {
                Id:          aws.String("123123"),
                MessageBody: aws.String("{ foo: 'bar' }"),
            },
        },
    })
    if err != nil {
        panic(err)
    }

}

And the raw response does not contain the Failed field:

-----------------------------------------------------
2023/11/29 17:51:00 DEBUG: Response sqs/SendMessageBatch Details:
---[ RESPONSE ]--------------------------------------
HTTP/1.1 200 OK
Content-Length: 137
Connection: keep-alive
Content-Type: application/x-amz-json-1.0
Date: Thu, 30 Nov 2023 01:51:00 GMT
X-Amzn-Requestid: REDACTED

-----------------------------------------------------
2023/11/29 17:51:00 {"Successful":[{"Id":"123123","MD5OfMessageBody":"0c7fa0326d501efb3db4a87bf0740f48","MessageId":"2ad45bb8-2fcf-4a52-85a2-78415e346376"}]}

My assumption is that SQS service supports both the new JSON rpc, and the XML(sqs Query) protocol for backwards compatibility reasons (thats why older versions still work and do not break after the move). However their model only specifies JSON rpc as the only wire protocol.

So the conclusion here is that SQS is not sending the same response in the JSON rpc protocol as the one it does through the XML based protocol. This is not something that the SDK can change as this data comes from the server itself.

Since @Noezor has already raised an internal ticket with AWS, I will re-route it to SQS for further investigation.

Thanks, Ran~

gersmann commented 7 months ago

@RanVaknin thank youf or the follow up. What's interesting in this case, this has been working fine on the updated versions of the SDK, but stopped working around 23rd 6pm UTC (for us). Others have made the same observation, see : https://github.com/ssut/nestjs-sqs/issues/72 + the TS types / AWS docs also currently contain 'Failed'.

@Noezor thanks for raising a support case. Can you keep us posted if you get a response or post the Case ID so that we can refer to it?

Noezor commented 7 months ago

+1 on the issue starting to appear at this time for us too. This created quite a lot of headaches to find the root cause.

CASE 14379988591 is where we filled it. I'll let support know that the issue has been rerouted to the SQS team, will also keep you posted if I get an answer quicker on support than here.

RanVaknin commented 7 months ago

Hi there,

Thank you all for your patience. After some more investigation, we have found out that the behavior of adding Failed: [] explicitly when there are no failed messages in the response was a behavior unique to JS SDK v2.

I have created a fix for v2 to continue (mis)behaving the way it did before the migration to JSON. But just a heads up, this behavior is not going to be the same in v3. In v3 we rely on the service to send back data, and the SDK does not model data that is not returned.

Thanks again for your cooperation. All the best, Ran~

Noezor commented 7 months ago

Thanks a lot Ran! Just tested and it works. Super speedy fix considering you had to ping an external team.

By the way, I used latest version (2.1512), but did not manage to find on the MR from which version onwards this was fixed. Do you know where to find on which release the MR was included?

RanVaknin commented 7 months ago

Hi @Noezor ,

Great to hear that this now is working. I ended up patching it locally, thats why it was fast 😄. This was fixed in 2.1511.0 https://github.com/aws/aws-sdk-js/blob/master/CHANGELOG.md#215110

Im going to close this issue. If something else comes up please open a separate issue in the v2 repo.

Thanks again, Ran~

github-actions[bot] commented 7 months ago

This issue is now closed.

Comments on closed issues are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one. If you wish to keep having a conversation with other community members under this issue feel free to do so.