Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extract does not offer correct return value on Exception #14

Open
cognifloyd opened this issue May 28, 2015 · 5 comments
Open

Extract does not offer correct return value on Exception #14

cognifloyd opened this issue May 28, 2015 · 5 comments

Comments

@cognifloyd
Copy link

Using "t3xutils.phar extract", the exit code is always 0, even if there was an exception.

The exit code is determined by $success when Dispatcher exits here: https://github.com/etobi/Typo3ExtensionUtils/blob/master/lib/etobi/extensionUtils/Dispatcher.php#L91

$status is supposed to be set in extractCommand with the return value of extractAction:
https://github.com/etobi/Typo3ExtensionUtils/blob/master/lib/etobi/extensionUtils/Dispatcher.php#L216

But, extractAction does not have a return value:
https://github.com/etobi/Typo3ExtensionUtils/blob/master/lib/etobi/extensionUtils/Controller/T3xController.php#L73

Also, the exception catcher does not exit with any error codes:
https://github.com/etobi/Typo3ExtensionUtils/blob/master/bin/t3xutils.php#L14

Either the extractAction needs to somehow return an appropriate boolean, or the exception catcher needs to exit with an error code.

@etobi
Copy link
Owner

etobi commented May 29, 2015

As you already digged into this: could you provide a patch/PR?

@cognifloyd
Copy link
Author

Which solution would you prefer? Does the exit line here ever return any error codes (should the fix focus on this)?
https://github.com/etobi/Typo3ExtensionUtils/blob/master/lib/etobi/extensionUtils/Dispatcher.php#L91

Or should I focus on catching exceptions here:
https://github.com/etobi/Typo3ExtensionUtils/blob/master/bin/t3xutils.php#L14

@etobi
Copy link
Owner

etobi commented Jun 3, 2015

i guess catching the exception and returning the correct exit code there might be the "right" way. All actions throw exceptions, if they don't succeed.

However, to not confuse the next developer looking at https://github.com/etobi/Typo3ExtensionUtils/blob/master/lib/etobi/extensionUtils/Dispatcher.php#L91
this should be cleaned up, too.

@etobi
Copy link
Owner

etobi commented Jun 3, 2015

I just double checked the actions. Seems like some actions do return TRUE/FALSE (like https://github.com/etobi/Typo3ExtensionUtils/blob/master/lib/etobi/extensionUtils/Controller/TerController.php#L125).

I like to suggest to streamline the error handling (always throw an exception, instead of returning TRUE/FALSE) in all actions, implementing an "own Exception" (e.g. ActionFailedException extends \Exception) and catch that in the Dispatcher or t3xutils.php

What do you think?

@cognifloyd
Copy link
Author

That could work. It'll be awhile before I can dig into this again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants