-
Notifications
You must be signed in to change notification settings - Fork 7.8k
add get_error_handler() and get_exception_handler() #969
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
Conversation
/* {{{ proto string get_error_handler(void) | ||
Returns the currently defined error handler, or null */ | ||
ZEND_FUNCTION(get_error_handler) | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here and in get_exception_handler(), you should use zend_parse_parameters_none(), else the changes look great!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hehe, nice catch, was copying most of the code from restore_error_handler() and surprisingly zend_parse_parameters_none() is not used there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Landing in 3, 2, 1, ... http://git.php.net/?p=php-src.git;a=commitdiff;h=607b7d662a854443713e8a9c651c888527718618
@Tyrael 👍 |
I'd propose setting the return_value of restore_error/exception_handler to the removed handler. That already should cover most use cases… and if that's not what you need, restore and set. |
what would restore_error/exception_handler() return when there is no user handler set? |
Actually, I think we relatively safely can assume the return value of restore_error/exception_handler() isn't used much currently. And yes, NULL would be a sensible return value then. |
@Tyrael can we know the current status of this PR please ? |
Having waited a month for feedback, and since there seems to be some unresolved questions, I'm closing this PR. Please take this action as encouragement to open a clean PR against a supported branch. |
@krakjoe was mostly just waiting for other's to join the discussion, gather more feedback, but maybe the lack of responses mean that this is not something which many people needs. |
I would use this 👍 +1 from me. Right now I have found some 3rd party library has changed my exception handler and I have no idea which one, and querying for the handler would expose the culprit library! Can at the very least, a workaround be published in the |
Sometimes it is useful to get the current error/exception handler without replacing it (which is already possible with set__handler and restore__handler albeit a a bit more work).
Merging this PR would also resolve https://bugs.php.net/bug.php?id=54033