Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:110315
MIME-Version: 1.0
References: <CAGgaK7KScGnNaCSnz6szCzJ3jndoipH0=yonW6FPKR7GbkPdLw@mail.gmail.com>
In-Reply-To: <CAGgaK7KScGnNaCSnz6szCzJ3jndoipH0=yonW6FPKR7GbkPdLw@mail.gmail.com>
Date: Sat, 30 May 2020 18:36:18 +0100
Message-ID: <CA+kxMuRVwhMjZxA0ctz4PK9VNvNTYSga6bg1V54KeqHXMguQgw@mail.gmail.com>
To: Max Semenik <maxsem.wiki@gmail.com>
Cc: Internals <internals@lists.php.net>
Content-Type: text/plain; charset="UTF-8"
Subject: Re: [PHP-DEV] RFC: Error backtraces
From: Danack@basereality.com (Dan Ackroyd)

On Fri, 29 May 2020 at 19:23, Max Semenik <maxsem.wiki@gmail.com> wrote:
>
> Hi, I'd like to present a new RFC for your consideration:
> https://wiki.php.net/rfc/error_backtraces
>


Hi Max,

I've also been thinking about how errors + warnings can be improved also.

For my own code, I convert all non-silenced errors and warnings into
an exception through the error handler*. That works pretty well, but
has one big downside - it only throws one type of exception.

I wrote some words about what we could do:
https://gist.github.com/Danack/6b5a86414ab9e9d9ac7e0a906216f1c5

The TL:DR of that is, we could add specific exceptions that should be
thrown if the error handler setup by the user decided to throw an
exception. I would be interested in hearing if that idea achieves the
benefits you're trying to get, in a less controversial way.

To give some feedback on your RFC.

"traces will be formatted exactly like for exceptions, and appended to
error messages,"

This has security implications as it would be possible for app secrets
to be included in the error message. That is less of a problem for
exceptions as you can write your own code for logging exceptions that
doesn't expose the variables if you don't want them exposed.

"..., traces are always generated for exceptions"

That depends on what callback function has been set for
'set_exception_handler'. Also....maybe use more formal language for
RFCs. Even minor swear words should be avoided imo.

"error_get_last() output will have another element added to it if a
trace is available:"

I strongly suspect that is a very bad idea. As the variables in the
stack trace are kept alive while that stack trace is kept alive, that
means the lifetime of those variables would depend on how long until
another error occurs.

That sounds like code that is hard to reason about.

"In the current proposed implementation, a new error flag
E_UNHANDLED_EXCEPTION is introduced,"

The RFC doesn't say what this is meant to do precisely.....but my gut
instinct doesn't like it.

cheers
Dan
Ack



* an error handler that converts all unsilenced errors that we care
about into an exception

function standardErrorHandler($errorNumber, $errorMessage, $errorFile,
$errorLine): bool
{
    if (error_reporting() === 0) {
        // Error reporting has been silenced
        if ($errorNumber !== E_USER_DEPRECATED) {
            return true;
        }
    }
    if ($errorNumber === E_DEPRECATED) {
        return false;
    }
    if ($errorNumber === E_CORE_ERROR || $errorNumber === E_ERROR) {
        // For these two types, PHP is shutting down anyway. Return false
        // to allow shutdown to continue
        return false;
    }
    $message = "Error: [$errorNumber] $errorMessage in file $errorFile
on line $errorLine.";

    throw new \Exception($message);
}