Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:94086
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmx.de designates 212.227.17.22 as permitted sender)
To: Niklas Keller <me@kelunik.com>
References: <CAEbg-4Y=DaJogK+EO8dH2x59Ak_vugwXxm4M4hPw4sgtSNqmug@mail.gmail.com>
 <2635b3b6-b971-9fb7-5def-26b86eafb54d@fleshgrinder.com>
 <CANUQDCiYhbyUBX-NTYTJ67er2h6VoqT88zUvhePNY8Q9bSxJJw@mail.gmail.com>
 <b87d8499-7575-435d-5ab6-fc0bd629e3ae@fleshgrinder.com>
 <CANUQDCgQBmvDJJnA6qMMi83TXzSi97eFUJY+2vSDOkJKXuc-ew@mail.gmail.com>
 <CAEbg-4aOf1opjUHJkMPERahcG2+8yYLNDzr4NO9siY7jxtcOUw@mail.gmail.com>
 <CAEbg-4YDOfUbR=skupt1LV3+07X-ocjtHxMqpKpaqbLtku6dJg@mail.gmail.com>
 <98f97016-066e-b5a4-8d1d-4bca179315fa@gmx.de>
 <CANUQDCjz1RwNddF0rB_pgudKuNiZNn2R1JtDAJkG8=x1iC6BGw@mail.gmail.com>
Cc: Alexander Lisachenko <lisachenko.it@gmail.com>,
 PHP Internals <internals@lists.php.net>
Message-ID: <ff9cc21d-6b77-2f91-6838-39d9c89f057e@gmx.de>
Date: Fri, 17 Jun 2016 18:47:22 +0200
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101
 Thunderbird/45.1.1
MIME-Version: 1.0
In-Reply-To: <CANUQDCjz1RwNddF0rB_pgudKuNiZNn2R1JtDAJkG8=x1iC6BGw@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] Throwing an Error for require expressions in PHP7.x
From: cmbecker69@gmx.de (Christoph Becker)

On 17.06.2016 at 17:44, Niklas Keller wrote:

> 2016-06-17 11:46 GMT+02:00 Christoph Becker <cmbecker69@gmx.de>:
> 
>> On 17.06.2016 at 10:29, Alexander Lisachenko wrote:
>>
>>> Nikita, Dmitry, ping. What do you think, shouldn't we replace all
>> possible
>>> places with Fatal Errors with correct throwing of Error exceptions?  In
>>> this concrete case it's for "require" operator.
>>>
>>> From what I can see Error will give us more control over the file loading
>>> process and make it more atomic, because additional checks
>>> if(file_exists($file) && is_readable($file)) generate extra stat
>> commands,
>>> they are slow and not reliable, because for highload projects, we can be
>> in
>>> the situation where "if" succeeded, but the "require" will fail because
>> our
>>> file was deleted immediately after check.
>>>
>>> Code like this:
>>> try {
>>>     require $fileName;
>>> } catch (Error $e) {
>>>     echo "Oops, maybe deleted? " . $e;
>>> }
>>>
>>> is much more reliable than following, by using "include" instead of
>>> "require". This was suggested in https://bugs.php.net/bug.php?id=72089:
>>
>> I have re-opened this ticket.
>>
>>> if (file_exists($fileName) && is_readable($fileName)) {
>>>     @include $fileName; // Silencing errors for the case of
>> race-condition,
>>> etc
>>> }
>>
>> I'm not generally against throwing exceptions from include (or several
>> other filesystem functions, for that matter), but the BC break has to be
>> considered.  I am, however, still opposed to offer the ability to catch
>> a failing *require*.
> 
> Why?

If something is required, one cannot do without it, so there's only one
course of action, namely to bail out.  In my opinion, this is the least
surprising way to handle missing required files, especially as it always
has been that way (consider the potential BC break).

Or do you really prefer to be able to do

    try {
        require $fileName;
    } catch (Error $e) {
        echo "Oops, maybe deleted? " . $e;
    }
    functionDefinedInFileName();

and get a fatal error that function no() is undefined?  I'd rather get a
fatal error that the required file couldn't be opened in the first place.

If, however, a file is not strictly required, one can (and in my
opinion, should) `include` the file.  And yes, there's no absolutely
failsafe way to do this without risking a warning or using the @
operator, but that affects other filesystem functions (e.g.
file_get_contents() and fopen()) as well.  Frankly, I can't see a single
good reason to replace the fatal error with an exception for require,
but leave include etc. alone.  And if include would throw an exception,
I don't see the use of changing require at all.

-- 
Christoph M. Becker