Everything

php

[patch] re-invoking default error handler

php.internals

Andrei Zmievski

22 years ago

Attached is a small patch that can let you do the following. If you set
a custom error handler via set_error_handler() but don't want to
implement all the details of handling every single error level, now you
can simply handle the ones you are interested. Basically, if you return
1 from your error handler, then the PHP default error handler gets
invoked, otherwise nothing happens. So:

function my_notice_handler($type, $str, $file, $line, $context)
{
if ($type == E_NOTICE || $type == E_USER_NOTICE) {
/* do something */
return false;
} else {
/* let PHP default handler do it */
return true;
}
}

It's backwards compatible with previous functionality, i.e. if you don't
return anything, the default handler does not get invoked.

Do you think it's harmless enough to include in PHP 5?

- Andrei

Jon Parise

22 years ago

On Thu, May 27, 2004 at 08:50:48AM -0700, Andrei Zmievski wrote:

> Attached is a small patch that can let you do the following. If you set
> a custom error handler via set_error_handler() but don't want to
> implement all the details of handling every single error level, now you
> can simply handle the ones you are interested. Basically, if you return
> 1 from your error handler, then the PHP default error handler gets
> invoked, otherwise nothing happens. So:
>
>   function my_notice_handler($type, $str, $file, $line, $context)
>   {
>      if ($type == E_NOTICE || $type == E_USER_NOTICE) {
>        /* do something */
>        return false;
>      } else {
>        /* let PHP default handler do it */
>        return true;
>      }
>   }

The convention(*) for these kinds of things is generally to return
'true' when you've handled the message/event and want to discontinue
further processing.  Returning 'false' indicates that you haven't
handled the event.

(*) I don't mean "PHP covention", but this is how most other messaging
systems work when chaining handlers.

> It's backwards compatible with previous functionality, i.e. if you don't
> return anything, the default handler does not get invoked.

This argument may overrule my preference, of course.

