- Issue created by @apotek
- Merge request !26Ensure messenger is instantiated before it is required by any methods. → (Open) created by apotek
The VclHandler class's constructor unfortunately validates its ability to connect on instantiation. And because this check happens in the middle of the constructor, several class properties are never initialized.
Here's the connection check:
$connection = $this->api->testFastlyApiConnection();
if (!$connection['status']) {
$this->addError($connection['message']);
return;
}
This means the constructor never completes if there is a configuration issue.
So if code calls `VclHandler::execute()` there will be an error, and the execute() function looks like this:
public function execute($activate = FALSE) {
// Check if there are connection errors from construct.
$errors = $this->getErrors();
if (!empty($errors)) {
foreach ($errors as $error) {
$this->messenger->addError($error);
}
return FALSE;
}
...
...
...
But the Messenger is never instantiated as a class variable, so, when execute() is called and Fastly's connection information is misconfigured, of course an error is found from the constructor, but when VclHandler tries to write the errors, it tries this:
$this->messenger->addError($error);
But messenger is not instantiated, so, in addition to the connection failure, we get an ugly Exception and stack trace.
Call VclHandler->execute() from your code, with bad connection info.
(For example, how I came across this was writing a custom drush command (fastly:pushvcl) where the VclHandler service is injected). If my connection information is good, everything is fine.
If I unset the service_id or token, the constructor returns early, the messenger property is never set and the execute() call fails with:
[error] Connection not Successful on service - status : 404
[error] Error: Call to a member function addError() on null in Drupal\fastly\VclHandler->execute() (line 495 of /var/www/html/web/modules/contrib/fastly/src/VclHandler.php) #0 /var/www/html/web/modules/custom/benz_deployment/src/Commands/BenzDeploymentCommands.php(39): Drupal\fastly\VclHandler->execute(true)
#1 [internal function]: Drupal\benz_deployment\Commands\BenzDeploymentCommands->pushVcl(Array)
#2 /var/www/html/vendor/consolidation/annotated-command/src/CommandProcessor.php(276): call_user_func_array(Array, Array)
#3 /var/www/html/vendor/consolidation/annotated-command/src/CommandProcessor.php(212): Consolidation\AnnotatedCommand\CommandProcessor->runCommandCallback(Array, Object(Consolidation\AnnotatedCommand\CommandData))
#4 /var/www/html/vendor/consolidation/annotated-command/src/CommandProcessor.php(175): Consolidation\AnnotatedCommand\CommandProcessor->validateRunAndAlter(Array, Array, Object(Consolidation\AnnotatedCommand\CommandData))
#5 /var/www/html/vendor/consolidation/annotated-command/src/AnnotatedCommand.php(387): Consolidation\AnnotatedCommand\CommandProcessor->process(Object(Symfony\Component\Console\Output\ConsoleOutput), Array, Array, Object(Consolidation\AnnotatedCommand\CommandData))
#6 /var/www/html/vendor/symfony/console/Command/Command.php(326): Consolidation\AnnotatedCommand\AnnotatedCommand->execute(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#7 /var/www/html/vendor/symfony/console/Application.php(1096): Symfony\Component\Console\Command\Command->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#8 /var/www/html/vendor/symfony/console/Application.php(324): Symfony\Component\Console\Application->doRunCommand(Object(Consolidation\AnnotatedCommand\AnnotatedCommand), Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#9 /var/www/html/vendor/symfony/console/Application.php(175): Symfony\Component\Console\Application->doRun(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#10 /var/www/html/vendor/drush/drush/src/Runtime/Runtime.php(110): Symfony\Component\Console\Application->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#11 /var/www/html/vendor/drush/drush/src/Runtime/Runtime.php(40): Drush\Runtime\Runtime->doRun(Array, Object(Symfony\Component\Console\Output\ConsoleOutput))
#12 /var/www/html/vendor/drush/drush/drush.php(139): Drush\Runtime\Runtime->run(Array)
#13 /var/www/html/vendor/drush/drush/drush(4): require('/var/www/html/v...')
#14 /var/www/html/vendor/bin/drush(119): include('/var/www/html/v...')
#15 {main}.
Best practice would be to not validate the connection in a constructor call, and then validate the connection in methods that need a valid connection.
The simpler/quicker fix though, is to move this line:
$this->messenger = $messenger;
Above the connection check, probably here:
$this->request = $requestStack->getCurrentRequest();
+ $this->messenger = $messenger;
$connection = $this->api->testFastlyApiConnection();
Submit a merge request with the simplest proposal, unless there is maintainer buy in on the larger issue of the constructor not always leaving the object in a predictable state.
none.
none.
none.
Active
4.0
Code