search

chrisboulton / php-resque

PHP port of resque (Workers and Queueing)
MIT License
3.43k stars 759 forks source link

Include message and backtrace from previous exceptions. #194

Closed StanAngeloff closed 10 years ago

StanAngeloff commented 10 years ago

If an exception wraps another, it is impossible to tell what the cause was looking at the outer exception only. If this pull request is merged, when a job fails the full backtrace of all previous exceptions is recorded.

Before:

Cannot create User.
#0 file.php(1): createUser

After:

Cannot create User.
#0 file.php(1): createUser

Caused by: Cannot connect to database.
#0 db.php(1): connect
danhunsaker commented 10 years ago

Or just use the getPrevious() method instead of only printing the current exception in your own code. This is how exceptions are designed to work throughout PHP, and not all Resque exceptions are caused by previous exceptions anyway, even when such exceptions exist. Including the previous exception's message in the message of the current one obfuscates that there is a previous exception that can be checked, and each step back in the chain has a great deal of useful information that is not transferred upward.

In other words, this seems like it might be useful to implement in your own exception handlers, but should not be implemented in the Exception child class itself.

StanAngeloff commented 10 years ago

While your points are valid, they do not take into account first-time experience. There are many cases where your application won't take action based on third-party exceptions. Whether or not the exceptions have been wrapped is out of your control. If an exception occurs in a job, it is my view that php-resque should provide the most accurate and detailed breakdown possible. log4j, for example, always includes the full backtrace. In the context of modern dependency injection-powered applications, exceptions can be caused and wrapped in many different contexts.

Exceptions are designed to be wrappable, e.g., this is why you have $previous in the Exception constructor. Yes, you can log it, but at that point you probably have the infrastructure to create your own messaging and scheduling queue as well... which obsoletes Resque altogether.

StanAngeloff commented 9 years ago

Revisiting this old PR, I wanted to clarify the original exception is not lost or overridden rather it is enhanced with any previous/nested exceptions.

If anyone is interested, the patch continues to apply cleanly on top of master.