Update DeployManager.php to hard fail on errors

[convenient]

We had a project fail to composer install with the following error

PHP Warning: Uncaught Exception: Warning: require(/var/www/vhosts/example.com/staging/releases/1602858012/setup/config/application.config.php): failed to open stream: No such file or directory in /var/www/vhosts/example.com/staging/releases/1602858012/vendor/magento/framework/Console/Cli.php on line 78 in /var/www/vhosts/example.com/staging/releases/1602858012/vendor/magento/framework/App/ErrorHandler.php:61
Stack trace:
#0 /var/www/vhosts/example.com/staging/releases/1602858012/vendor/magento/framework/Console/Cli.php(78): Magento\Framework\App\ErrorHandler->handler(2, 'require(/var/ww...', '/var/www/vhosts...', 78, Array)
#1 /var/www/vhosts/example.com/staging/releases/1602858012/vendor/magento/framework/Console/Cli.php(78): require()
#2 /var/www/vhosts/example.com/staging/releases/1602858012/bin/magento(22): Magento\Framework\Console\Cli->__construct('Magento CLI')
#3 {main}
thrown in /var/www/vhosts/example.com/staging/releases/1602858012/vendor/magento/framework/App/ErrorHandler.php on line 61

When debugging i could see that a lot of the files from magento/magento2-base had not been copied over. I can see an error produced when running with -vvv

start magento deploy via deployManager
start magento deploy for magento/magento2-base
mkdir(): File exists
start magento deploy for magento/magento2-base
mkdir(): File exists
start magento deploy for magento/magento2-ee-base
jump over deployLibraries as no Magento libraryPath is set

I debugged and the issue was that previously on our NFS we had no pub/media/import but now we have had one created.

Now when magento tried to composer install it was trying to copy over https://github.com/magento/magento2/blob/2.4-develop/pub/media/import/.htaccess, it faced this File exists error and stopped copying over the remainder of the files silently.

I think composer install should fail loud and early in these scenarios, removing the error handling entirely allows the exception to bubble up.

Any feedback appreciated

[hostep]

Aha, this might fix magento/magento2#10292 (comment) I think, which would be very nice!

[convenient]

@hostep yeah this is the exact kind of error this will flag up and catch.

[convenient]

Who maintains this repository? Can I have some feedback or a review? I think having composer install fail silently in the background is bad code smell.

[hostep]

Let me try to pull in @sidolov or @sivaschenko, it looks like community team sometimes focusses too much on https://github.com/magento/magento2 and forgets about all their other repositories 😉

[convenient]

Thanks @hostep

I think this change is sensible, no matter what way I spin it I want errors in composer install to be loud and not silenced, as a silenced error is something i'll just have to debug.

No one should be running composer install directly on the live site, and even if they are it will have failed to do something with this silencing try/catch. So probably best to bubble it up and make the debugging of the task easier.

If composer install is being run in some other circumstance and is being packaged before deployment, well then the live site is not affected by this change and the developer will have something to help them debug the error that is otherwise silent.

[sivaschenko]

@convenient @hostep thanks for the contribution and for reaching out, we will discuss this change, however, as we are processing PRs by priority it might require some time.

[sivaschenko]

These try-catch blocks have been introduced back in 2016 in the scope of internal task MAGETWO-48256 to fix magento/magento2#3056

Testing instructions from the internal ticket:
You should have zip based installation.

1. run composer update magento/magento-composer-installer and make sure you have installed version 0.1.6
2. run composer require magento/product-community-edition 2.0.1 --no-update
3. run composer update

[convenient]

@sivaschenko thank you for digging those notes out.

Wow, 2.0.1 references haha.

Before I try and reproduce whatever ancient error that is, are zip based Magento installs actually supported or recommended anywhere ?

I am on my phone but do not see any references to zip on https://devdocs.magento.com/guides/v2.4/install-gde/bk-install-guide.html

[hostep]

They are available from here, so I guess they are still supported: https://magento.com/tech-resources/download

[convenient]

Thanks @hostep

I'm thinking that the original "fix" to suppress all errors was a bit of a sledgehammer approach and we need something a bit more nuanced.

[convenient]

@sivaschenko I realistically don't have time to repro the zip install process.

Would you accept a sledgehammer on a sledgehammer fix? Can I put in some additional options like in the extra like we have for magento-deploy-sort-priority? Call it strict-failures to escalate these suppressed warnings or something?

That would be backwards compatible.

[hostep]

@sivaschenko: any updates here?

We lost about an hour of time today trying to debug a seemingly unrelated issue in our deploy. It turned out in the end that the pub/media directory which was on a NFS share was 100% full. And somehow the marshalling part of this composer plugin failed silently and didn't write out all the files it needed to write out (in our case, the setup directory was missing, and probably some other files/directories as well).

