There is nothing unusual about this module. Any ideas why I would be getting this error?

drush coder-review dcycle --minor --comment --verbose --debug

...

Coder found 1 projects, 2 files, 1 minor warnings, 0 warnings were   [status]
flagged to be ignored [0.69 sec, 17.71 MB]
Drush command terminated abnormally due to an unrecoverable error.   [error]
[0.69 sec, 17.67 MB]

Cheers,

Albert.

Members fund testing for the Drupal project. Drupal Association Learn more

Comments

alberto56’s picture

The reason I need to fix this is, ideally, is because I'm using a Jenkins server which detects the "[error]" and marks the build as failed.

Another thing: I don't get this error message for all modules; I'm trying to figure out what triggers.

Thanks for any help.

alberto56’s picture

To reproduce:

- new d7 site
- install, enable coder
- drush coder-review coder --minor

alberto56’s picture

Title: "Drush command terminated abnormally" message even after everything worked » drush coder-review sometime considers the summary as an error, triggerint "Drush command terminated abnormally"
FileSize
203.24 KB

See attached image.

The problem is that coder_review sets a summary message if --minor is selected, possibly if there are errors as well, and that summary is sent to coder_review_print_drush_messages(), which in turn triggers drush error exit code:

function coder_review_print_drush_messages(array $summary) {
  // Count the number of warnings/errors.
  $sum = array_sum($summary['sums']);
  _coder_review_exit($sum ? 0 : 1);
...

Notice in the enclosed image that the summary is printed twice when --minor is selected: once which triggers an error, and once which does not. When --minor not selected, the summary is printed once.

alberto56’s picture

Title: drush coder-review sometime considers the summary as an error, triggerint "Drush command terminated abnormally" » drush coder-review sometime considers the summary as an error, triggering "Drush command terminated abnormally"

fixing typo in the title

alberto56’s picture

Status: Active » Needs review
FileSize
1.83 KB

Drush's exit code has nothing to do with the number of coding errors. I thnk coder_review_print_drush_messages() is superfluous: all it does is prints coder errors twice if there are any, and makes drush think an internal error occurred if there are what are in fact coding errors, no?

Here is a patch which fixes the "Drush command terminated abnormally" problem by removing coder_review_print_drush_messages() entirely.

Cheers,

Albert.

douggreen’s picture

Thie idea here was to give an exit code that jenkins, or git, could react to when there is an error. I haven't applied or tested the patch. But we do need to exit differently when there are valid errors. Have you tested/printed what the exit code is when there is an error and what it is when there isn't an error? from the shell you can print $?.

douggreen’s picture

"Drush's exit code has nothing to do with the number of coding errors." is true, but Coder's exit code does have something to do with the existence of errors. When git integration was added, we needed to return this in the exit code thus we needed _coder_review_exit instead of drush_exit to actually return the exit code, because as you point out, drush_exit doesn't do this.

douggreen’s picture

Status: Needs review » Needs work
Sylvain Lecoy’s picture

My build fail as well while there is just warnings... I deactivated this module as it is now unusable for our team.

douggreen’s picture

@Sylvain, "I deactivated this module as it is now unusable for our team" makes me think that you have a "team" of people who could make coder better or fix this problem. Is this unusable because of the exit code? You can't make Jenkins ignore the exit code? You can't wrap it in a shell script? Are you running drush as a plugin? A recent fix to the plugin/module issues might of solved this.

Shuairan’s picture

Had the same problem with Jenkins and Phing, after upgrading coder to 7.x-2.0.

because I only want to get the code analysis result without breaking the build I looked into the code of the related tools:
The best way I found is to change the build.xml for phing to ignore errors:

<drush command="${coder.review.command}" assume="yes"
       haltonerror="false" pipe="yes" returnProperty="xml">

The haltonerror="false" does the magic!

After changing this I thought about getting a different return code from coders drush task if there are critical issues.
But drush only returns "1" if the drush-command terminated abnormaly, without respecting exit-codes of the actual command. So this would cause patches in drush, coder and DrushTask.php for phing...
Than I drank another coffee to realize that this isn't needed at all, because Jenkins can break the build according to checkstyle-results, which is configurable ("Break if there are more than 5 criticial issues or more than 25 warning...")

---

Info: My Jenkins/Phing build file is based upon this:
https://github.com/reload/phing-drupal-template
which uses
https://github.com/kasperg/phing-drush-task
but generally if you use something similar to and any version of DrushTask.php it should work the same way!

build.xml for coder2.0:
https://github.com/shuairan/phing-drupal-template/compare/reload:master....

Sylvain Lecoy’s picture

Hi douggreen,

I didn't tested it since, but I think drush should not throw an error return code when its actually warnings. It makes the build fail while it is perfectly ok to degrade temporarily the quality, its not to fail on this.

I think Shuairan explained more in depth why this is not needed than me, so I wont repeat him :)

