bin/magento must return non zero on exceptions

[BlackIkeEagle]

When you use bin/magento in a deployment flow it must return a non zero
exitcode when there was an error or an exception. Now the exceptions
caught and as a result the exitcode is 0. In a script we assume exit
code 0 to be all clear everything went fine.

So we must fix this here.

Signed-off-by: BlackEagle [email protected]

[BlackIkeEagle]

@vancoz I've added more things that are related to incorrect exit codes, these are open for discussion

[BlackIkeEagle]

I'll fix the failing tests probably later today

[BlackIkeEagle]

@vancoz Could we discuss here if the changes I've done are good for you. Maybe it can be taken further and instead of returning non-zero from the execute methods we could just throw exceptions instead.

[BlackIkeEagle]

Sorry for the delay, I got it sorted out now.

[vancoz]

Hi @BlackIkeEagle, thank you for contribution, and sorry for delay.
Can you please use constants for exit codes? Thank you.

[erikhansen]

@BlackIkeEagle Did you see the latest feedback from @vancoz about "Can you please use constants for exit codes"? Are you going to be able to work on that small change?

Thanks for working on this fix, as it solved PR #3060 that myself and others have wanted to see fix.

[BlackIkeEagle]

@erikhansen Sorry I have seen the comment by @vancoz but I did not yet get to it to update this PR.

[erikhansen]

@BlackIkeEagle It looks like the latest CI tests failed. I saw other tests that were failing for the same reason, so it looks like it was an issue with code from develop. Also, your branch now has conflicts with develop. Can you merge develop into your branch and fix any merge conflicts? Hopefully that will result in successful CI tests.

[BlackIkeEagle]

@erikhansen Ok I'll rebase again :)

[BlackIkeEagle]

pff is there any way for magento to use something more stable than travis please

there are almost every time issues with stuff that is rebooting and whatever

[erikhansen]

@vancoz It looks like the Travis CI build randomly failed. Can you restart the tests?

[vancoz]

Can you please use constant here?

[BlackIkeEagle]

thanks for spotting that, I completely missed it

[BlackIkeEagle]

Fingers crossed travis does not reboot anything now :)

[BlackIkeEagle]

please help me here:

integration 1 php 5.5

The system is going down for power off NOW!
FEPHP Fatal error:  Uncaught exception 'PDOException' with message 'SQLSTATE[HY000] [2002] Connection refused' in

integration 1 php 5.6

The system is going down for power off NOW!
EPHP Warning:  PDOStatement::execute(): MySQL server has gone away in

integration 2 php 7

The system is going down for power off NOW!
EPHP Fatal error:  Uncaught PDOException: SQLSTATE[HY000] [2002] Connection refused in

integration integrity php 7

The system is going down for power off NOW!
Exception: Starting Magento installation:
....
  [Exception]
  Warning: PDOStatement::execute(): MySQL server has gone away in /home/travi
  s/build/magento/magento2/vendor/magento/zendframework1/library/Zend/Db/Stat
  ement/Pdo.php on line 228

static:

I don't think the issue is caused by my changes so fixing it here would be out-of-scope unless you guys say 'just fix it here :)'

[BlackIkeEagle]

I'm really tired of this stupid failures with travis

[Vinai]

Restarted the build.

[BlackIkeEagle]

@Vinai thanks, but again there is such an issue:

The system is going down for power off NOW!

[davidalger]

I'm really tired of this stupid failures with travis

@BlackIkeEagle Does it help at all to know others (like myself) are pretty sick of it too? :) Seems to have all started when Travis started shifting their infrastructure over to GCE, and unfortunately the only option we've got is to restart the failed builds.

[BlackIkeEagle]

@davidalger I did not at all assume I was alone with my annoyance. So we can conclude if we cumulate all 'failed' builds that is basically succeeds :)

[mazhalai]

@BlackIkeEagle there are some static failures specific to your changes. Please see: https://travis-ci.org/magento/magento2/jobs/125264905#L622

[BlackIkeEagle]

@mazhalai yeah sorry I already noticed but did not yet have the time to update the PR

[BlackIkeEagle]

@mazhalai I checked again, and if you want less dependencies the code must be refactored. Its not like the new 'dependency' will make it more messy.

[BlackIkeEagle]

@mazhalai do you want me to add PHPMD ignores or something like that

[mazhalai]

We do now allow any new suppress warning, I believe there is a task/story in backlog to remove all existing ones. Yes this will need to be refactored.

[piotrekkaminski]

Internal issue id: MAGETWO-52458, reported as well by @LeeSaferite

[mazhalai]

@BlackIkeEagle this PR has been merged. It was combined with other logging related issues, hence PR was not automatically closed. Thank you very much for your contribution!

[BlackIkeEagle]

@mazhalai superb, thanks