[patch] re-invoking default error handler

21 years ago by Andrei Zmievski — view source

unread

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

21 years ago by Jon Parise — view source

unread

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/)

21 years ago by lenar@city.ee — view source

unread

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

21 years ago by Sara Golemon — view source

unread

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

21 years ago by Andrei Zmievski — view source

unread

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

21 years ago by Sara Golemon — view source

unread

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?

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

21 years ago by Rasmus Lerdorf — view source

unread

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

21 years ago by Sara Golemon — view source

unread

+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

21 years ago by Andrei Zmievski — view source

unread

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

21 years ago by Sara Golemon — view source

unread

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?

certainly doesn't look like it would, but we've got a good month before
branch so there's time to worry about that before committing's a
possibility....

21 years ago by Derick Rethans — view source

unread

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?

I don't mind, but it's up to the release master IMO.

regards,
Derick

21 years ago by Bert Slagter — view source

unread

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

21 years ago by Derick Rethans — view source

unread

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

21 years ago by Bert Slagter — view source

unread

Derick Rethans 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

21 years ago by Bert Slagter — view source

unread

Bert Slagter wrote:

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);

Hm, i tried manually setting the second parameter of set_error_handler
to E_ALL | E_STRICT and guess what.. that does the trick :).

I don't know why I didn't think of this first..

So:

set_error_handler('myErrorHandler', `E_ALL` | E_STRICT);

Is this by design?

Bert

21 years ago by Derick Rethans — view source

unread

Is this by design?

No, it's fixed in CVS now.

regards,
Derick

21 years ago by lenar@city.ee — view source

unread

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

[patch] re-invoking default error handler

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

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

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

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

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

So:

set_error_handler('myErrorHandler', E_ALL | E_STRICT);

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

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

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

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

set_error_handler('myErrorHandler', `E_ALL` | E_STRICT);