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

setup:static-content:deploy, setup:di:compile and deploy:mode:set will not return a non-zero exit code if any error occurs #3060

Closed
moleman opened this issue Jan 21, 2016 · 7 comments
Labels
Area: Frontend improvement Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development

Comments

@moleman
Copy link

moleman commented Jan 21, 2016

When you run commands in an build environment, like Jenkins, you want the commands to return a non-zero exit code if an error occurred so that Jenkins can fail the build.

Currently if any error occurs when you run setup:static-content:deploy it will not return a non-zero exit code which makes the build succeed even though the build is incomplete.

Example:

=== frontend -> Vaimo/XXX -> en_US ===
...........................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................
Compilation from source:
frontend/Vaimo/XXX/en_US/css/email-inline.less
variable @media-common is undefined in file /XXX/en_US/css/source/_theme.less in _theme.less on line 24, column 9
22| //  Global
23| //  _____________________________________________
24| & when (@media-common = true) {
25|     .svg-link {
26|         pointer-events: none;
27|     }>

This will make the generation of static files to stop right in the middle because of an error but it will still not return a non-zero exit code which makes the build incomplete and Jenkins will not know about it.

The same behaviour occurs when you run setup:di:compile and when you switch deploy mode with deploy:mode:set. All of these commands should return a non-zero exit code if any error occurs.

@erikhansen
Copy link
Contributor

+1 for fixing this issue. I just experienced the problem with failures not returning a non-zero return code. I am using Capistrano to deploy to stage/production servers. I run bin/magento setup:static-content:deploy -q as a part of my deployment process. I ran into an issue where there was a LESS compilation error, but since the return code was 0, Capistrano continued with the deployment. This resulted in the site site being updated with a broken frontend since the CSS did not compile.

@erikhansen
Copy link
Contributor

@magento - I did some digging into this, and it looks like what needs to happen is that the execute methods (that use the Symfony Console library) need to return "1" when there is an error. This isn't documented in the Symfony documentation, but you can see an example of it on this page: http://fideloper.com/laravel-symfony-console-commands-stderr

So if you search for all instances of $output->writeln('<error>, you'll see at least most of the places that you need to update.

For example, the \Magento\Setup\Console\Command\AdminUserCreateCommand::execute method (https://github.com/magento/magento2/blob/develop/setup/src/Magento/Setup/Console/Command/AdminUserCreateCommand.php#L59) should change from this:

if ($errors) {
    $output->writeln('<error>' . implode('</error>' . PHP_EOL .  '<error>', $errors) . '</error>');
    return;
}

to this:

if ($errors) {
    $output->writeln('<error>' . implode('</error>' . PHP_EOL .  '<error>', $errors) . '</error>');
    return 1;
}

@fooman
Copy link
Contributor

fooman commented Mar 28, 2016

Seems there is already a PR for this #3189

@erikhansen
Copy link
Contributor

@fooman Yes, you are correct. Thanks for sharing. Once that PR gets merged, this ticket should be closed.

@piotrekkaminski
Copy link
Contributor

Thank you for your submission.

We recently made some changes to the way we process GitHub submissions to more quickly identify and respond to core code issues.

Feature Requests and Improvements should now be submitted to the new Magento 2 Feature Requests and Improvements forum (see details here).

We are closing this GitHub ticket and have moved your request to the new forum.

@davidalger
Copy link
Member

FTR - It appears based on the history of #3189 that this bug was fixed in 2.1.0

@magento-team
Copy link
Contributor

Internal ticket to track issue progress: MAGETWO-67500

@magento-team magento-team added Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development develop Area: Frontend improvement labels Jul 31, 2017
magento-engcom-team pushed a commit that referenced this issue Aug 21, 2018
[2.3.0-Regression] Bug Fixes PR
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Frontend improvement Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development
Projects
None yet
Development

No branches or pull requests

8 participants