Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Version 2.1491.0 introduces breaking SQS changes #4523

Closed
award1230 opened this issue Nov 10, 2023 · 10 comments
Closed

Version 2.1491.0 introduces breaking SQS changes #4523

award1230 opened this issue Nov 10, 2023 · 10 comments
Assignees
Labels
bug This issue is a bug. p3 This is a minor priority issue

Comments

@award1230
Copy link

award1230 commented Nov 10, 2023

Describe the bug

After updating to version 2.1491.0 our SQS queue creation code throws an error.

Expected Behavior

The queue should still be created. Minor version updates should not introduce breaking changes.

Current Behavior

This error is thrown after updating

Error Creating Queue [UnknownError]: Bad Request
    at Request.extractError (/build/node_modules/aws-sdk/lib/protocol/json.js:80:27)
    at Request.callListeners (/build/node_modules/aws-sdk/lib/sequential_executor.js:106:20)
    at Request.emit (/build/node_modules/aws-sdk/lib/sequential_executor.js:78:10)
    at Request.emit (/build/node_modules/aws-sdk/lib/request.js:686:14)
    at Request.transition (/build/node_modules/aws-sdk/lib/request.js:22:10)
    at AcceptorStateMachine.runTo (/build/node_modules/aws-sdk/lib/state_machine.js:14:12)
    at /build/node_modules/aws-sdk/lib/state_machine.js:26:10
    at Request.<anonymous> (/build/node_modules/aws-sdk/lib/request.js:38:9)
    at Request.<anonymous> (/build/node_modules/aws-sdk/lib/request.js:688:12)
    at Request.callListeners (/build/node_modules/aws-sdk/lib/sequential_executor.js:116:18) {
  code: 'UnknownError',
  statusCode: 400,
  time: 2023-11-10T22:03:18.971Z,
  requestId: undefined,
  retryable: false,
  retryDelay: 57.62829967850942
}

Reproduction Steps

First run npm install [email protected] (working version)

Then run the code:

// init client
const AWS = require('aws-sdk');
const myCredentials = new AWS.Credentials('localkey', 'localsecret');
const sqs = AWS.SQS({
    apiVersion: '2012-11-05',
    credentials: myCredentials,
    region: 'us-west-2',
    endpoint: 'http://localstack:4576'
});

// create queue
const params = {
    QueueName: 'test-queue',
        Attributes: {
            'DelaySeconds': '60',
            'MessageRetentionPeriod': '86400'
    }
};
sqs.createQueue(params, function(err, data) {
    if (err) {
        console.error("Error Creating Queue", err);             
    } else {
        console.log("Created Queue!", JSON.stringify(data, null, 4));
    }
});

This logs

Created Queue! {
    "ResponseMetadata": {
        "RequestId": "00000000-0000-0000-0000-000000000000"
    },
    "QueueUrl": "http://localhost:4576/queue/test-queue"
}

Now run: npm install [email protected] (broken version)
This logs the error

Error Creating Queue [UnknownError]: Bad Request
    at Request.extractError (/build/node_modules/aws-sdk/lib/protocol/json.js:80:27)
    at Request.callListeners (/build/node_modules/aws-sdk/lib/sequential_executor.js:106:20)
    at Request.emit (/build/node_modules/aws-sdk/lib/sequential_executor.js:78:10)
    at Request.emit (/build/node_modules/aws-sdk/lib/request.js:686:14)
    at Request.transition (/build/node_modules/aws-sdk/lib/request.js:22:10)
    at AcceptorStateMachine.runTo (/build/node_modules/aws-sdk/lib/state_machine.js:14:12)
    at /build/node_modules/aws-sdk/lib/state_machine.js:26:10
    at Request.<anonymous> (/build/node_modules/aws-sdk/lib/request.js:38:9)
    at Request.<anonymous> (/build/node_modules/aws-sdk/lib/request.js:688:12)
    at Request.callListeners (/build/node_modules/aws-sdk/lib/sequential_executor.js:116:18) {
  code: 'UnknownError',
  statusCode: 400,
  time: 2023-11-10T22:03:18.971Z,
  requestId: undefined,
  retryable: false,
  retryDelay: 57.62829967850942
}

Nothing changes except updating the aws-sdk version. Note, I am using localstack in this example.

Possible Solution

No response

Additional Information/Context

No response

SDK version used

2.1491.0

Environment details (OS name and version, etc.)

node running on mac/linux

@award1230 award1230 added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Nov 10, 2023
@RanVaknin RanVaknin self-assigned this Nov 10, 2023
@RanVaknin RanVaknin added the p1 This is a high priority issue label Nov 10, 2023
@kennie98
Copy link

same error encountered.

The breaking change was probably introduced here:

    {
        "type": "feature",
        "category": "SQS",
        "description": "This release enables customers to call SQS using AWS JSON-1.0 protocol."
    }

