dusterio / laravel-plain-sqs

Custom SQS connector for Laravel (or Lumen) that supports third-party, plain JSON messages
MIT License
131 stars 93 forks source link

Wrong payload format stored when jobs fail and are tried multiple times #4

Closed sebastianvilla closed 8 years ago

sebastianvilla commented 8 years ago

Hey @dusterio, great package!

While trying different scenarios, we noticed failed jobs cannot be tried multiple times because each time their payload is stored in the failed_jobs table, the format is lost and it keeps burying the original message body deeper each time the job fails.

We have tried a few things in hopes of submitting a PR but haven't had much success yet and figured you may have any ideas.

You can see this behaviour by setting up a job that throws an Exception, which will cause it to go into the failed_jobs table. Retry the job a few times using php artisan queue:retry ID and observe the entire payload being re-serialized into the data property each time.

Here is what the payload looks like for a native Laravel failed job (using AWs SQS as well): {"job":"Illuminate\\Queue\\CallQueuedHandler@call","data":{"command":"O:39:\"App\\Jobs\\TestSQSJobHandler\":5:{s:16:\"\u0000*\u0000song_id\";i:3;s:10:\"connection\";N;s:5:\"queue\";s:14:\"songsToProcess\";s:5:\"delay\";N;s:6:\"\u0000*\u0000job\";N;}"}}

Here is the payload for a failed job using laravel-plain-sqs:

First time it fails: {"job":"App\\Jobs\\TestSQSJobHandlerPlain@handle","data":{"song_id":55,"artist":"Unknown"}}

After trying once via php artisan queue:retry job_id: {"job":"App\\Jobs\\TestSQSJobHandlerPlain@handle","data":{"job":"App\\Jobs\\TestSQSJobHandlerPlain@handle","data":{"song_id":55,"artist":"Unknown"}}}

After trying twice via php artisan queue:retry job_id: {"job":"App\\Jobs\\TestSQSJobHandlerPlain@handle","data":{"job":"App\\Jobs\\TestSQSJobHandlerPlain@handle","data":{"job":"App\\Jobs\\TestSQSJobHandlerPlain@handle","data":{"song_id":55,"artist":"Unknown"}}}}

Notice how each time the job is re-tried it re-encodes the data for the previous job, rendering the actual job unusable since the handler expects the data to contain specific JSON data such as song_id and artist.

Thanks!

dusterio commented 8 years ago

Thanks for spotting and reporting this! I will fix this shortly