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

Enhanced exception handling for BeanExceptions in AnnotationBeanFactory #31

Merged

Conversation

dropdevcoding
Copy link
Contributor

I recently ran into a problem concerning the exception handling while instanciating a bean.

If you write or use a class, which throws an exception during the constructor call, I get the following when trying to instanciate the bean by calling $beanFactory->get('myClass') for a CLI application:

PHP Fatal error:  Uncaught exception 'bitExpert\Disco\BeanException' with message 'Holy sh*t!' in /var/www/acme/vendor/bitexpert/disco/src/bitExpert/Disco/AnnotationBeanFactory.php:70
Stack trace:
#0 /var/www/acme/web/index.php(26): bitExpert\Disco\AnnotationBeanFactory->get('myClass')
#1 {main}
  thrown in /var/www/acme/vendor/bitexpert/disco/src/bitExpert/Disco/AnnotationBeanFactory.php on line 70

what hides any information about the original exception and makes it difficult to debug.

In my case the error message was "Make sure you are passing in the correct parameters" which also could be an error of my bean configuration as well (wrong reference inside a @parameter annotation).

If the original exception is passed as previous for the BeanException, we will see the correct stack trace (at least when using XDebug).

@shochdoerfer
Copy link
Member

To discuss: Maybe we should pass the original exception to the BeanException and not just the exception message? Because now we loose the filename and line number where the original exception occurred. In some cases this could make the debugging quite complicated, I assume. What do you think?

@dropdevcoding
Copy link
Contributor Author

I recently tested your suggestion and I'm struggling right now between having the exception and its trace as message by using:

throw new BeanException($e)

which gives a nice representation of the previous exception probably even without using XDebug

PHP Fatal error:  Uncaught exception 'bitExpert\Disco\BeanException' with message 'exception 'Exception' with message 'Holy sh*t' in /var/www/acme/src/Acme/MyClass.php:25
Stack trace:
#0 /var/www/acme/src/Acme/Config.php(205): Acme\MyClass->__construct(Array, Array)
#1 /tmp/acme/di/ProxyManagerGeneratedProxy__PM__AcmeConfigGenerated453471822132c786ea6e8ab9d9ab9b3a.php(363): Acme\Config->myClass(Array)
#2 [internal function]: ProxyManagerGeneratedProxy\__PM__\Acme\Config\Generated453471822132c786ea6e8ab9d9ab9b3a->myClass()
#3 /var/www/acme/vendor/bitexpert/disco/src/bitExpert/Disco/AnnotationBeanFactory.php(68): call_user_func(Array)
#4 /var/www/acme/web/index.php(18): bitExpert\Disco\AnnotationBeanFactory->get('myClass')
#5 {main}' in /var/www/acme/vendor/bitexpert/disco/src/bitExpert/Disco/AnnotationBeanFactory.php:76
Stack trace:
#0 /var/www/acme/web/index.php(18): bitExpert\Disco\AnnotationBeanFactory->get('myClass')
#1 {main}
 in /var/www/acme/vendor/bitexpert/disco/src/bitExpert/Disco/AnnotationBeanFactory.php on line 76

and

throw new BeanException($message, null, $e)

which is imho the technically correct approach but gives you only a nice trace if using XDebug:

PHP Fatal error:  Uncaught exception 'Exception' with message 'Holy sh*t' in /var/www/acme/src/Acme/MyClass.php:25
Stack trace:
#0 /var/www/acme/src/Acme/Config.php(205): Acme\MyClass->__construct(Array, Array)
#1 /tmp/acme/di/ProxyManagerGeneratedProxy__PM__AcmeConfigGenerated453471822132c786ea6e8ab9d9ab9b3a.php(363): Acme\Config->myClass(Array)
#2 [internal function]: ProxyManagerGeneratedProxy\__PM__\Acme\Config\Generated453471822132c786ea6e8ab9d9ab9b3a->myClass()
#3 /var/www/acme/vendor/bitexpert/disco/src/bitExpert/Disco/AnnotationBeanFactory.php(68): call_user_func(Array)
#4 /var/www/acme/web/index.php(18): bitExpert\Disco\AnnotationBeanFactory->get('myClass')
#5 {main}

Next exception 'bitExpert\Disco\BeanException' with message 'Exception occured while instanciating bean "myClass"' in /var/www/acme/vendor/bitexpert/disco/src/bitExpert/Disco/AnnotationBeanFactory.php:75
Stack trace:
#0 /var/www/acme/web/index.php(18): bitEx in /var/www/acme/vendor/bitexpert/disco/src/bitExpert/Disco/AnnotationBeanFactory.php on line 75

When using the first approach we would lose the possibility to traverse the exception stack via $e->getPrevious(), which may cause other problems when using logging libraries etc. and that is why I prefer the second approach.

@shochdoerfer
Copy link
Member

Sounds good to me. Thanks. Can you make the needed change to the PR please?

@dropdevcoding
Copy link
Contributor Author

The second approach has already been implemented in this pull request to achieve the goal described above.

shochdoerfer added a commit that referenced this pull request Feb 21, 2016
…nfactory_exception

Enhanced exception handling for BeanExceptions in AnnotationBeanFactory
@shochdoerfer shochdoerfer merged commit ca24019 into master Feb 21, 2016
@shochdoerfer shochdoerfer deleted the bitExpert/feature/annotationbeanfactory_exception branch February 21, 2016 09:25
@shochdoerfer shochdoerfer added this to the 0.3.0 milestone Mar 5, 2016
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

Successfully merging this pull request may close these issues.

2 participants