Not sure if this PR would have made that more clear, and would have pointed our attention in the correct direction immediately, but it would be nice if this could get some attention again.

So in case anything fails in the marshalling part of this composer plugin, we would like to see a very clear error message about what failed, and the composer install process should output an error code instead of 0.
Steps to reproduce have been written down some years ago in magento/magento2#10292 (comment)

Thanks!

[convenient]

@hostep I think this change would have made it kick up some fuss, its definitely done that for file permission errors in media for us in the past so a full disk seems like the kind of thing. It's a bit annoying burning an hour or so and then finding it was this.

I tried to do a composer patch to bring this in but this plugin was being triggered before the vaimo/composer-patches and I didn't want to be responsible for keeping a fork of this module up to date and replace this version so I've just left it for now.

When I suspect a bug is an issue with this plugin my approach is a bit like the following, it allows you to hack in this file and then trigger the install process for all magento modules and see where it goes bang.

# see my issue
composer install

# Move the magento-composer-installer out of the way
mv vendor/magento/magento-composer-installer/ magento-composer-installer

# Delete vendor magento install files
rm -rf vendor/magento
mkdir vendor/magento

# Put the magento-composer-installer back
mv magento-composer-installer vendor/magento/magento-composer-installer/

# put back in the hack to make exceptions noisy 🎉 
nano vendor/magento/magento-composer-installer/src/MagentoHackathon/Composer/Magento/DeployManager.php

# Re-run composer install to trigger the error
composer install

[hostep]

Hi @xmav, @karyna-tsymbal-atwix, @andrewbess: since you guys made significant changes to this plugin in the last couple of days and probably did a lot of testing, do you guys have an opinion here perhaps? (I want to try to use this momentum where a lot of people are working on this plugin to make it a bit more robust if possible 🙂)

[hostep]

@sidolov, @xmav: can we also get some attention on this one please? 🙂

[convenient]

#27 (comment)

I think this would be the least impact change without me having to go back and regression test a weird install process. It would be opt in, but at least something

[xmav]

@convenient Please change target branch to 0.4.x
Could you please also try to run tests with latest composer 2.2.x and 2.3.x:

docker run --rm -v /<project-path>/magento-composer-installer:/testInstaller magento/magento-cloud-docker-php:8.1-cli-1.3.2](http://magento-composer-installer/testInstaller%20magento/magento-cloud-docker-php:8.1-cli-1.3.2) /bin/bash -c "cd /testInstaller && php -f composer.phar self-update -- 2.2.14 && php -f vendor/bin/phpunit -c phpunit.xml.dist"


docker run --rm -v /<project-path>/magento-composer-installer:/testInstaller magento/magento-cloud-docker-php:8.1-cli-1.3.2](http://magento-composer-installer/testInstaller%20magento/magento-cloud-docker-php:8.1-cli-1.3.2) /bin/bash -c "cd /testInstaller && php -f composer.phar self-update -- 2.3.7 && php -f vendor/bin/phpunit -c phpunit.xml.dist"

[convenient]

Hey @xmav I'm not sure I can run these tests out of your ecosystem, like where do I find magento/magento2-base-mock

Prep

wget https://getcomposer.org/download/2.2.14/composer.phar
chmod +x composer.phar

Running the tests

[10:14:09] lukerodgers [~/src/magento-composer-installer]$ docker run --platform linux/amd64 --rm -v /Users/lukerodgers/src/magento-composer-installer:/testInstaller magento/magento-cloud-docker-php:8.1-cli-1.3.2 /bin/bash -c "cd /testInstaller && php -f composer.phar self-update -- 2.2.14 && php -f vendor/bin/phpunit -c phpunit.xml.dist"
You are running Composer as "root", while "/composer" is owned by "www"
You are already using the latest available Composer version 2.2.14 (stable channel).
PHP:  syntax error, unexpected '=' in /testInstaller/phpunit.xml.dist on line 1
PHPUnit 9.5.24 #StandWithUkraine

................................................FFFFF..........  63 / 127 ( 49%)
............................................................... 126 / 127 ( 99%)
.                                                               127 / 127 (100%)

Time: 03:24.315, Memory: 12.00 MB

There were 5 failures:

1) MagentoHackathon\Composer\Magento\FullStack\GlobalPluginTest::testGlobalInstall
process for <code>./composer.phar global install</code> exited with 2: Misuse of shell builtinsfrom class MagentoHackathon\Composer\Magento\FullStack\GlobalPluginTest
Error Message:
Changed current directory to /testInstaller/tests/FullStackTest/home
No composer.lock file present. Updating dependencies to latest instead of installing from lock file. See https://getcomposer.org/install for more information.
Loading composer repositories with package information
Info from https://repo.packagist.org: #StandWithUkraine
Updating dependencies
Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - Root composer.json requires magento/magento2-base-mock, it could not be found in any version, there may be a typo in the package name.

