bestmomo / nice-artisan

This package is to add a web interface for Laravel 5 and earlier Artisan.
213 stars 23 forks source link

Invalid argument supplied for foreach() when using queue:retry #18

Open svolenszki opened 5 years ago

svolenszki commented 5 years ago

Artisan command queue:retry accept array as id of failed job(s), but the NiceArtisanController command function assign it as a single variable (line 152), instead of array.

I don't know, if any other command has requirements like this, but an exception must handled in the assignation, when command is queue:retry.

Or the config file should have parameters, where the command can be setted up for this purpose.

kepernyofoto 2019-01-16 - 14 39 04

bestmomo commented 5 years ago

Hello I didn't know we can pass an array for this command. In the comment we can see: protected $signature = 'queue:retry {id* : The ID of the failed job.}'; Anyway when I try with an array like [1,2] I don't get your error.

svolenszki commented 5 years ago

Hi, if you debug the data in vendor/laravel/framework/src/Illuminate/Queue/Console/RetryCommand.php 32nd line (dump the $ids variable to file), you will get this, if you execute queue:retry in terminal:

Array ( [0] => 1 )

and this, if you use your user interface:

1

It means, the data passed in array, if you use the command in terminal, and passed as integer, if you use your user interface.

And if you look at the 38th line of the above mentioned file, you will see, the $ids variable must be a loopable variable:

foreach ($ids as $id) {...

And the error comes from this point.

I don't know if it is en environmental bug, or what, but in my install of Laravel 5.3, I get always this error in queue:retry executing from your user interface (in terminal all ok), no matter I type 1, or [1,2] or [0=>1].

bestmomo commented 5 years ago

I tested on Laravel 5.5, there have been many changes since release 5.3

svolenszki commented 5 years ago

I see. Do you have different branch for 5.3? Maybe you can fix it in the 5.3 branch, if not, I think you must specify, this is for Laravel 5.5.

bestmomo commented 5 years ago

I didn't make branches for this little package. Just made a new version for each Laravel release, so for Laravel 5.3 it's the V0.5, but so I cant make changes on this version.

svolenszki commented 5 years ago

Have you tested it in Laravel 5.3? If you tested, and you also can reproduce the error, then the fix would be nice.

bestmomo commented 5 years ago

Which version do you use in Laravel 5.3 ?

svolenszki commented 5 years ago

5.3.31

bestmomo commented 5 years ago

I know it is a very bad practice but you can skirt the problem easily casting for array in Laravel command on line 32:

$ids = (array)$this->argument('id');

I'll see forward for the package.

svolenszki commented 5 years ago

I know, but this app is deployed with docker to AWS, so the source is not the part of the project. I think I will override your controller with a provider, until you come out with the fix. Thank you.