(https://github.com/aws/aws-sdk-js/blob/master/.changes/2.1491.0.json)

@diegoleme
Copy link

Please review this

@Noezor
Copy link

Noezor commented Nov 28, 2023

Hello @RanVaknin , do you have any update on this issue? Will it be fixed(/is it?) in a future release?

@RanVaknin
Copy link
Contributor

RanVaknin commented Nov 29, 2023

Hi everyone,

I apologize for the long response time, and wanted to thank you for your patience.

Recently the SQS service moved from XML wire protocol to a more modern JSON RPC protocol. Since the SQS service changed the protocol, the SQS Client now sends the requests in JSON format. Since you mentioned Localstack, its likely that Localstack did not implement this change on their service side.
https://docs.aws.amazon.com/AWSSimpleQueueService/latest/SQSDeveloperGuide/sqs-json-faqs.html

When I remove local stack and send the requests to AWS directly I get a valid response.

const AWS = require('aws-sdk');

const sqs = new AWS.SQS({
    region: 'us-east-1',
});

const params = {
    QueueName: 'test-queue-3',
        Attributes: {
            'DelaySeconds': '60',
            'MessageRetentionPeriod': '86400'
    }
};
sqs.createQueue(params, function(err, data) {
    if (err) {
        console.error("Error Creating Queue", err);             
    } else {
        console.log("Created Queue!", JSON.stringify(data, null, 4));
    }
});

/*
Created Queue! {
    "ResponseMetadata": {
        "RequestId": "REDACTED"
    },
    "QueueUrl": "https://sqs.us-east-1.amazonaws.com/REDACTED/test-queue-3"
}
*/

I assume the other folks on the thread that are commenting are also using some sort of AWS Service clone?

The conclusion here is that the SDK is working as expected - as specified by the SQS service itself. The solution here would be for Localstack to make the necassary changes to follow the SQS service JSON RPC wire implementation.

Thanks,
Ran~

@RanVaknin RanVaknin added guidance Question that needs advice or information. response-requested Waiting on additional info and feedback. Will move to \"closing-soon\" in 7 days. and removed bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. p1 This is a high priority issue labels Nov 29, 2023
@award1230
Copy link
Author

@RanVaknin Thank you for the response. I am on an older localstack version, I can update and see if that solves the issue.

Recently the SQS service moved from XML wire protocol to a more modern JSON RPC protocol. Since the SQS service changed the protocol, the SQS Client now sends the requests in JSON format

Is there any way to specify that we want to use the XML wire protocol still? Or is that protocol completely unsupported in AWS at this point? It would be ideal to have a way to make this backward compatible with older Localstack/AWS versions as updating localstack may take us some time.

@Noezor
Copy link

Noezor commented Nov 29, 2023

👋 @RanVaknin , thanks for the answer.

(realizing I'm addressing a slightly different concern about SendMessageBatch, but I guess both issues are linked and you probably came here because this is the issue we raised to Support, let me know if you instead want to move the discussion to aws/aws-sdk#657)

I'm not super convinced by the depiction that the SDK is working as expected, or at least not as I would expect. As pointed here by @gersmann, the Failed field is required in the API response of SendMessageBatch. It only feels natural that it is also included in the sdk answer, and that libraries would build on this assumption and not expect a minor sdk update to stop returning a field that is required in the API.

Could we re-add this field in the SDK response so as to be less surprising vs the API documentation, or modify the API documentation to make it explicit the SDK does not match it?

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to \"closing-soon\" in 7 days. label Nov 30, 2023
@RanVaknin
Copy link
Contributor

Hi @award1230 ,

Unfortunately the SDK does not support both wire protocols, the SDK is generated directly from the API model itself, and since there is only one network protocol supported per service, the SDK will only be generated with support for that protocol.

While I understand the pain point, AWS cannot guarantee compatibility with 3rd party tools like Localstack or other clones.
I suggest you give the newest Localstack version a try, and if its not updated, I suggest reaching out to Localstack to upgrade their offering to match the new SQS protocol change.

Hi @Noezor ,

The assumption that the SDK behaves correctly was with regards to this specific problem. There is a good chance that this rollout did break other things, and I appreciate you pointing us to your findings. I'll look into the aws/aws-sdk#657 with priority and let you know what I can find.

Thanks,
Ran~

@RanVaknin RanVaknin added p0 This issue is the highest priority bug This issue is a bug. and removed guidance Question that needs advice or information. labels Dec 3, 2023
@RanVaknin
Copy link
Contributor

RanVaknin commented Dec 4, 2023

Closing because no action is needed from the SDK team.

@Noezor
Copy link

Noezor commented Dec 5, 2023

Hi @RanVaknin . Does this also fix aws/aws-sdk#657 ?

@RanVaknin
Copy link
Contributor

@Noezor ,

Sorry! That answer was meant for aws/aws-sdk#657.
I updated that ticket.

Please pull the latest version and see if it solves the problem.

Thanks,
Ran~

@RanVaknin RanVaknin added p3 This is a minor priority issue and removed p0 This issue is the highest priority labels Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. p3 This is a minor priority issue
Projects
None yet
Development

No branches or pull requests

5 participants