--
Jon Parise (jon@php.net) :: The PHP Project (http://www.php.net/)

Lenar Lõhmus

22 years ago

Jon Parise wrote:

> The convention(*) for these kinds of things is generally to return
> 'true' when you've handled the message/event and want to discontinue
> further processing.  Returning 'false' indicates that you haven't
> handled the event.

You are right. That's how it should be done.

>> It's backwards compatible with previous functionality, i.e. if you don't
>> return anything, the default handler does not get invoked.
>
> This argument may overrule my preference, of course.

Since error_handler is called from inside PHP engine, there should be no
noticable BC problem. Only the case when developer calls his function
directly too and does so in if/while/etc might be problem. But I don't
think this is showstopper.

Anyway, neat feature, please add (after return value inversion).

Lenar

Andrei Zmievski

22 years ago

On Thu, 27 May 2004, Lenar L?hmus wrote:

> >> It's backwards compatible with previous functionality, i.e. if you don't
> >> return anything, the default handler does not get invoked.
> >
> > This argument may overrule my preference, of course.
>
> Since error_handler is called from inside PHP engine, there should be no
> noticable BC problem. Only the case when developer calls his function
> directly too and does so in if/while/etc might be problem. But I don't
> think this is showstopper.

Of course there is a BC problem. Current error handlers do not have to
return anything. Once the patch is added, the default handler will all
of a sudden be invoked, when the developer did not really mean it. I
realize that the "convention" is to return false if you didn't process
it, but in our case we have to invert it.

- Andrei

Sara Golemon

22 years ago

> Of course there is a BC problem. Current error handlers do not have to
> return anything. Once the patch is added, the default handler will all
> of a sudden be invoked, when the developer did not really mean it. I
> realize that the "convention" is to return false if you didn't process
> it, but in our case we have to invert it.
>

We don't necessarily *have* to.  Unless people are explicitly returning a
false value (as opposed to simply not using return) we can make the
distinction.  Recall that not returning anything is passed as a return value
of NULL.  So we could say "If NULL, don't invoke internal handler, otherwise
convert to boolean and use 'normal' logic".  i.e.  False we call internal
handler, True we don't.

This just means replacing the first line of that patch with:
if (Z_TYPE_P(retval) != IS_NULL && !zend_is_true(retval)) {

Here the only BC break potential is if someone out there has an error
callback that does explicitly return a non-null false value.  The question
"why?" comes to mind, but it's not entirely out of the question.  So let's
tighten it down even further maybe....

if (Z_TYPE_P(retval) == IS_BOOL && Z_LVAL_P(retval) == 0) {

Here, the ONLY way the internal error handler will be called is if the
script explicitly returns FALSE (not 0, not blank string, not empty array,
etc...).  But maybe that's a bit pedantic.....

-Sara

Andrei Zmievski

22 years ago

On Thu, 27 May 2004, Sara Golemon wrote:

> This just means replacing the first line of that patch with:
>       if (Z_TYPE_P(retval) != IS_NULL && !zend_is_true(retval)) {

This would be fine with me. What do we all thing about feasibility of
this patch going into PHP 5?

- Andrei

Sara Golemon

22 years ago

> > This just means replacing the first line of that patch with:
> >       if (Z_TYPE_P(retval) != IS_NULL && !zend_is_true(retval)) {
>
> This would be fine with me. What do we all thing about feasibility of
> this patch going into PHP 5?
>

+0

No great harm in doing it, but for myself if I'm going to override the error
handler, I'm going to override the entire error handler.

-Sara

Rasmus Lerdorf

22 years ago

On Thu, 27 May 2004, Sara Golemon wrote:

> > > This just means replacing the first line of that patch with:
> > >       if (Z_TYPE_P(retval) != IS_NULL && !zend_is_true(retval)) {
> >
> > This would be fine with me. What do we all thing about feasibility of
> > this patch going into PHP 5?
> >
> +0
>
> No great harm in doing it, but for myself if I'm going to override the error
> handler, I'm going to override the entire error handler.

The need that came up here was that people only wanted to use their error
handler for their own trigger_error()'ed errors.  For normal PHP errors
they still wanted to use the default handler.  This request does make some
sense, and hence Andrei's patch.

-Rasmus

Sara Golemon

22 years ago

> > +0
> >
> > No great harm in doing it, but for myself if I'm going to override the

error

> > handler, I'm going to override the entire error handler.
>
> The need that came up here was that people only wanted to use their error
> handler for their own trigger_error()'ed errors.  For normal PHP errors
> they still wanted to use the default handler.  This request does make some
> sense, and hence Andrei's patch.
>

Nah, I know.  I'm not saying I'm against it, all I'm saying is that noone
will hear any objections from me.  It puts an extra tool in the hands of the
user at negligible cost and that's a good thing.  All I meant by my comment
was that it's a tool I don't ever see myself (personally) using.

-Sara

Andrei Zmievski

22 years ago

On Thu, 27 May 2004, Sara Golemon wrote:

> Nah, I know.  I'm not saying I'm against it, all I'm saying is that noone
> will hear any objections from me.  It puts an extra tool in the hands of the
> user at negligible cost and that's a good thing.  All I meant by my comment
> was that it's a tool I don't ever see myself (personally) using.

So.. are we agreed that it won't do harm to PHP 5 at this point?

- Andrei

Bert Slagter

22 years ago

Sara Golemon wrote:

>>>This just means replacing the first line of that patch with:
>>>      if (Z_TYPE_P(retval) != IS_NULL && !zend_is_true(retval)) {
>>
>>This would be fine with me. What do we all thing about feasibility of
>>this patch going into PHP 5?
>>
>
> +0
>
> No great harm in doing it, but for myself if I'm going to override the error
> handler, I'm going to override the entire error handler.
>
> -Sara

You can't handle all errorlevels with a custom error handler, at the
moment E_STRICT (why this one by the way?), E_ERROR, E_PARSE,
E_CORE_ERROR, E_CORE_WARNING, E_COMPILE_ERROR and E_COMPILE_WARNING
cannot be handled. So I can understand that one decides to leave
E_NOTICE and E_WARNING to PHP as well and only handles E_USER_* himself.

I see a situation where it might be useful, but I won't use it myself.

And a point to be taken in consideration: it might obscure the error
handling process a bit. Right now it isn't very easy to understand (I
see new colleagues have a hard time trying to understand this monthly) :).

Bert

Derick Rethans

22 years ago

On Fri, 28 May 2004, Bert Slagter wrote:

> You can't handle all errorlevels with a custom error handler, at the
> moment E_STRICT (why this one by the way?), E_ERROR, E_PARSE,
> E_CORE_ERROR, E_CORE_WARNING, E_COMPILE_ERROR and E_COMPILE_WARNING
> cannot be handled. So I can understand that one decides to leave
> E_NOTICE and E_WARNING to PHP as well and only handles E_USER_* himself.

E_STRICT can not be handled you say, can you give a small example script
on that?

regards,
Derick

Bert Slagter

22 years ago

Derick Rethans wrote:

> On Fri, 28 May 2004, Bert Slagter wrote:
>
> E_STRICT can not be handled you say, can you give a small example script
> on that?
>
> regards,
> Derick

Of course :)

-----------
<?php
Foo::bar();
$Foo->foo = 0;

class Foo
{	var $baz;
function bar()
{
}
}

class Foo2 extends Foo
{	function bar($param1)
{
}
}
?>
-----------

This piece of sh.. erm code produces 4 E_STRICT errors:

-----------
Strict Standards: var: Deprecated. Please use the
public/private/protected modifiers in test2.php on line 6

Strict Standards: Declaration of Foo2::bar() should be compatible with
that of Foo::bar() in test2.php on line 16

Strict Standards: Non-static method Foo::bar() should not be called
statically in test2.php on line 2

Strict Standards: Creating default object from empty value in test2.php
on line 3
-----------

Now I add a custom errorhandler:

-----------
<?php

function myErrorHandler($errno, $errstr, $errfile, $errline)
{	print "<p>We've got a $errno on line $errline in $errfile!</p>";
}

set_error_handler('myErrorHandler');
error_reporting(0);

Foo::bar();
$Foo->foo = 0;

class Foo
{	var $baz;
function bar()
{
}
}

class Foo2 extends Foo
{	function bar($param1)
{
}
}
?>
-----------

The output now is:

-----------
Strict Standards: var: Deprecated. Please use the
public/private/protected modifiers in test2.php on line 14

Strict Standards: Declaration of Foo2::bar() should be compatible with
that of Foo::bar() in test2.php on line 24
-----------

As you can see, two of the E_STRICT errors are still being displayed
(this is logical, because they're cast on parse/compile time instead of
runtime). The other two though, are not displayed but not passed to my
handler either!!

If I generate a notice (for example add $bar[a] = 0;) I instantly get an
E_NOTICE in my own error handler for the undefined constant:

-----------
We've got a 8 on line 13 in test2.php!
-----------

So.. Why are the E_STRICTs not passed to my error handler?

Bert

Lenar Lõhmus

22 years ago

Sara Golemon wrote:

> We don't necessarily *have* to.  Unless people are explicitly returning a
> false value (as opposed to simply not using return) we can make the
> distinction.  Recall that not returning anything is passed as a return
> value
> of NULL.  So we could say "If NULL, don't invoke internal handler,
> otherwise
> convert to boolean and use 'normal' logic".  i.e.  False we call internal
> handler, True we don't.

Oh, you said this. Exactly what I was going to write.

Lenar