set_error_handler and set_exception_handler behavior View as Markdown

Hi internals!

Laruence and myself yesterday tried to slightly modify the
set_error_handler behavior, but the change which we considered trivial
was not perceived as such by others. So we are taking this to the ML.

Current state of things:
 * set_error_handler(callback) and set_exception_handler(callback)
will return the previous error/exception handler
 * set_exception_handler(NULL) will reset the exception handler (i.e.
use the default PHP exception handler again) and return bool(true)
 * set_error_handler(NULL) throws an invalid argument error => inconsistent

What we wanted to do:
Allow the error handler being reset using set_error_handler(NULL). To
be consistent with set_exception_handler(NULL) this was supposed to
return bool(true).

What people objected to:
 * Stas: The bool(true) return value does not really make sense.
Instead the previous error/exception handler should be returned, as
always.
 * Pierre: One shouldn't be able to reset the error/exception handler
like that in any case. This behavior should be removed and instead
there should be additional reset_error_handler() and
reset_exception_handler() functions (and also get_error_handler() and
get_exception_handler() if I got that right.)

What I would do:
 Add support for set_error_handler(NULL) and change the return value
of set_error_handler(NULL)+set_exception_handler(NULL) to the previous
handler (i.e. Stas option). I implemented this option in this PR:
https://github.com/php/php-src/pull/20

So, what are your opinions on this?
I was plan to explain why, but you described very well :)

thanks

Nikita

--

--
Laruence  Xinchen Hui
http://www.laruence.com/

Hi!

What people objected to:

• Stas: The bool(true) return value does not really make sense.
  Instead the previous error/exception handler should be returned, as
  always.
• Pierre: One shouldn't be able to reset the error/exception handler
  like that in any case. This behavior should be removed and instead
  there should be additional reset_error_handler() and
  reset_exception_handler() functions (and also get_error_handler() and
  get_exception_handler() if I got that right.)

I don't think ability to reset handlers by passing null is a big
problem, and creating another four functions seems excessive to me.
Also, the use case for the latter ones is not clear - why would you need
old error handler if you are not changing it? If you are changing it,
the case for getting the old one is clear - you may want to restore it
later. But if you're not, why you'd need it?

As for returning true on null, I see no sense if this behavior. Can
anybody explain to me what is it useful for? Why not return old handler
just as it is done in all other cases of setting the handler?

What I would do:
Add support for set_error_handler(NULL) and change the return value
of set_error_handler(NULL)+set_exception_handler(NULL) to the previous
handler (i.e. Stas option). I implemented this option in this PR:
https://github.com/php/php-src/pull/20

I think this makes sense (of course, since it's my proposal :). It is a
behavior change, so it should be confined to master branch.

--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227