Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:106767
MIME-Version: 1.0
References: <CAF+90c9Kc6m_bv=aJxbD3-RmnNXXLqv6mPjHp7U7qxX_ck27qw@mail.gmail.com>
In-Reply-To: <CAF+90c9Kc6m_bv=aJxbD3-RmnNXXLqv6mPjHp7U7qxX_ck27qw@mail.gmail.com>
Date: Wed, 28 Aug 2019 21:30:15 +0200
Message-ID: <CAGKNXQEr-Pa9+Y1t2RO+XBU+LGApR_hx-crx8TsdVbKKAGtpZQ@mail.gmail.com>
To: Nikita Popov <nikita.ppv@gmail.com>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000d03a350591326b30"
Subject: Re: [PHP-DEV] [RFC] Reclassifying engine warnings
From: phofstetter@sensational.ch (Philip Hofstetter)

--000000000000d03a350591326b30
Content-Type: text/plain; charset="UTF-8"

Hi,

On Wed, Aug 28, 2019 at 11:33 AM Nikita Popov <nikita.ppv@gmail.com> wrote:

>
> I think it's time to take a look at our existing warnings & notices in the
> engine, and think about whether their current classification is still
> appropriate. Error conditions like "undefined variable" only generating a
> notice is really quite mind-boggling.
>

As a user dealing with a big legacy code-base that's still being in heavy
use and active development, while I appreciate this RFC in general, let me
please voice my concerns about promoting "Undefined index: %s" from
E_NOTICE to E_WARNING without also doing other changes.

The point is that in many cases, PHP still tends to just emit an E_WARNING
for otherwise pretty exceptional cases, like being unable to open a file,
or failing network operations. Now normally, you will of course check
return values, but if you mess it up, the risk of bad things happening is
too big, so right now, it's sensible to treat E_WARNING as fatal in
production.

E_NOTICEs on the other hand are extremely uncritical (minus the undefined
variable case you've highlighted), so it's sensible to ignore those in
production and it's tempting to do so even in development and, I freely
admit, that's totally the case in the code-base I'm dealing with and I
would assume others would also have it configured that way. Code designed
to avoid "Undefined index: %s" is very boilerplaty to write and in various
code-bases (including ours), accessing undefined indexes in arrays is
mostly inconsequential.

So this leaves me (and I assume others) in kind of a pickle, because it
feels that right now it's still too risky to not treat E_WARNING as fatal
in production and on the other hand, this RFC is going to turn completely
harmless undefined index access into an E_WARNING.

Even if you don't care about my particular code-base (which I would totally
understand), please consider that this is quite the BC breakage in general
and in our case, it will probably force us to run with a patched PHP engine
as we are unable to clean the code up in any kind of reasonable time frame.

Again - that's my problem. I know. But I still would like to voice this
concern.

Of course, even when passing this RFC in full, there are options you could
provide for people in our kind of situation:

* If all functions that currently emit an E_WARNING in cases where they
should actually throw could be made to throw, then it would be safe to run
production without treating E_WARNING as fatal. I'm afraid that too is a
big BC break, but at least it's breaking code running into otherwise
serious issues rather than code doing something mostly benign.
* If this can't be done, which I totally understand, then please consider a
way for a error handler to distinguish between various errors, notices and
warnings without having to string match on error messages. That feels
incredibly brittle and also ties the PHP language into ossified error
messages that can never be changed without also causing potential BC issues.

Still. I wholeheartedly thank you all for your recent efforts to clean up
the language and bring it forward to the current state of things. But
please consider the huge swaths of existing code which sometimes cannot be
practically changed.

I do not have any kind of voting rights, so in the end my opinion doesn't
matter, but if I had the ability to vote, I would have to vote "no" over
this unless some kind of escape hatch is provided.

Thank you

Philip

--000000000000d03a350591326b30--