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

Fix possible undefined index when caching config data #13649

Conversation

mimarcel
Copy link
Contributor

Description

Fix possible undefined index when caching config data.

This replicates as per below steps to replicate.

Manual testing scenarios

  1. Install clean Magento 2.2 with the following commands:
cd /var/www/temp
git clone git@github.com:magento/magento2.git --branch 2.2-develop
cd magento2
composer --no-ansi --no-interaction install --no-progress --prefer-dist --optimize-autoloader --no-dev
  1. In order to replicate the issue, we will need to add some action which connects to config data during build command. We will add a logger processor which does add request_uri value for every log message, if the configuration flag dev/log/request_uri is enabled. The following sub-steps add a new logger processor in Magento_Developer module:
  • Add in file /var/www/temp/magento2/app/code/Magento/Developer/etc/di.xml the following content under <config> tag:
    <type name="\Magento\Framework\Logger\Monolog">
        <arguments>
            <argument name="processors" xsi:type="array">
                <item name="Magento_Developer" xsi:type="object">\Magento\Developer\Logger\Processor</item>
            </argument>
        </arguments>
    </type>
  • Add new file in
    /var/www/temp/magento2/app/code/Magento/Developer/Logger/Processor.php with the following content:
<?php
namespace Magento\Developer\Logger;

class Processor
{
    /** @var \Magento\Framework\App\Config\ScopeConfigInterface */
    private $scopeConfig;

    public function __construct(
        \Magento\Framework\App\Config\ScopeConfigInterface $scopeConfig
    ) {
        $this->scopeConfig = $scopeConfig;
    }

    public function __invoke($record)
    {
        if ($this->scopeConfig->isSetFlag('dev/log/request_uri') && is_array($record)) {
            if (!isset($record['extra'])) {
                $record['extra'] = [];
            }
            $record['extra']['request_uri'] = $_SERVER['REQUEST_URI'] ?? null;
        }

        return $record;
    }
}
  1. Run the following two commands which are part of usual build procedure:
php bin/magento module:enable --all
php bin/magento setup:di:compile -v

Expected
Command setup:di:compile is successful.

Actual
Command setup:di:compile is not successful.
The following error occurs:

  [Exception]
  Notice: Undefined index: websites in /private/var/www/temp/magento2/app/code/Magento/Config/App/Config/Type/System.php on line 248


Exception trace:
 () at /private/var/www/temp/magento2/lib/internal/Magento/Framework/App/ErrorHandler.php:61
 Magento\Framework\App\ErrorHandler->handler() at /private/var/www/temp/magento2/app/code/Magento/Config/App/Config/Type/System.php:248
 Magento\Config\App\Config\Type\System->cacheData() at /private/var/www/temp/magento2/app/code/Magento/Config/App/Config/Type/System.php:192
 Magento\Config\App\Config\Type\System->loadDefaultScopeData() at /private/var/www/temp/magento2/app/code/Magento/Config/App/Config/Type/System.php:152
 Magento\Config\App\Config\Type\System->get() at /private/var/www/temp/magento2/lib/internal/Magento/Framework/App/Config.php:131
 Magento\Framework\App\Config->get() at /private/var/www/temp/magento2/lib/internal/Magento/Framework/App/Config.php:80
 Magento\Framework\App\Config->getValue() at /private/var/www/temp/magento2/lib/internal/Magento/Framework/App/Config.php:93
 Magento\Framework\App\Config->isSetFlag() at /private/var/www/temp/magento2/app/code/Magento/Developer/Logger/Processor.php:23
 Magento\Developer\Logger\Processor->__invoke() at n/a:n/a
 call_user_func() at /private/var/www/temp/magento2/vendor/monolog/monolog/src/Monolog/Logger.php:333
 Monolog\Logger->addRecord() at /private/var/www/temp/magento2/lib/internal/Magento/Framework/Logger/Monolog.php:48
 Magento\Framework\Logger\Monolog->addRecord() at /private/var/www/temp/magento2/vendor/monolog/monolog/src/Monolog/Logger.php:532
 Monolog\Logger->debug() at /private/var/www/temp/magento2/lib/internal/Magento/Framework/Cache/InvalidateLogger.php:42
 Magento\Framework\Cache\InvalidateLogger->execute() at /private/var/www/temp/magento2/lib/internal/Magento/Framework/Cache/Frontend/Decorator/Logger.php:58
 Magento\Framework\Cache\Frontend\Decorator\Logger->log() at /private/var/www/temp/magento2/lib/internal/Magento/Framework/Cache/Frontend/Decorator/Logger.php:48
 Magento\Framework\Cache\Frontend\Decorator\Logger->clean() at /private/var/www/temp/magento2/lib/internal/Magento/Framework/App/Cache.php:102
 Magento\Framework\App\Cache->clean() at /private/var/www/temp/magento2/setup/src/Magento/Setup/Console/Command/DiCompileCommand.php:152
 Magento\Setup\Console\Command\DiCompileCommand->execute() at /private/var/www/temp/magento2/vendor/symfony/console/Command/Command.php:242
 Symfony\Component\Console\Command\Command->run() at /private/var/www/temp/magento2/vendor/symfony/console/Application.php:842
 Symfony\Component\Console\Application->doRunCommand() at /private/var/www/temp/magento2/vendor/symfony/console/Application.php:193
 Symfony\Component\Console\Application->doRun() at /private/var/www/temp/magento2/lib/internal/Magento/Framework/Console/Cli.php:104
 Magento\Framework\Console\Cli->doRun() at /private/var/www/temp/magento2/vendor/symfony/console/Application.php:117
 Symfony\Component\Console\Application->run() at /private/var/www/temp/magento2/bin/magento:23

setup:di:compile

Note that valid code added in the previous steps does trigger the above error. The exception trace contains line:

/private/var/www/temp/magento2/app/code/Magento/Developer/Logger/Processor.php:23

Explanation
The error occurs because at this step of loading config data, there are no stores and no websites since Magento is not installed. As a consequence, config data does not contain any store-specific or website-specific configurations.

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

@orlangur orlangur self-assigned this Feb 14, 2018
@orlangur
Copy link
Contributor

Hi @mimarcel

Install clean Magento 2.2 with the following commands:

The error occurs because at this step of loading config data, there are no stores and no websites since Magento is not installed.

So Magento is not installed actually? Why need to execute bin/magento setup:di:compile prior to installation?

@mimarcel
Copy link
Contributor Author

mimarcel commented Feb 14, 2018

@orlangur this occurs when I push to a Magento Cloud environment where Magento was not installed yet, as part of php ./vendor/bin/m2-ece-build script.

UPDATE: It actually seems that the environment has a Magento instance installed, but I suppose that php ./vendor/bin/m2-ece-build script runs in a separate box/environment to prepare the build.

@sidolov
Copy link
Contributor

sidolov commented Aug 10, 2018

Hi @mimarcel , please, sign CLA, otherwise, we can't process your pull request

@mimarcel
Copy link
Contributor Author

@sidolov , I have troubles signing the license agreement.
image
How should I proceed?

@sidolov
Copy link
Contributor

sidolov commented Aug 10, 2018

@mimarcel , try to refresh the page few times, sometimes we have issue with CLA page load

@mimarcel
Copy link
Contributor Author

@sidolov, done.

@magento-engcom-team
Copy link
Contributor

@mimarcel thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository.

@magento-engcom-team
Copy link
Contributor

Hi @mimarcel. Thank you for your contribution.
We will aim to release these changes as part of 2.2.7.
Please check the release notes for final confirmation.

Please, consider to port this solution to 2.3 release line.
You may use Porting tool to port commits automatically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants