DT-669: Deploy command continues despite composer failure #3796

josephdpurcell · 2019-08-05T15:34:12Z

Describe the bug
The blt artifact:deploy command will deploy artifacts even if a step in the build process fails. The specific example used is a patch that fails to apply to a contrib module, while fails the composer step, which fails to generate the vendor/autoload.php, which causes the deployed artifact to crash the environment with an error similar to this:

Fatal error: Uncaught Exception: Could not locate autoload.php. cwd is /mnt/www/html/myacsf.01dev/docroot; __DIR__ is /mnt/www/html/myacsf01dev/vendor/drush/drush in /mnt/www/html/myacsf01dev/vendor/drush/drush/drush.php:54

To Reproduce

Steps to reproduce the behavior, ideally starting from a fresh install of BLT:

Run composer create-project acquia/blt-project --no-interaction
Add a contrib module, here we'll use this module as an example: composer require drupal/svg_image_field

Add a patch for that file that we know doesn't apply in the composer.json:

"patches": {
    "drupal/svg_image_field": {
      "This doesn't apply": "https://www.drupal.org/files/issues/entity-browser-view-context-2865928-14.patch"
    },

Commit the changes
Run blt artifact:deploy
Notice that it succeeds in pushing commits to Acquia. Specifics to observe in the output are:
a. Error that the patch does not apply:
[Exception] Cannot apply patch This doesn't apply (https://www.drupal.org/files/issues/entity-browser-view-context-2865928-14.patch)!
b. The ExecStack attempts to exit the script:
[ExecStack] Exit code 1 Time 16.434s
c. The script continues:
Sanitizing artifact...

Expected behavior
At step 6 in the steps above the expected behavior is that if the patch does not apply the entire artifact:deploy script fails and exits. It should NOT push commits to Acquia.

Detailed error output

Commit message is set to Automated commit by Travis CI for Build .
Branch is set to develop-build.
Preparing artifact directory...
(lost of output)
  - Applying patches for drupal/svg_image_field
    https://www.drupal.org/files/issues/entity-browser-view-context-2865928-14.patch (This doesn't apply)
   Could not apply patch! Skipping. The error was: Cannot apply patch https://www.drupal.org/files/issues/entity-browser-view-context-2865928-14.patch


  [Exception]
  Cannot apply patch This doesn't apply (https://www.drupal.org/files/issues/entity-browser-view-context-2865928-14.patch)!


install [--prefer-source] [--prefer-dist] [--dry-run] [--dev] [--no-dev] [--no-custom-installers] [--no-autoloader] [--no-scripts] [--no-progress] [--no-suggest] [-v|vv|vvv|--verbose] [-o|--optimize-autoloader] [-a|--classmap-            authoritative] [--apcu-autoloader] [--ignore-platform-reqs] [--] [<packages>]...

[ExecStack]  Exit code 1  Time 16.434s
Sanitizing artifact...
[info] Find Drupal core text files...
(more BLT output)
[ExecStack] Done in 5.343s
Pushing artifact to git.remotes...
[ExecStack] git push 470bf68fce4b28e6f615e854987d6dc3 develop-build
[ExecStack] Running git push 470bf68fce4b28e6f615e854987d6dc3 develop-build in /tmp/deploy
Counting objects: 117, done.
Delta compression using up to 2 threads.
Compressing objects: 100% (104/104), done.
Writing objects: 100% (117/117), 56.54 KiB | 0 bytes/s, done.
Total 117 (delta 81), reused 0 (delta 0)
To [email protected]:myacsf.git
   11d586f..f8f9bc2  develop-build -> develop-build
[ExecStack] Done in 2.455s
01:35 total time elapsed.

set +v

BLT doctor output

+---------------------------+--------------------------------------------------------------+
| Property                  | Value                                                        |
+---------------------------+--------------------------------------------------------------+
| %paths.%root              | /var/www/myacsf/docroot                                      |
| %paths.%site              | sites/default                                                |
| %paths.%modules           | sites/all/modules                                            |
| %paths.%themes            | sites/all/themes                                             |
| %paths.%config-sync       | ../docroot/profiles/custom/sfprofile/config/sync             |
| %paths.%config-vcs        | ../docroot/profiles/custom/sfprofile/config/sync             |
| %paths.%files             | sites/g/files/kebobs123/files                                |
| %paths.%temp              | /tmp                                                         |
| %paths.%private           | /var/www/myacsf/files-private/kebobs123                      |
| admin-theme               | seven                                                        |
| alias-searchpaths.0       | /var/www/myacsf/drush/sites                                  |
| blt-version               | 9.2.8                                                        |
| bootstrap                 | Successful                                                   |
| composer-version          | Composer version 1.8.5 2019-04-09 17:46:47                   |
| config-sync               | ../docroot/profiles/custom/sfprofile/config/sync             |
| db-driver                 | mysql                                                        |
| db-hostname               | 127.0.0.1                                                    |
| db-name                   | myacsf                                                       |
| db-password               | myacsf                                                       |
| db-port                   | 3306                                                         |
| db-status                 | Connected                                                    |
| db-username               | myacsf                                                       |
| drupal-settings-file      | sites/default/settings.php                                   |
| drupal-version            | 8.7.2                                                        |
| drush-alias-files.0       | /var/www/myacsf/drush/sites/mysite.site.yml                  |
|                           | yml                                                          |
| drush-cache-directory     | /home/vagrant/.drush/cache                                   |
| drush-conf.0              | /var/www/myacsf/vendor/drush/drush/drush.yml                 |
| drush-conf.1              | /var/www/myacsf/drush/drush.yml                              |
| drush-conf.2              | /var/www/myacsf/docroot/sites/default/local.drush.yml        |
| drush-script              | /var/www/myacsf/vendor/bin/drush                             |
| drush-temp                | /tmp                                                         |
| drush-version             | 9.5.2                                                        |
| files                     | sites/g/files/kebobs123/files                                |
| install-profile           | sfprofile                                                    |
| modules                   | sites/all/modules                                            |
| php-bin                   | /usr/bin/php7.1                                              |
| php-conf.0                | /etc/php/7.1/cli/php.ini                                     |
| php-os                    | Linux                                                        |
| private                   | /var/www/myacsf/files-private/kebobs123                      |
| root                      | /var/www/myacsf/docroot                                      |
| site                      | sites/default                                                |
| stacks.drupal-vm.inited   | true                                                         |
| stacks.dev-desktop.inited | false                                                        |
| temp                      | /tmp                                                         |
| theme                     | bartik                                                       |
| themes                    | sites/all/themes                                             |
| uri                       | http://myacsf.local                                          |
+---------------------------+--------------------------------------------------------------+
+--------------------------------------+-----------------------------------------------------------+
| Check                                | Problem                                                   |
+--------------------------------------+-----------------------------------------------------------+
| NodeCheck:checkNodeVersionFileExists | Neither .nvmrc nor .node-version file found in repo root. |
+--------------------------------------+-----------------------------------------------------------+

System information

Operating system type: Debian
Operating system version: Ubuntu 16.04.5 LTS
BLT version: 9.2.8

Additional context
In practice, this appears to happen composer requests a patch from drupal.org and gets a non-200 or corrupted response which results in composer not being able to apply the patch. It seems like it would be rare, but it has happened twice in the last week:

Fri Aug 2 14:00:45 2019 +0000
Sat Aug 3 01:29:46 2019 +0000

The text was updated successfully, but these errors were encountered:

josephdpurcell · 2019-08-05T15:52:17Z

This might be related to #1807? Though, there's not much info on that ticket.

The part in the code that seems suspicious to me is here: https://github.com/acquia/blt/blob/10.x/src/Robo/Commands/Deploy/DeployCommand.php#L421

    $this->buildCopy();
    $this->composerInstall();
    $this->sanitize();

I would assume that something should be done with the returned result on each task? But, digging through Robo and BLT I see there are some exceptions thrown/caught, so I might be looking at this wrong.

danepowell · 2019-08-06T18:52:18Z

Thanks for the detailed testing steps and report, it makes this much easier to reproduce. I can confirm this is a bug and will add it to the backlog.

It looks like to fix it in just this spot, we should check the return code Composer (which is 1 here) and throw an exception. But more generally, it seems like we'd benefit from having all tasks throw an exception on failure rather than relying on the caller to check the return code.

josephdpurcell · 2019-08-08T18:35:40Z

@danepowell Are you able to help me set expectations on if and when a resolution will be made for this issue? Given the frequency of this issue and the severity of it (site outage requiring rollback), it is critical for me and the projects my team is working on. So, the reason I ask is because if this is lower priority I assume a fix won't be available soon and we'll need to work towards a solution.

In the event this is lower priority or no one is available to work on it, I am interested and willing to collaborate on remediation of this issue. But, to increase the likelihood the work gets committed back, I do ask for some guidance on what the changes should look like.

ayalon · 2019-08-12T14:00:52Z

We just destroyed an enviroment with exact the same issue. This is super critical. Commiting and deploying an artifact without an autoloader kills every environment.
In fact it can happen any time if you have patches references from drupal.org via https. As soon as drupal.org is not reachable, you will deploy a broken page.

danepowell · 2019-08-12T21:39:52Z

I certainly understand the frustration, but I don't have any information on priority or timelines, sorry. The best ways to support this issue would be to do one or more of the following:

Submit Product Feedback via your technical account manager or a support ticket
+1 this issue
Submit a pull request, which has priority in terms of reviews

matteodem · 2019-08-13T09:49:29Z

Here's a related issue that would solve the problem in the robo task runner library consolidation/robo#891.

josephdpurcell · 2019-08-13T15:47:55Z

We will be working on this today. If anyone is interested in collaborating on this, please reach out to @josephdpurcell in the #blt Drupal slack.

matteodem · 2019-08-13T15:54:32Z

Maybe my implementation is of interest. Anyways looking forward to having this in blt 🎉

danepowell · 2019-08-13T16:13:03Z

As guidance for implementation: if you just want to follow the pattern established elsewhere of checking if the task exited successfully, that's totally fine and should only take a few minutes to implement. Something like this:

blt/src/Robo/Commands/Deploy/DeployCommand.php

Lines 153 to 155 in 219da38

    
           if (!$options['ignore-dirty'] && !$result->wasSuccessful()) { 
        
             throw new BltException("Unable to determine if local git repository is dirty."); 
        
           }

Longer term failed tasks should probably throw an exception automatically but that's a much bigger lift / refactor.

josephdpurcell · 2019-08-13T20:37:20Z

@danepowell I've reviewed #3809 and it matches your suggestion. I spoke with @nhmbounteous and he was able to test / verify it locally.

Should I create a follow up ticket to address other areas where this type of issue would occur?

danepowell · 2019-08-13T20:41:30Z

I've already created such a ticket: #3811

Feel free to add anything missing to that. Thanks again for the contributions.

* Made the deploy command fail and stop on `composer install` exception. * Made the deploy command fail and stop on `composer install` exception. * Update DeployCommand.php Added space in between if statement for code standards * Update DeployCommand.php Updated composer error message

josephdpurcell · 2019-08-14T18:31:46Z

Thank you @danepowell for helping work towards a solution here!

josephdpurcell · 2019-08-14T18:32:46Z

Just a reflection to share: while this didn't fix all cases, I suspect it solved the most prone-to-error case due to the likelihood of network related errors on composer install.

Thanks again!

* Made the deploy command fail and stop on `composer install` exception. * Made the deploy command fail and stop on `composer install` exception. * Update DeployCommand.php Added space in between if statement for code standards * Update DeployCommand.php Updated composer error message

josephdpurcell added the Bug Something isn't working label Aug 5, 2019

danepowell added this to the Backlog milestone Aug 6, 2019

danepowell changed the title ~~Deploy command fails silently with reproducible steps~~ Deploy command continues despite composer failure Aug 6, 2019

matteodem added a commit to matteodem/blt that referenced this issue Aug 13, 2019

Abort deploy command when command fails to run (acquia#3796)

b143c75

nhmbounteous mentioned this issue Aug 13, 2019

Fixes #3796: Deploy command continues despite composer failure #3809

Merged

danepowell changed the title ~~Deploy command continues despite composer failure~~ DT-669: Deploy command continues despite composer failure Aug 13, 2019

danepowell mentioned this issue Aug 13, 2019

DT-682: Failing commands should throw an exception #3811

Closed

danepowell closed this as completed in #3809 Aug 14, 2019

nhmbounteous mentioned this issue Aug 14, 2019

9.x Fixes #3796: Deploy command continues despite composer failure #3813

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DT-669: Deploy command continues despite composer failure #3796

DT-669: Deploy command continues despite composer failure #3796

josephdpurcell commented Aug 5, 2019 •

edited

Loading

josephdpurcell commented Aug 5, 2019

danepowell commented Aug 6, 2019

josephdpurcell commented Aug 8, 2019

ayalon commented Aug 12, 2019

danepowell commented Aug 12, 2019

matteodem commented Aug 13, 2019

josephdpurcell commented Aug 13, 2019

matteodem commented Aug 13, 2019

danepowell commented Aug 13, 2019

josephdpurcell commented Aug 13, 2019

danepowell commented Aug 13, 2019

josephdpurcell commented Aug 14, 2019

josephdpurcell commented Aug 14, 2019

DT-669: Deploy command continues despite composer failure #3796

DT-669: Deploy command continues despite composer failure #3796

Comments

josephdpurcell commented Aug 5, 2019 • edited Loading

josephdpurcell commented Aug 5, 2019

danepowell commented Aug 6, 2019

josephdpurcell commented Aug 8, 2019

ayalon commented Aug 12, 2019

danepowell commented Aug 12, 2019

matteodem commented Aug 13, 2019

josephdpurcell commented Aug 13, 2019

matteodem commented Aug 13, 2019

danepowell commented Aug 13, 2019

josephdpurcell commented Aug 13, 2019

danepowell commented Aug 13, 2019

josephdpurcell commented Aug 14, 2019

josephdpurcell commented Aug 14, 2019

josephdpurcell commented Aug 5, 2019 •

edited

Loading