lpeabody’s picture

Version: 7.x-2.0-beta2 » 7.x-2.2
Issue summary: View changes
Status: Needs work » Needs review
FileSize
1.7 KB

I'm attaching my proposed fix for this issue. I just added an additional option (--ignores-pass) specifying that ignores will not count towards generating a failing shell exit code.

I'm using a pre-commit hook for a module I'm building that uses drush and coder review to ensure Drupal coding standards are in place, but unfortunately it involves CTools so I'm having to put some ignores here and there... this patch has worked very well for me.

kenorb’s picture

Status: Needs review » Needs work

-

kenorb’s picture

Status: Needs work » Needs review

I've wrongly tested patch, still needs review.

deetergp’s picture

My Issue

I am in a very similar situation to what some of the previous posters are in: I'm running Jenkins tests against all new builds and the "Drush command terminated abnormally due to an unrecoverable error" errors are flagging all my builds as "Failed", regardless of whether or not Coder is properly being told to ingore the errors. The journey of finding out how to properly ignore only to learn that the error still gets thrown, led me to this thread and the patch from Comment #: 13. I took the following steps to test the patch and found that it appears to work for my needs:

My Testing

- Run the following command against a custom module file that extends a class from Views that uses snake-cased titles:

drush coder --no-empty --minor --comment --i18n --security --sql --style --druplart --sniffer --ignorename [my module]

- Confirmed that Drush was "terminated abnormally" in spite of the 3 confirmed ignores I had set in my _.module_ file.
- `cd`ed into `~/.drush` and renamed the `coder` folder to `coder-non-git`.
- I then cloned the Coder project into my `~/.drush` folder and applied the patch from Comment #: 13.
- Finally, I `cd`ed back to my project folder and ran the following:

drush coder --no-empty --minor --comment --i18n --security --sql --style --druplart --sniffer --ignorename --ignores-pass [my module]

- With the patch applied and the extra switch, I can confirm that Coder does not return an error and Drush does not throw the "Drush command terminated abnormally due to an unrecoverable error" message.

TLDR

Patch #: 13 worked for me.

deetergp’s picture

Status: Needs review » Reviewed & tested by the community
deetergp’s picture

I figured out how to apply patch #13 at build time in Jenkins, if anyone is curious. You can use Drush to download the Coder git repo & check out the latest stable branch using the following commands:

# Download Coder 7.x-2.x
drush dl coder --package-handler=git_drupalorg
# Use wget patch to enable using the `--ignores-pass` switch with Coder
wget -O sites/all/modules/coder/drush-ignores-pass.patch http://www.drupal.org/files/issues/1974654-drush-coder-review-summary-error-13.patch --no-check-certificate  
# Go to the coder directory so we can apply the patch
cd sites/all/modules/coder  
# Apply the patch to coder
git apply drush-ignores-pass.patch

Hopefully that might help somebody.

  • klausi committed 4e6913f on 7.x-2.x authored by lpeabody
    Issue #1974654 by lpeabody: drush coder-review sometime considers the...
klausi’s picture

Version: 7.x-2.2 » 7.x-2.x-dev
Status: Reviewed & tested by the community » Fixed

Committed, thanks!

Note that you can also make use of Coder Sniffer for any build integration which has a lot more checks and less false positives, see https://www.drupal.org/node/1419988

Status: Fixed » Closed (fixed)

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