Skip to content

[Console] Show code when an exception is thrown #17504

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

Closed
wants to merge 4 commits into from

Conversation

maidmaid
Copy link
Contributor

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -
License MIT
Doc PR -

Hi !

When an exception is thrown, sf console doesn't show the code exception, while this code is very useful during developpement (for example, guzzle put http status code in his code exception).

I propose you that we show the code exception only when the code exception > 0 and with -v verbosity. It means :

  • throw new Exception('Not found', 0) with normal verbosity --> no change
  • throw new Exception('Not found', 0) with -v verbosity --> no change
  • throw new Exception('Not found', 404) with normal verbosity --> no change
  • but throw new Exception('Not found', 404) with -v verbosity --> showing :

image

$title = $output->isVerbose() && ($code = $e->getCode()) > 0
? sprintf(' [%s - code %s] ', get_class($e), $code)
: sprintf(' [%s] ', get_class($e))
;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I propose to change 2 things here:

  • Use 0 !== $code instead of $code > 0, just anything that's not the default value should be printed imo

  • Use just one sprintf call:

    $title = sprintf('  [%s%s]', get_class($e), $output->isVerbose() && ($code = $e->getCode()) ? ' - code '.$code : '')

@wouterj
Copy link
Member

wouterj commented Jan 23, 2016

I like the idea. Maybe the format should be changed to[Exception (404)], especially considering the long exception FQC names out there. However, I'm not sure if it's very clear that the number is the exception code in such case.

@maidmaid
Copy link
Contributor Author

Thank you for your feedback @wouterj, I did changes.
About long exception FQC names, maybe it's better for readability to show code in new line, like :

  [Exception]
  Code 404
  Not Found

No ?

$title = sprintf(
' [%s%s] ',
get_class($e),
$output->isVerbose() && ($code = $e->getCode()) !== 0 ? ' ('.$code.')' : ''
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This condition should be in yoda style:
0 !== ($code = $e->getCode())

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right @paradajozsef, it's done.

@marek-pietrzak-tg
Copy link
Contributor

@maidmaid good idea 👍 Could you also add a test for that please? ref: https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Console/Tests/ApplicationTest.php

@maidmaid
Copy link
Contributor Author

Thank you @mheki, I done tests but I don't understand why it fail...

@stof
Copy link
Member

stof commented Jan 26, 2016

@maidmaid not related to your changes. We have a bug in our testsuite.

@maidmaid
Copy link
Contributor Author

@stof thank you for this information. Maybe list the errors can help you a little bit :

FrameworkBundle:

Fatal error: Call to undefined method Symfony\Component\DependencyInjection\Definition::getParent() in /home/travis/build/symfony/symfony/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php on line 535
PHP Fatal error:  Call to undefined method Symfony\Component\DependencyInjection\Definition::getParent() in /home/travis/build/symfony/symfony/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php on line 535

Form:

Fatal error: Call to a member function validate() on null in /home/travis/build/symfony/symfony/src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php on line 55
PHP Fatal error:  Call to a member function validate() on null in /home/travis/build/symfony/symfony/src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php on line 55

Only on Travis PHP 5.6.

@fabpot
Copy link
Member

fabpot commented Jan 27, 2016

Thank you @maidmaid.

@fabpot fabpot closed this Jan 27, 2016
fabpot added a commit that referenced this pull request Jan 27, 2016
…aid)

This PR was squashed before being merged into the 3.1-dev branch (closes #17504).

Discussion
----------

[Console] Show code when an exception is thrown

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

Hi !

When an exception is thrown, sf console doesn't show the code exception, while this code is very useful during developpement (for example, [guzzle put http status code in his code exception](https://github.com/guzzle/guzzle/blob/6.1.1/src/Exception/RequestException.php#L30-L33)).

I propose you that we show the code exception only when the code exception > 0 and with ``-v`` verbosity. It means :

- ``throw new Exception('Not found', 0)`` with normal verbosity --> no change
- ``throw new Exception('Not found', 0)`` with ``-v`` verbosity --> no change
- ``throw new Exception('Not found', 404)`` with normal verbosity --> no change
- **but** ``throw new Exception('Not found', 404)`` with ``-v`` verbosity --> showing :

![image](https://cloud.githubusercontent.com/assets/4578773/12530638/052991f2-c1e5-11e5-92f1-b7b3f60cc4ba.png)

Commits
-------

f8cd9e8 [Console] Show code when an exception is thrown
@fabpot fabpot mentioned this pull request May 13, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants