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

[BUG]: Register CLI application throwing warning #16186

Closed
dz3n opened this issue Oct 26, 2022 · 9 comments · Fixed by #16189
Closed

[BUG]: Register CLI application throwing warning #16186

dz3n opened this issue Oct 26, 2022 · 9 comments · Fixed by #16189
Assignees
Labels
5.0 The issues we want to solve in the 5.0 release bug A bug report status: low Low

Comments

@dz3n
Copy link

dz3n commented Oct 26, 2022

Describe the bug
Throwing deprecated warning

To Reproduce
Based on documentation, to register CLI application code like this is required:

//cli.php
$console = new Console($container);

$arguments = [];
foreach ($argv as $k => $arg) {
    if ($k === 1) {
        $arguments['task'] = $arg;
    } elseif ($k === 2) {
        $arguments['action'] = $arg;
    } elseif ($k >= 3) {
        $arguments['params'][] = $arg;
    }
}
$console->handle($arguments);

Which cause this kind of warning

Deprecated: Phalcon\Dispatcher\AbstractDispatcher::setActionName(): Passing null to parameter #1 ($actionName) of type string is deprecated in /var/www/application/apps/crontabs/cli.php on line 283

Details

  • Phalcon version: 5.0.4
  • PHP Version: PHP 8.1.7
  • Operating System:
  • Installation type: Ubuntu + Docker
  • Server: Nginx
@dz3n dz3n added bug A bug report status: unverified Unverified labels Oct 26, 2022
@Jeckerson
Copy link
Member

Could please specify what is on line 283 in cli.php file?

@Jeckerson Jeckerson self-assigned this Oct 26, 2022
@dz3n
Copy link
Author

dz3n commented Oct 26, 2022

Could please specify what is on line 283 in cli.php file?

I've edited my previous post.

//line 283
$console->handle($arguments);

Thanks!

@niden
Copy link
Member

niden commented Oct 26, 2022

@dz3n If possible can you also post the task you are trying to run? (the task class) as well as the command you are invoking in the CLI?

@Alistar84
Copy link

I have the same problem. I solved it by always passing the second parameter, even though it's mainAction

@dz3n
Copy link
Author

dz3n commented Oct 26, 2022

@dz3n If possible can you also post the task you are trying to run? (the task class) as well as the command you are invoking in the CLI?

@niden

The task class doesn't matter really

the command is as follow:
/usr/local/bin/php /var/www/application/apps/crontabs/cli.php taskclass

without any action and params arguments

If I change it with second param like this
/usr/local/bin/php /var/www/application/apps/crontabs/cli.php taskclass main

I don't get warnings, so @Alistar84 is right, I need to pass second argument in order to get rid of this error

@Jeckerson Jeckerson assigned niden and unassigned Jeckerson Oct 26, 2022
@Jeckerson Jeckerson added status: low Low 5.0 The issues we want to solve in the 5.0 release and removed status: unverified Unverified labels Oct 26, 2022
@niden
Copy link
Member

niden commented Oct 28, 2022

@dz3n I am having some trouble recreating this.

First of all in your code you can make a small change so that you don't have to run each command with main. Look at this:

//cli.php
$console = new Console($container);

$arguments = [
    'action' => 'main',
];
foreach ($argv as $k => $arg) {
    if ($k === 1) {
        $arguments['task'] = $arg;
    } elseif ($k === 2) {
        $arguments['action'] = $arg;
    } elseif ($k >= 3) {
        $arguments['params'][] = $arg;
    }
}
$console->handle($arguments);

Now for my test I am using the tasks in our testing suite. My cli.php is as follows:

<?php
declare(strict_types=1);

error_reporting(E_ALL);
ini_set("display_errors", 'On');

use Phalcon\Cli\Console;
use Phalcon\Cli\Dispatcher;
use Phalcon\Di\FactoryDefault\Cli as CliDI;

require_once 'vendor/autoload.php';

$container  = new CliDI();
$dispatcher = new Dispatcher();

$dispatcher->setDefaultNamespace('Phalcon\Tests\Fixtures\Tasks');
$container->setShared('dispatcher', $dispatcher);

$console = new Console($container);

$arguments = [];
foreach ($argv as $k => $arg) {
    if ($k === 1) {
        $arguments['task'] = $arg;
    } elseif ($k === 2) {
        $arguments['action'] = $arg;
    } elseif ($k >= 3) {
        $arguments['params'][] = $arg;
    }
}

try {
    $console->handle($arguments);
} catch (\Exception $e) {
    fwrite(STDERR, $e->getMessage() . PHP_EOL);
    exit(1);
} catch (Throwable $throwable) {
    fwrite(STDERR, $throwable->getMessage() . PHP_EOL);
    exit(1);
}

Then I run (in a localr docker environment)

php -d extension=ext/modules/phalcon.so aa.php params

This executes the ParamsTask (tests/_data/fixtures/Tasks/ParamsTask.php) which does not have a mainAction in it. The result I get back is as expected:

Action 'main' was not found in handler 'params'

If I run it for the EchoTask (tests/_data/fixtures/Tasks/EchoTask.php) and modify that task slightly to echo 'Hello' in the main action I get

php -d extension=ext/modules/phalcon.so aa.php echo
Hello

@niden
Copy link
Member

niden commented Oct 28, 2022

I am not sure why this happens and I cannot get this to replicate. Feel free to email me directly ([email protected]) so that we can arrange some sort of screen sharing to figure this out. Alternatively, if this is not feasible, have a look above for any changes between my code and yours and let me know.

@niden
Copy link
Member

niden commented Oct 28, 2022

@dz3n never mind I managed to reproduce it

@niden niden linked a pull request Oct 31, 2022 that will close this issue
5 tasks
@niden
Copy link
Member

niden commented Oct 31, 2022

Resolved in #16189

Thank you @dz3n

@niden niden closed this as completed Oct 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5.0 The issues we want to solve in the 5.0 release bug A bug report status: low Low
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants