search

codeclimate / codeclimate-phpmd

Code Climate PHPMD Engine
MIT License
10 stars 12 forks source link

Exit with code 1 when child process exits #38

Closed chrishulton closed 7 years ago

chrishulton commented 7 years ago

When looking into some issues with this engine, one thing that seemed strange was that fatal errors were occurring, but the CLI was reporting "0 issues found" rather than exiting with an error, and the errors were only visible in debug mode.

[DEBUG] phpmd:stable engine stderr: PHP Fatal error:  Uncaught TypeError: Argument 1 passed to PHPMD\PHPMD::setFileExtensions() must be of the type array, null given, called in /usr/src/app/Runner.php on
line 101 and defined in /usr/src/app/vendor/phpmd/phpmd/src/main/php/PHPMD/PHPMD.php:136
Stack trace:
#0 /usr/src/app/Runner.php(101): PHPMD\PHPMD->setFileExtensions(NULL)
...
  thrown in /usr/src/app/vendor/phpmd/phpmd/src/main/php/PHPMD/PHPMD.php on line 136

Analysis complete! Found 0 issues.

I went to do some research on the issue, and saw that we had made a similar fix for this in a different php engine: https://github.com/barracudanetworks/forkdaemon-php/issues/26 https://github.com/codeclimate/codeclimate-phpcodesniffer/pull/38

We made most of the fix in https://github.com/codeclimate/codeclimate-phpmd/pull/21, but we need to add the if (count($results) == 0) check, because the logic that happens inside the loop to check the exception will not run if the length is 0.

Can verify this change leads to the expected behavior.

Starting analysis
Running phpmd: Done!
error: (CC::Analyzer::Engine::EngineFailure) engine phpmd:stable failed with status 1 and stderr
PHP Fatal error:  Uncaught TypeError: Argument 1 passed to PHPMD\PHPMD::setFileExtensions() must be of the type array, null given, called in /usr/src/app/Runner.php on line 101 and defined in /usr/src/app
/vendor/phpmd/phpmd/src/main/php/PHPMD/PHPMD.php:136
Stack trace:
#0 /usr/src/app/Runner.php(101): PHPMD\PHPMD->setFileExtensions(NULL)
#1 /usr/src/app/vendor/barracudanetworks/forkdaemon-php/fork_daemon.php(1852): CodeClimate\PHPMD\Runner->run(Array)
#2 /usr/src/app/vendor/barracudanetworks/forkdaemon-php/fork_daemon.php(1772): fork_daemon->invoke_callback(Array, Array, false)
#3 /usr/src/app/vendor/barracudanetworks/forkdaemon-php/fork_daemon.php(1673): fork_daemon->fork_work_unit(Array, '', -1)
#4 /usr/src/app/vendor/barracudanetworks/forkdaemon-php/fork_daemon.php(1445): fork_daemon->process_work_unit(-1)
#5 /usr/src/app/Runner.php(31): fork_daemon->process_work(false)
#6 /usr/src/app/engine.php(35): CodeClimate\PHPMD\Runner->queueDirectory('/code')
#7 {main}
  thrown in /usr/src/app/vendor/phpmd/phpmd/src/main/php/PHPMD/PHPMD.php on line 136
chrishulton commented 7 years ago

Hm, this has since changed around in the other engine. Need to find a way to support the case where the results are empty. Maybe the bubbling up just isn't working.

chrishulton commented 7 years ago

Think I figured out what's going on here. Going to open a different PR.