Potential causes:
 - A typo in the package name
 - The package is not available in a stable-enough version according to your minimum-stability setting
   see <https://getcomposer.org/doc/04-schema.md#minimum-stability> for more details.
 - It's a private package and you forgot to add a custom repository to find it

Read <https://getcomposer.org/doc/articles/troubleshooting.md> for further common problems.

Output:

Failed asserting that 2 matches expected 0.

/testInstaller/tests/MagentoHackathon/Composer/Magento/FullStack/AbstractTest.php:136
/testInstaller/tests/MagentoHackathon/Composer/Magento/FullStack/GlobalPluginTest.php:39

2) MagentoHackathon\Composer\Magento\FullStack\GlobalPluginTest::testGlobalUpdate
process for <code>./composer.phar global update</code> exited with 2: Misuse of shell builtinsfrom class MagentoHackathon\Composer\Magento\FullStack\GlobalPluginTest
Error Message:
Changed current directory to /testInstaller/tests/FullStackTest/home
Loading composer repositories with package information
Updating dependencies
Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - Root composer.json requires magento/magento2-base-mock, it could not be found in any version, there may be a typo in the package name.

Potential causes:
 - A typo in the package name
 - The package is not available in a stable-enough version according to your minimum-stability setting
   see <https://getcomposer.org/doc/04-schema.md#minimum-stability> for more details.
 - It's a private package and you forgot to add a custom repository to find it

Read <https://getcomposer.org/doc/articles/troubleshooting.md> for further common problems.

Output:

Failed asserting that 2 matches expected 0.

/testInstaller/tests/MagentoHackathon/Composer/Magento/FullStack/AbstractTest.php:136
/testInstaller/tests/MagentoHackathon/Composer/Magento/FullStack/GlobalPluginTest.php:51

3) MagentoHackathon\Composer\Magento\FullStackTest::testEverything with data set #0 ('symlink')
Failed asserting that file "/testInstaller/tests/FullStackTest/htdocs/app/code/Magento/ModuleMock/etc/module.xml" exists.

/testInstaller/tests/MagentoHackathon/Composer/Magento/FullStackTest.php:152

4) MagentoHackathon\Composer\Magento\FullStackTest::testEverything with data set #1 ('copy')
Failed asserting that file "/testInstaller/tests/FullStackTest/htdocs/app/code/Magento/ModuleMock/etc/module.xml" exists.

/testInstaller/tests/MagentoHackathon/Composer/Magento/FullStackTest.php:152

5) MagentoHackathon\Composer\Magento\FullStackTest::testEverything with data set #2 ('copy_force')
Failed asserting that file "/testInstaller/tests/FullStackTest/htdocs/app/code/Magento/ModuleMock/etc/module.xml" exists.

/testInstaller/tests/MagentoHackathon/Composer/Magento/FullStackTest.php:152

FAILURES!
Tests: 127, Assertions: 220, Failures: 5.

[convenient]

Hello @sidolov @xmav ? How do i even run the tests? Please see above

[convenient]

Hello @xmav @sidolov ? Can you please respond to my above query? how do i run these tests?

[ishakhsuvarov]

Hey @convenient

Looks like I am getting same error with the tests as you do with this one. I've checked in with @sidolov and right now both of us have no idea how to approach this best.

We will try to find someone who is more knowledgeable on this and get some insights.

Sorry for the delay.

[convenient]

let me know @ishakhsuvarov

I couldnt even figure out how to composer patch this into my instances due to the ordering of the plugins being run, so there is currently no workaround that I know of other than pray bugs do not occur.

[convenient]

Hey @sidolov

I see you've recently merged here 8549610

Did yous figure out how to run the tests to be able to merge into this repository?

[convenient]

Hi hope you're well @ishakhsuvarov @sidolov

Any word on how to support / develop for this plugin ? Without the ability to run tests it seems tricky.

Thanks

[lbajsarowicz]

This PR saved my life. Due to the dev/ permission issues, most of the magento2-base files were not installed. Thanks to @convenient, I found this patch and the real, the exception about the invalid permissions helped me nail down the issue.

[convenient]

Hi @ishakhsuvarov @sidolov

Any update / ideas on how to handle this error suppressing issue?

[convenient]

Hi @ishakhsuvarov @sidolov

Any update / ideas on how to handle this error suppressing issue? I am unable to run the tests as per #27 (comment)

[convenient]

Hi @ishakhsuvarov @sidolov

Any update / ideas on how to handle this error suppressing issue? I am unable to run the tests as per #27 (comment)