brefphp / laravel-bridge

Package to use Laravel on AWS Lambda with Bref
https://bref.sh/docs/frameworks/laravel.html
MIT License
314 stars 63 forks source link

SQS QueueHandler tried to delete message, got error `400 Bad Request` #120

Closed hungnv-sr closed 1 year ago

hungnv-sr commented 1 year ago

Problem: QueueHandler tries to delete message, which it should not, since Lambda will delete the message for you if the function success.

Queue handler code:

...
return $app->makeWith(QueueHandler::class, [
    'connection' => getenv('QUEUE_CONNECTION'),
    'queue' => getenv('SQS_QUEUE'),
]);

Error:

Error executing "DeleteMessage" on "https://sqs.REGION.amazonaws.com/144211545523/arn:aws:sqs:ap-northeast-1:144211545523:dev-master-queue"; AWS HTTP error: Client error: `POST https://sqs.ap-northeast-1.amazonaws.com/144211545523/arn:aws:sqs:ap-northeast-1:144211545523:dev-master-queue` resulted in a `400 Bad Request` response:
...
AWS.SimpleQueueService.NonExistentQueue (client): The specified queue does not exist for this wsdl version. - <?xml version=\"1.0\"?><ErrorResponse xmlns=\"http://queue.amazonaws.com/doc/2012-11-05/\"><Error><Type>Sender</Type><Code>AWS.SimpleQueueService.NonExistentQueue</Code><Message>The specified queue does not exist for this wsdl version.</Message><Detail/></Error><RequestId>80b77a59-755a-511a-94cb-2686bcb3ec0b</RequestId></ErrorResponse> at /var/task/vendor/aws/aws-sdk-php/src/WrappedHttpHandler.php:195)
...

Changing configuration like this doesn't work

'queue' => getenv('SQS_PREFIX').'/'.getenv('SQS_QUEUE'),

Quick solution: You need to change SQS_QUEUE to the actual queue name, which is not in sync with Laravel configuration.

SQS_QUEUE: !GetAtt DenchouSqsQueue.QueueName

Another solution: Do not delete the job after processing.

hungnv-sr commented 1 year ago

I tried to create a pull request but got 403 when pushed to a branch

mnapoli commented 1 year ago

It looks like you provided the queue ARN instead of URL in the config (which is invalid), could you share how you configured it?

hungnv-sr commented 1 year ago

My settings in template.yml :

    SQS_QUEUE: !GetAtt SQS.Arn
    SQS_PREFIX: !Sub 'https://sqs.${AWS::Region}.amazonaws.com/${AWS::AccountId}'
    SQS_QUEUE_NAME: !GetAtt SQS.QueueName

Laravel need SQS_QUEUE and SQS PREFIX in env, which works fine

        'sqs' => [
            'driver' => 'sqs',
            'key' => env('AWS_ACCESS_KEY_ID'),
            'secret' => env('AWS_SECRET_ACCESS_KEY'),
            'token' => env('AWS_SESSION_TOKEN'),
            'prefix' => env('SQS_PREFIX', 'https://sqs.us-east-1.amazonaws.com/your-account-id'),
            'queue' => env('SQS_QUEUE', 'default'),
            'suffix' => env('SQS_SUFFIX'),
            'region' => env('AWS_DEFAULT_REGION', 'ap-northeast-1'),
            'after_commit' => false,
        ],

I tried this but it didn't work, it still generate the queue arn as SQS_PREFIX + SQS_QUEUE

'queue' => getenv('SQS_PREFIX').'/'.getenv('SQS_QUEUE_NAME'),

I can reproduce it every time.

The problem is that it should not delete the queue after processing the message, Lambda will do it for you.

For example, if you make Laravel to retry the queue, after worker fails to retry, the queue is gone while it should go to DeadLetterQueue

This can be fixed by removing line 94-96 in src/Queue/QueueHandler.php

            if (! $job->hasFailed() && ! $job->isDeleted()) {
                $job->delete();
            }

Edit: I had to chang SQS_QUEUE for it work, it didn't read from custom settimg 'queue' => ...,

mnapoli commented 1 year ago

Try:

    SQS_QUEUE: !GetAtt SQS.QueueUrl
hungnv-sr commented 1 year ago

@mnapoli What about this? What's your opinion?

The problem is that it should not delete the queue after processing the message, Lambda will do it for you.

this works btw, it must be a different setting from Laravel (which also discussed in another issue)

SQS_QUEUE: !GetAtt SQS.QueueName
mnapoli commented 1 year ago

The problem is that it should not delete the queue after processing the message, Lambda will do it for you.

This is not a problem. It has no negative consequence.

You are seeing an error with this action because your configuration is wrong. Not because the action is happening. That is why I am trying to help you fix your configuration, because it is the problem.

hungnv-sr commented 1 year ago

You are seeing an error with this action because your configuration is wrong

I agree.

The problem is that it should not delete the queue after processing the message, Lambda will do it for you.

I still think this is a problem

For example, if you make Laravel to retry the queue, after worker fails to retry, the queue is gone while it should go to DeadLetterQueue

Thanks for replying.