Incorrect complaint wrt provided types

[sawilde]

Describe the bug

Migrating aws/aws-sdk-php from 3.285.0 to 3.286.1 we received the following error

{"status":"CRITICAL","timestamp":1700084969235,"channel":"console","message":"User Warning: The provided type for `Attributes -> ReceiveMessageWaitTimeSeconds` value was `integer`. The modeled type is `string`.","command":"localstack:setup:queues","error":{"message":"User Warning: The provided type for `Attributes -> ReceiveMessageWaitTimeSeconds` value was `integer`. The modeled type is `string`.","stack":"#0 /var/www/vendor/aws/aws-sdk-php/src/QueryCompatibleInputMiddleware.php(90): Aws\\QueryCompatibleInputMiddleware->processScalar()\n#1 /var/www/vendor/aws/aws-sdk-php/src/QueryCompatibleInputMiddleware.php(145): Aws\\QueryCompatibleInputMiddleware->processInput()\n#2 /var/www/vendor/aws/aws-sdk-php/src/QueryCompatibleInputMiddleware.php(87): Aws\\QueryCompatibleInputMiddleware->processMap()\n#3 /var/www/vendor/aws/aws-sdk-php/src/QueryCompatibleInputMiddleware.php(60): Aws\\QueryCompatibleInputMiddleware->processInput()\n#4 /var/www/vendor/aws/aws-sdk-php/src/Middleware.php(89): Aws\\QueryCompatibleInputMiddleware->__invoke()\n#5 /var/www/vendor/aws/aws-sdk-php/src/IdempotencyTokenMiddleware.php(77): Aws\\Middleware::Aws\\{closure}()\n#6 /var/www/vendor/aws/aws-sdk-php/src/AwsClientTrait.php(64): Aws\\IdempotencyTokenMiddleware->__invoke()\n#7 /var/www/vendor/aws/aws-sdk-php/src/AwsClientTrait.php(58): Aws\\AwsClient->executeAsync()\n#8 /var/www/vendor/aws/aws-sdk-php/src/AwsClientTrait.php(86): Aws\\AwsClient->execute()\n#9 /tmp/var/cache/Container3WXPXpM/appAppKernelDevDebugContainer.php(72425): Aws\\AwsClient->__call()\n#10

when trying the create a queue in localstack (we are using the latest v3 of localstack that now supports JSON 1.0 for SQS) using the following code

$this->sqsClient->createQueue([
           'QueueName' => $queueName,
           'Attributes' => [
               'ReceiveMessageWaitTimeSeconds' => 20_000,
           ],
       ]);

fixing this is trivial (convert to string) but I am not convinced this is the correct approach as the model we were using is correct for the spec for CreateQueue (https://docs.aws.amazon.com/AWSSimpleQueueService/latest/APIReference/API_CreateQueue.html) indicates that ReceiveMessageWaitTimeSeconds is an integer

I also get the same issue with MaximumMessageSize attribute (also defined as an integer).

Expected Behavior

Integer attributes for fields such as ReceiveMessageWaitTimeSeconds and MaximumMessageSize should be accepted as integers as there were in 3.285.0

Current Behavior

Integer values are currently being rejected and now need to be converted to strings

Reproduction Steps

 $this->sqsClient->createQueue([
            'QueueName' => $queueName,
            'Attributes' => [
                'ReceiveMessageWaitTimeSeconds' => 20_000,
            ],
        ]);

Possible Solution

No response

Additional Information/Context

No response

SDK version used

3.286.1

Environment details (Version of PHP (php -v)? OS name and version, etc.)

PHP 8.2.9 (cli) (built: Aug 17 2023 22:52:44) (NTS)

[stobrien89]

Hi @sawilde,

Sorry for the noise. The documentation you're referencing is unfortunately incorrect. I'll contact the SQS team to have that updated.

That particular shape is modeled as a list of strings. In the process of migrating SQS from query to json protocol, we found that things like this (i.e. ints being passed when they should be strings) worked because they're built into a query string and parsed out by the service. In order to avoid breaking customers, we added middleware that emits a warning if an incorrect type is found and then casts it to its correct type. I'd recommend updating the value to a string whenever you have a chance.

[sawilde]

@stobrien89 thanks for the update - I am surprised the doc error has gone unnoticed for so long

[sawilde]

However should we be exposing API internals to users like this in the SDK, since the field is an integer (I can't give it wibble as a valid value) by using the correct type in the SDK is surely better and safer?

[stobrien89]

I think it's just due to the fact that it didn't matter before since it all ended up in a string. Now that input values must be their modeled type (after the migration to json), I think we might see a bit more of this. The SQS team is aware of this and should have their docs fixed soon.

Could you elaborate on your last comment?

[sawilde]

I think having to convert something that is an integer (real world usage eg message size) into a string to meet a API transport layer need is something I would expect an SDK to do and is not something a developer should have to think about. We model it as a integer in our heads and that is how we expose it through our own code on our way to calling the SDK. Now if it is a string there is potential for developers to leak that implementation specific of message size being a string (though it is really an int converted to a string) through their own code.

This is just my opinion of what I expect of an SDK to do for me.

[stobrien89]

I see what you're saying now and I understand how that can be frustrating as a developer. I think historically we as an organization were of that mind years ago (see the JS SDK V2), but that has changed over time as we've added new (approaching 400) services while simultaneously striving for behavior parity across all of the AWS language SDKs. Delegating the responsibility of handling input validation to AWS services is more of the direction we're heading now— this is a much more scaleable approach for us.

Closing this for now, but please let us know if you have any additional questions.

[github-actions]

⚠️COMMENT VISIBILITY WARNING⚠️

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.

[coopan]

@stobrien89 @sawilde I understand both points of view here. Thanks for the comments. But I'm a little confused and seeing these warnings too. The part I'm confused about is we are getting conflicting warnings for the same element name. It just depends if its in Attributes or not. Attributes seem to always need to be strings, even if the element also needs to be Int in another location.

E.G. Consider VisibilityTimeout

$args = [
            'QueueUrl' => $queueUrl,
            'ReceiptHandle' => $message['ReceiptHandle'],
            'VisibilityTimeout' => (string) $delayTime,
        ];
$result = $this->sqsClient->changeMessageVisibility($args);

E_USER_WARNING: The provided type for VisibilityTimeout value was string. The modeled type is integer.

$queueAttributes = [
            'MessageRetentionPeriod' => '345600',
            'DelaySeconds' => 0,
            'RedrivePolicy' => json_encode([
                'maxReceiveCount' => 10,
                'deadLetterTargetArn' => $defaultDeadLetterArn,
            ]),
            'VisibilityTimeout' => 900,
            'KmsMasterKeyId' => 'alias/aws/sqs',
        ];
        $createQueueResult = $this->sqsClient->createQueue(
            [
                'QueueName' => $queueFullName,
                'Attributes' => $queueAttributes,
            ]
        );

E_USER_WARNING: The provided type for Attributes -> VisibilityTimeout value was integer. The modeled type is string

I can see what the fix is here, but beyond the discussion regarding type handling, isn't this terribly inconsistent? Or am I missing something?