When running remote commands regardless of what happens drush exits with 0. This means that errors can't be caught. Please ensure that drush remote actions return a non 0 value on error.

Here is some sample output from a jenkins job on a broken site:

drush @my-alias vset --always-set site_name test
PHP Fatal error:  Call to undefined function entityreference_get_behavior_handlers() in /path/to/sites/all/modules/contrib/entityreference/entityreference.install on line 44
Drush command terminated abnormally due to an unrecoverable error.       [error]
Error: Call to undefined function
entityreference_get_behavior_handlers() in
/path/to/sites/all/modules/contrib/entityreference/entityreference.install,
line 44
Finished: SUCCESS
Files: 

Comments

greg.1.anderson’s picture

I don't think that #1 is related to #0, is it? I agree that it would be a good idea to fix #0.

nedjo’s picture

#1 seems to be a source of the specific error reported in #0, so I added the reference in case people arrive here searching for that error. But, yes, the Drush return value is a distinct question, so this isn't a duplicate of #1459540.

greg.1.anderson’s picture

Status:Active» Needs review
StatusFileSize
new1.03 KB

Here's a patch that could be committed to master + backported to 5.x. The mid-stream 'exit' is inelegant, but calling drush_set_error here causes problems with the Drush shutdown process, whereas this runs cleanly. Unit tests pass.

Perhaps we could commit this for now to get things working, and work on cleaning up bootstrap + error recovery in a follow-on issue later.

moshe weitzman’s picture

Status:Needs review» Reviewed & tested by the community

This looks pretty inelegant. I'm OK with committing it, but lets please do that larger refactor soon.

greg.1.anderson’s picture

Status:Reviewed & tested by the community» Fixed

Committed to master and 7.x-5.x. Will work out a better resolution for Drush-6, and leave Drush-5 as it is.

greg.1.anderson’s picture

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

webchick’s picture

Status:Closed (fixed)» Active

It doesn't look like this was ever fixed for 6. Using latest bleeding-edge 8.x-6.x branch:

#!/bin/bash
drush dl token
echo $?

drush dl akhsdjkahjskdhakjsdhkjashkdad
echo $?

Yields:

Project token (7.x-1.4) downloaded to /Users/abyron/token.           [success]
0
No release history was found for the requested project               [warning]
(akhsdjkahjskdhakjsdhkjashkdad).
0
webchick’s picture

greg.1.anderson’s picture

Title:Remote commands always exit with 0» Drush pm-download exit with 0 even when no release is found

#9 is different than the closed issue, but can be tracked here. The new issue is that drush dl calls drush_log instead of drush_set_error when no release is found for a requested project.

greg.1.anderson’s picture

Version:» 8.x-6.x-dev
Component:Core Commands» PM (dl, en, up ...)
Assigned:Unassigned» greg.1.anderson

In updatexml_get_release_history_xml():

  if ($error = $xml->xpath('/error')) {
    // Don't set an error here since it stops processing during site-upgrade.
    drush_log($error[0], 'warning'); // 'DRUSH_PM_COULD_NOT_LOAD_UPDATE_FILE',
    return FALSE;
  }

So, we could fix this issue, but break pm-updatecode just by changing that drush_log to a drush_error. A more clever solution is needed. Perhaps updatexml_get_release_history_xml() could be changed to return a string in case of error, or an array of xml in case of no error. All of its clients would then need to call drush_log or drush_set_error as needed.

Assigning to myself so its not forgotten, but patches welcome if anyone else wants to pick this up.

Jon Pugh’s picture

I seem to be having this problem on all commands that produce a drush error.... I still get 0 exit code when running an unknown command, for example:

drush wtf; echo $?
The drush command 'wtf' could not be found.  Run `drush cache-clear drush` to clear the commandfile cache [error]
if you have installed new extensions.
A Drupal installation directory could not be found                                                        [error]
0
greg.1.anderson’s picture

Status:Active» Fixed

#13 did not seem to be a problem for me on the latest master.

I fixed #12 not by adjusting the API call, but by calling drush_set_error explicitly in pm-download if fetching a release failed (and the failure is not due to a user abort).

greg.1.anderson’s picture

Also pushed to 6.x.

greg.1.anderson’s picture

Status:Fixed» Closed (won't fix)
Issue tags:+needs migration

This issue was marked closed (won't fix) because Drush has moved to Github.

If desired, you may copy this bug to our Github project and then post a link here to the new issue. Please also change the status of this issue to closed (duplicate).

Please ask support questions on Drupal Answers.

greg.1.anderson’s picture

Issue summary:View changes

s/

/