Re: [PHP-DEV] [RFC]: Change Default mysqli Error Mode

[Kamil Tekiela]

Your opinion is highly appreciated, Johannes. I feel that we will have to
implement this change as you are proposing.
I don't think I am rushing this change, because I believe is already late.
It should have been fixed in PHP 7 or PHP 8, but because very little people
use mysqli this extension hasn't seen much love. I know we can't remove
mysqli completely so I am trying to improve the situation a little bit. The
reason why I think the time is now is because of changes to PDO default
error reporting mode and the move from Warnings to Errors in PHP 8. Moving
to mysqli exceptions makes sense now. Other warnings in mysqli extension
have already been turned to exceptions.
I don't think the deprecation notice will reach as many people as we hope,
but at least it will give us peace of mind.
I am sorry if I sound a little bit defensive about my ideas, but I know
from experience that many developers are allergic to any kind of change and
I have to use a lot of convincing to achieve anything.
I'll close the voting tomorrow and then I will think on how to adapt the
RFC to deploy deprecation notice first.

Thanks.
Kamil

Re: [PHP-DEV] [RFC]: Change Default mysqli Error Mode

[Johannes Schlüter]

On February 27, 2021 7:29:43 PM GMT+01:00, Kamil Tekiela  
wrote:
>> The code shown here
>https://www.w3schools.com/php/func_mysqli_connect.asp is
>relatively correct at the moment. Maybe not nice, but it handles the
>error.
>That is the second hit on google (after php.net manual) when searching
>for
>"php tutorial mysqli_conect") Most other books, tutorials, ... teach
>the
>same.
>
>w3schools is hardly something that should be regarded as a reputable
>source
>of information 

That is correct. I wouldn't advice following that.

> and should not impact the direction PHP is heading in.

That I don't agree with. The things covered in tutorials and books is what 
people use to learn. What people learn is what they use for some time in their 
path. We have a responsibility towards our users.

>This problem comes mostly from the fact that the developers who wrote
>these
>tutorials were unaware that there is automatic error reporting
>available.

For many that is true. In some cases tutorial writers are happy about the fact 
that they don't have to teach exceptions early.

Error handling in PHP is a mess, but I consider the path towards more 
exceptions good. But has to be done with care.

>PHP manual didn't explain this either. This RFC proposes to change that by
>making it the default so that people can't miss it.
>The reason why I really want this default setting to change is that I
>was
>lied to by books and tutorials when I was learning PHP. I wish I knew
>about
>automatic error reporting instead of struggling with invisible SQL
>errors.
>I still hold a grudge against the tutorials teaching me manual error
>checking or the use of real_escape_string. I do not want new developers
>in
>the era of PHP 8 to suffer the same pain I did.

These tutorials are a pain and there is a large group of people creating worst 
quality content and hoping to benefit from SEO stuff and then earning some 
money from ads. Others focus on content and don't get attention. That is a big 
problem we're having.

I disagree however that throwing out errors the readers don't understand helps. 
How many will say "oh the tutorial is broken" and how many will say "my PHP is 
broken"? Back in my days, I remember, I was more often looking for failures by 
me or my system configuration than blaming the tutorial.

[...]

>> In 8.1 add deprecation notices to mysqli_connect & friends AND to
>access
>to the connection_error (if I remember right that goes via the __get
>hook,
>thus we can add a deprecation note to a "property" access)
>> The later is a good place as many people use the @ operator on the
>connect, but not on the error check.
>> In a later version we change it.
>> That gives a chance to reach users. We will still miss many, but for
>a
>notable amount of people there is a chance.
>
>I believe Nikita has proposed it already. I am not very happy about it
>as
>this would mostly go unnoticed. People who have mysqli error reporting
>silenced often have every other error reporting silenced too including
>deprecation notices. Adding such deprecation notice would just create
>another confusing PHP error that developers would have to fix.

The difference is: With a deprecation warning we control the error message 
better.

> I am sorry
>to say this about my fellow developers but many of the ones that you are
>targeting with this proposal who wouldn't read an upgrade guide before
>upgrading, are also the ones who would ignore this deprecation warning in
>their error logs. "Hey, the code still works, I don't know what this
>deprecation notice is talking about!"
>For these reasons, I am unwilling to go through a deprecation phase. I
>believe it would be just a waste of time. But if this is the only way
>that
>this change will land in PHP then I would also accept it as a very
>unsatisfying compromise.

I don't know why there is a specific time pressure. That behavior is now almost 
twenty years old.

And we'll, about visibility: Tutorial writers hopefully don't ignore it and can 
catch up. Also deprecation warnings have a chance to be reported to the authors 
etc. Yes, it won't reach everybody, but we're talking here about a change 
affecting huge numbers of systems.  If we help just a few hundred thousands of 
those to make a smoother transition it's already a good thing.

And yes, there are more people reading deprecstion notices than changelogs. 
Outside the enthusiasts only few do. Partially since most entries are 
irrelevant, partially since there are so many changelogs one could read all 
day, partially since people don't even know what version they are running or 
that the host is being upgraded by an admin.

People look at it, if they notice a problem. But they first need a way to even 
notice. Silently ignoring half the code (the existing error handling) goes 
unnoticed.

Last thing: I truly think it is great that your trying to push this forward! We 
always need people coming up here and fixing the painpoints they had! It is 
easy 

Re: [PHP-DEV] [RFC]: Change Default mysqli Error Mode

[Benjamin Eberlei]

On Sat, Feb 27, 2021 at 5:24 PM Kamil Tekiela  wrote:

> >
> > The issue is, as said, that this only happens in an error situation.
> > Thus if users only test "does my site still work after update?" (what
> > many do ...) won't notice this, till something fails, which would have
> > been caught nice beforehand.
> >
>
> I don't see why this would be such an issue. If the code fails for any
> reason then an error is produced. The error might be surprising, but it is
> not something that would cause problems.
>

I think you are very wrong about this, there could be perfectly valid code
doing:

if (!$conn->query("select 1") && $conn->errno == 2006) { // mysql gone away
$conn = new mysqli();
}

With your proposed change the query would throw an exception instead,
potentially breaking follow up code that is required to run.

This is the kind of code you have that only gets triggered every 10
times and that is very hard to write a real test for. If the test here uses
a mock, then you wont catch the behavior change.


> This change fixes more problems than it creates. I believe it is still
> worthy to put it in 8.1.  Every PHP release contains backwards-incompatible
> changes. In comparison to other BC changes PHP has put in minor releases
> this one has a very easy fix. You don't have to scour the code looking for
> places to fix.
>

There are projects out there that do a mysqli_connect on *every* script.
You underestimate the impact of this break.

PHP can't improve if we have to think of people who upgrade without reading
> the release notes. We also shouldn't be held back by outdated tutorials. In
> fact, this will help people who still use such tutorials. This isn't a
> major change that would affect everyone on the planet. This will only
> affect people who still use mysqli and didn't set the error reporting mode
> before upgrading.
>

I am sorry, but to me that feels a bit like you want to take the easy way
out..

As a counterexample, Nikita made a very good RFC for 8.1 / 9.0 deprecating
and changing the nullable to internal functions behavior:
https://wiki.php.net/rfc/deprecate_null_to_scalar_internal_arg

This RFC goes above and beyond to provide a migration path, deprecation
messages and such.

Yes it has a higher potential break impact, but i don't expect this change
to wait until 9.0 either.

I would wish your RFC would throw a deprecation message in 8.1 that you
cannot make a query without setting the reporting level before by calling
mysqil_report() explicitly. Then in 8.2 we could change the behavior.


> I understand your worries but I don't think this is a good enough reason to
> not go ahead with this change.
>
> Kind regards,
> Kamil
>

Re: [PHP-DEV] [RFC]: Change Default mysqli Error Mode

[Rowan Tommins]

On 27/02/2021 18:10, Kamil Tekiela wrote:


Select * From products Where product_id = 'hello world'

If product_id is a column of type int, then the database will raise an
error about incompatible types.


No, this query would produce a warning. I am unaware of an SQL mode 
that would turn this warning into an error.



Right, I forgot we were talking about MySQL, which has the same "ignore 
and carry on" tradition as PHP; in other DBMSes, that would be an error:


PostgreSQL:

> ERROR: invalid input syntax for type integer: "hello world"
> LINE 1: Select * From products Where product_id = 'hello world';

Microsoft SQL Server:

> Msg 245 Level 16 State 1 Line 1
> Conversion failed when converting the varchar value 'hello world' to 
data type int.



However, the main point of the example was to show that an error could 
go unnoticed, and actually have no immediate negative effects, such that 
replacing it with an exception makes things worse.





Examples, that would truly be effected are something like this:

$stmt = $mysqli->query('INSERT INTO tableA (uniqueColumn) 
VALUE("repeated value")');

if (1062 === $mysqli->errno) {
    echo "Can't add this value because it already exists!";
}

If the developer didn't silence the errors then the user will see a 
generic error instead of the specific one.



Precisely, and this is what people are concerned about: if someone is 
writing code from scratch, exception-by-default is a good thing; but if 
they have existing error handling, by-passing that handling by throwing 
an exception is not.



Regards,

--
Rowan Tommins
[IMSoP]

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] [RFC]: Change Default mysqli Error Mode

[Kamil Tekiela]

> The code shown here https://www.w3schools.com/php/func_mysqli_connect.asp is
relatively correct at the moment. Maybe not nice, but it handles the error.
That is the second hit on google (after php.net manual) when searching for
"php tutorial mysqli_conect") Most other books, tutorials, ... teach the
same.

w3schools is hardly something that should be regarded as a reputable source
of information and should not impact the direction PHP is heading in.
This problem comes mostly from the fact that the developers who wrote these
tutorials were unaware that there is automatic error reporting available.
PHP manual didn't explain this either. This RFC proposes to change that by
making it the default so that people can't miss it.
The reason why I really want this default setting to change is that I was
lied to by books and tutorials when I was learning PHP. I wish I knew about
automatic error reporting instead of struggling with invisible SQL errors.
I still hold a grudge against the tutorials teaching me manual error
checking or the use of real_escape_string. I do not want new developers in
the era of PHP 8 to suffer the same pain I did.

> Spend a day looking at stackoverflow and you see a different world. Also
many users just want to use a PHP app and don't care about PHP.

Yes, but once again the problem is the old tutorials that teach it
incorrectly. Most of them still are vulnerable to SQL injection. We can't
change the tutorials but we can change the recommended setting in PHP.

> In 8.1 add deprecation notices to mysqli_connect & friends AND to access
to the connection_error (if I remember right that goes via the __get hook,
thus we can add a deprecation note to a "property" access)
> The later is a good place as many people use the @ operator on the
connect, but not on the error check.
> In a later version we change it.
> That gives a chance to reach users. We will still miss many, but for a
notable amount of people there is a chance.

I believe Nikita has proposed it already. I am not very happy about it as
this would mostly go unnoticed. People who have mysqli error reporting
silenced often have every other error reporting silenced too including
deprecation notices. Adding such deprecation notice would just create
another confusing PHP error that developers would have to fix. I am sorry
to say this about my fellow developers but many of the ones that you are
targeting with this proposal who wouldn't read an upgrade guide before
upgrading, are also the ones who would ignore this deprecation warning in
their error logs. "Hey, the code still works, I don't know what this
deprecation notice is talking about!"
For these reasons, I am unwilling to go through a deprecation phase. I
believe it would be just a waste of time. But if this is the only way that
this change will land in PHP then I would also accept it as a very
unsatisfying compromise.

>

Re: [PHP-DEV] [RFC]: Change Default mysqli Error Mode

[Kamil Tekiela]

>
> Select * From products Where product_id = 'hello world'
>
> If product_id is a column of type int, then the database will raise an
> error about incompatible types.
>

No, this query would produce a warning. I am unaware of an SQL mode that
would turn this warning into an error. Maybe I am wrong and there is a way
to convert this into an error, but that would be pointless.
I believe you have chosen the wrong example. That query, even when executed
with PDO, doesn't throw an exception.

The point of this RFC is that mysqli should treat SQL errors the same way
that PDO does. By default, all errors should be reported. An error in SQL
is the same as an exception in PHP code; it means that something went
terribly wrong. I have been using PDO for a few years now and I never had a
need to silence its error reporting. SQL doesn't spit out exceptions
randomly.

Examples, that would truly be effected are something like this:

$stmt = $mysqli->query('INSERT INTO tableA (uniqueColumn) VALUE("repeated
value")');
if (1062 === $mysqli->errno) {
echo "Can't add this value because it already exists!";
}

If the developer didn't silence the errors then the user will see a generic
error instead of the specific one.

Re: [PHP-DEV] [RFC]: Change Default mysqli Error Mode

[Rowan Tommins]

On 27/02/2021 16:57, Kamil Tekiela wrote:


If product_id is actually an integer column, this function is
technically broken: given a non-integer input, it will produce an
error
in the database.


I'm sorry, but I do not understand why would that code produce an 
error. The value is properly escaped and formatted so there should be 
no error at all. Is this based on some SQL setting? That SQL looks 
correct to me and I do not see any syntax errors.



That's precisely why it's the kind of code that goes unfixed for years, 
but it is broken: if $product_id is the string 'hello world', then this 
line...


$sql = "Select * From products Where product_id = '" . 
$dbWrapper->escape($product_id) . "'";


...produces this SQL:

Select * From products Where product_id = 'hello world'

If product_id is a column of type int, then the database will raise an 
error about incompatible types.



If the PHP database wrapper just swallows this error and returns false, 
then somewhere else in the code, you can write this:


$product = get_product($_GET['product_id']);
if ( ! $product ) {
   display_error_message('Sorry, we could not find that product.');
}

So although the code is wrong, the user always gets a reasonable error 
message. Now replace the database implementation with one that throws 
exceptions, and that message will never display; if you're lucky, 
there's a default exception handler set; if not, the user will get a 
blank white page.



Regards,

--
Rowan Tommins
[IMSoP]

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] [RFC]: Change Default mysqli Error Mode

[Johannes Schlüter]

On February 27, 2021 5:23:47 PM GMT+01:00, Kamil Tekiela  
wrote:
>>
>> The issue is, as said, that this only happens in an error situation.
>> Thus if users only test "does my site still work after update?" (what
>> many do ...) won't notice this, till something fails, which would
>have
>> been caught nice beforehand.
>>
>
>I don't see why this would be such an issue. If the code fails for any
>reason then an error is produced. The error might be surprising, but it
>is
>not something that would cause problems. In situations where the code
>depends on the silenced errors
> to work properly

The code shown here https://www.w3schools.com/php/func_mysqli_connect.asp is 
relatively correct at the moment. Maybe not nice, but it handles the error. 
That is the second hit on google (after php.net manual) when searching for "php 
tutorial mysqli_conect") Most other books, tutorials, ... teach the same.

> then the developers
>would
>know this and they would test for it during the upgrade.

That is true for the ones posting here and talking at conferences.
Spend a day looking at stackoverflow and you see a different world. Also many 
users just want to use a PHP app and don't care about PHP. In many cases some 
agency (which often have no idea about software development, but mostly care 
about the UI design and reaching a goal quickly) created an app and the users 
just want to run it and got an server with thestest version. This isn't nice, 
but it is a reality.

>This change fixes more problems than it creates. I believe it is still
>worthy to put it in 8.1.

It is true that more consistent error reporting via Exceptions would be better 
and solve many problems. These are the reminders of the "OO wars" we fought 
around the time of the PHP 4 - PHP 5 transition and I think it is great that 
many people are working on cleaning this up!

>  Every PHP release contains
>backwards-incompatible
>changes.

There is a balance we're always fighting about. There are cases where BC breaks 
are done and hopefully most of them explode loud. For instance of we add a new 
keyword. This emits and error quite quickly, a simple lint check is enough to 
find it. Just including the file will fail.

Here we are talking about a case where the break will only occur in an errorous 
situation. Which users will only spot when reading the upgrading guide 
carefully. In an area 99% of beginner tutorials and books touch.

> In comparison to other BC changes PHP has put in minor
>releases
>this one has a very easy fix. You don't have to scour the code looking
>for places to fix.

The fix is easy. Getting awareness is hard.

>PHP can't improve if we have to think of people who upgrade without
>reading
>the release notes. We also shouldn't be held back by outdated
>tutorials. In
>fact, this will help people who still use such tutorials. This isn't a
>major change that would affect everyone on the planet. This will only
>affect people who still use mysqli and didn't set the error reporting
>mode
>before upgrading.
>I understand your worries but I don't think this is a good enough
>reason to
>not go ahead with this change.

I am not about stagnating and not doing anything. As said: I think the idea is 
good. I am about thinking about the process and timing.

What about this approach:

In 8.1 add deprecation notices to mysqli_connect & friends AND to access to the 
connection_error (if I remember right that goes via the __get hook, thus we can 
add a deprecation note to a "property" access)

The later is a good place as many people use the @ operator on the connect, but 
not on the error check.

In a later version we change it.

That gives a chance to reach users. We will still miss many, but for a notable 
amount of people there is a chance.

johannes

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] [RFC]: Change Default mysqli Error Mode

[Kamil Tekiela]

>
> If product_id is actually an integer column, this function is
> technically broken: given a non-integer input, it will produce an error
> in the database.
>

I'm sorry, but I do not understand why would that code produce an error.
The value is properly escaped and formatted so there should be no error at
all. Is this based on some SQL setting? That SQL looks correct to me and I
do not see any syntax errors.
Also, silenced or not, if an error occurs on MySQL side then PHP has to
check for it one way or another or it will break with some other horrendous
error.

Re: [PHP-DEV] [RFC]: Change Default mysqli Error Mode

[Rowan Tommins]

On 27/02/2021 16:23, Kamil Tekiela wrote:

I don't see why this would be such an issue. If the code fails for any
reason then an error is produced. The error might be surprising, but it is
not something that would cause problems.



Just addressing this argument, not the proposal itself, a function 
throwing an exception that previously failed silently absolutely could 
cause problems.


Consider this code:

function get_product($product_id) {
    $sql = "Select * From products Where product_id = '" . 
$dbWrapper->escape($product_id) . "'";

    return $dbWrapper->runQuery($sql);
}

If product_id is actually an integer column, this function is 
technically broken: given a non-integer input, it will produce an error 
in the database. However, with lax error handling, the calling code will 
simply treat this as the product not existing, and handle it gracefully.


If $dbWrapper->runQuery suddenly starts throwing exceptions, functions 
that were inadvertently relying on this behaviour will start failing, 
possibly with an uncaught exception propagating to the user.


The exception is certainly pointing out a real bug, and the function can 
easily be fixed once the problem is spotted, but the change has a real 
negative impact on the application.



To be clear, I am not saying I agree or disagree with this particular 
proposal. I note that the user could in this example update their DB 
wrapper to set the error mode explicitly, or to catch exceptions. I just 
wanted to show that "it only happens on error" doesn't mean the impact 
is zero.


Regards,

--
Rowan Tommins
[IMSoP]

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] [RFC]: Change Default mysqli Error Mode

[Kamil Tekiela]

>
> The issue is, as said, that this only happens in an error situation.
> Thus if users only test "does my site still work after update?" (what
> many do ...) won't notice this, till something fails, which would have
> been caught nice beforehand.
>

I don't see why this would be such an issue. If the code fails for any
reason then an error is produced. The error might be surprising, but it is
not something that would cause problems. In situations where the code
depends on the silenced errors to work properly then the developers would
know this and they would test for it during the upgrade.
This change fixes more problems than it creates. I believe it is still
worthy to put it in 8.1.  Every PHP release contains backwards-incompatible
changes. In comparison to other BC changes PHP has put in minor releases
this one has a very easy fix. You don't have to scour the code looking for
places to fix.
PHP can't improve if we have to think of people who upgrade without reading
the release notes. We also shouldn't be held back by outdated tutorials. In
fact, this will help people who still use such tutorials. This isn't a
major change that would affect everyone on the planet. This will only
affect people who still use mysqli and didn't set the error reporting mode
before upgrading.
I understand your worries but I don't think this is a good enough reason to
not go ahead with this change.

Kind regards,
Kamil

Re: [PHP-DEV] [RFC]: Change Default mysqli Error Mode

[Johannes Schlüter]

On Wed, 2021-01-27 at 15:17 +0100, Sebastian Bergmann wrote:
> Am 25.01.2021 um 11:03 schrieb Nikita Popov:
> > Fully support this proposal. Throwing exceptions is the right
> > default, it
> > matches what PDO does, and restoring the old behavior is one line
> > of code.
> 
> I second that emotion.

The issue is, as said, that this only happens in an error situation.
Thus if users only test "does my site still work after update?" (what
many do ...) won't notice this, till something fails, which would have
been caught nice beforehand.

I agree that the proposed behavior is better, but we have to mind
migration paths.

johannes

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] [RFC]: Change Default mysqli Error Mode

[Kamil Tekiela]

Hi All,

If there are no more comments I would like to start voting on this RFC
soon. If you would like me to change something or if you want to discuss it
further please let me know.

Kind Regards,
Kamil

On Sun, 31 Jan 2021 at 18:20, Kamil Tekiela  wrote:

> Hi All,
>
> I have updated the RFC at https://wiki.php.net/rfc/mysqli_default_errmode
> and provided FAQ with the aim to explain your concerns as best as I could.
> I have also added a section that explains all mysqli error reporting modes.
>
> Best regards,
> Kamil
>

Re: [PHP-DEV] [RFC]: Change Default mysqli Error Mode

[Kamil Tekiela]

Hi All,

I have updated the RFC at https://wiki.php.net/rfc/mysqli_default_errmode
and provided FAQ with the aim to explain your concerns as best as I could.
I have also added a section that explains all mysqli error reporting modes.

Best regards,
Kamil

Re: [PHP-DEV] [RFC]: Change Default mysqli Error Mode

[Björn Larsson]

Den 2021-01-28 kl. 01:31, skrev Kamil Tekiela:


I'm missing a thing here when we talk about "your code". I mean on
our
site we have third party libraries. They can typically have a release
schedule on a couple of releases each year. So all code is not up to
us to change. So how does this proposal adress that scenario when it's
not up to us to do the necessary changes? Is it e.g. a blocker to
upgrade to PHP 8.1 with exceptions vs warnings until the library
is updated?

As a side note I'm at the moment migrating our test site to PHP 8 and
the two most prominent hickups are located in a third party library:
"Undefined array key" and "Attempt to read property "value" on null"
which are warnings. Not sure how it would have been with these as
exceptions, e.g. would the test site stop running and thereby stop
testing of other parts.

And as I have said earlier, I do like exceptions! It's the migration
aspect I'm wondering about...

r//Björn L


This is a valid concern. I have thought about it for a while and I 
think that we should handle this the same way we did with PDO error 
mode change. We don't do anything special.


I really don't see a way that we could make a smoother upgrading 
experience. For users that avail of old libraries the onus is on them 
to check the library's compatibility with the new PHP version. 
Ideally, the change should have been made in PHP 8.0 where it would be 
more justifiable to change a default setting.


The good thing about mysqli error reporting compared to PDO is that 
the setting is global. If the library you are using is abandoned or 
you can't wait for the library to be updated then you can set the 
error reporting level in your own code. The only problem comes when 
you want to mix both modes in the same code, but that is a huge code 
smell anyway. This means that this change is not a blocker for the 
upgrade.


I think the proposal is good to be implemented in PHP 8.1 as it is 
now. But if anyone thinks otherwise or has any ideas on how to improve 
I would gladly take it into consideration. If there is any way that we 
could make the migration smoother then it would be great.


Kamil


Yes, this proposal would probably have been close to a no-brainer in
PHP 8.0.

I think the RFC would benefit on expanding on some of the key issues
that has been discussed, e.g. the clarification above about migration
aspects.

/r//Björn L/

Re: [PHP-DEV] [RFC]: Change Default mysqli Error Mode

[Kamil Tekiela]

>
> I'm missing a thing here when we talk about "your code". I mean on our
> site we have third party libraries. They can typically have a release
> schedule on a couple of releases each year. So all code is not up to
> us to change. So how does this proposal adress that scenario when it's
> not up to us to do the necessary changes? Is it e.g. a blocker to
> upgrade to PHP 8.1 with exceptions vs warnings until the library is
> updated?
>
> As a side note I'm at the moment migrating our test site to PHP 8 and
> the two most prominent hickups are located in a third party library:
> "Undefined array key" and "Attempt to read property "value" on null"
> which are warnings. Not sure how it would have been with these as
> exceptions, e.g. would the test site stop running and thereby stop
> testing of other parts.
>
> And as I have said earlier, I do like exceptions! It's the migration
> aspect I'm wondering about...
>
> r//Björn L
>

This is a valid concern. I have thought about it for a while and I think
that we should handle this the same way we did with PDO error mode change.
We don't do anything special.

I really don't see a way that we could make a smoother upgrading
experience. For users that avail of old libraries the onus is on them to
check the library's compatibility with the new PHP version. Ideally, the
change should have been made in PHP 8.0 where it would be more justifiable
to change a default setting.

The good thing about mysqli error reporting compared to PDO is that the
setting is global. If the library you are using is abandoned or you can't
wait for the library to be updated then you can set the error reporting
level in your own code. The only problem comes when you want to mix both
modes in the same code, but that is a huge code smell anyway. This means
that this change is not a blocker for the upgrade.

I think the proposal is good to be implemented in PHP 8.1 as it is now. But
if anyone thinks otherwise or has any ideas on how to improve I would
gladly take it into consideration. If there is any way that we could make
the migration smoother then it would be great.

Kamil

Re: [PHP-DEV] [RFC]: Change Default mysqli Error Mode

[Björn Larsson]

Den 2021-01-27 kl. 17:13, skrev Björn Larsson:

Den 2021-01-27 kl. 16:07, skrev Kamil Tekiela:


Wouldn't this mean everybody would add mysqli_report() to their code?


That is the idea. That is what everyone should already have in their
codebase.

   And wouldn't this render the default of mysqli_report() entirely 
useless?


I'm sorry, but I don't follow the train of thought? Why would it make 
that

useless? You must set the correct error reporting mode anyway and that
function helps in doing so.

   Or is your idea that at a later stage you then want to create a 
warning

for manual mysqli_report() calls?


What are manual mysqli_report() calls? You either have that line in your
code or you don't. The idea that Nikita suggested is to meet your 
concerns.

The only way to figure out if your code needs an adjustment is to have a
warning emitted from any mysqli connect methods if you have not called
mysqli_report before. It would inform users that they are missing the
setting.

   No, I want to know if I have any code which needs fixing so I can 
see how
I want to change those places e.g. a duplicate key from INSERT to 
REPLACE

or whatever the correct fix is. Without the code aborting.


You can do that right now. This proposal will not make it easier or more
difficult for you to do that. Just paste 
mysqli_report(MYSQLI_REPORT_ERROR)
before mysqli_connect() and PHP will generate a warning whenever your 
SQL

throws an error.

   Otherwise the temptation is big to just add a global

mysqli_report(MYSQLI_REPORT_OFF) everywhere just in case.


I don't get what's wrong with that. This is literally what I said in the
RFC you can do if you don't want mysqli to report any errors. Whoever
doesn't want to see errors can use this line to silence them.
I would even make a counterpoint. If we set  MYSQLI_REPORT_ERROR as the
default then users will definitely silence errors and forget. The 
warning

mode is so useless and annoying that people will prefer to silence it
seeing no benefit to it. If we want this change to be useful then we 
should

make SQL errors report as exceptions by default.

Regarding Nikita's proposal: The compromise is reasonable, but this will
not be well-received by the community. Additional warnings will only 
make
things more confusing without providing outstanding benefits. If we 
want to
force people to set the error reporting mode that they want, then it 
would
be better to set a sensible default value and give a clear 
explanation in
the migration guide how to set it back to the previous value. Of 
course, I
don't hate Nikita's proposal and I might say I even like it a little 
bit,
but only in connection with exception mode as the default value. This 
way
users will not be surprised by an exception if it happens that they 
forget

to silence. But I would prefer that everyone embraced exceptions.

I'm missing a thing here when we talk about "your code". I mean on our 
site
we have third party libraries. They can typically have a release 
schedule on
a couple of releases each year. So all code is not up to us to change. 
So how

does this proposal adress that scenario when it's  not up to us to do the
necessary changes? Is it e.g. a blocker to upgrade to PHP 8.1 with
exceptions vs warnings until the library is updated?

As a side note I'm at the moment migrating our test site to PHP 8 and
the two most prominent hickups are located in a third party library:
"Undefined array key" and "Attempt to read property "value" on null"
which are warnings. Not sure how it would have been with these as
exceptions, e.g. would the test site stop running and thereby stop
testing of other parts.

And as I have said earlier, I do like exceptions! It's the migration
aspect I'm wondering about...

r//Björn L


PS And sorry for the lousy formatting in earlier response.

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] [RFC]: Change Default mysqli Error Mode

[Björn Larsson]

Den 2021-01-27 kl. 16:07, skrev Kamil Tekiela:


Wouldn't this mean everybody would add mysqli_report() to their code?


That is the idea. That is what everyone should already have in their
codebase.

   And wouldn't this render the default of mysqli_report() entirely useless?

I'm sorry, but I don't follow the train of thought? Why would it make that
useless? You must set the correct error reporting mode anyway and that
function helps in doing so.

   Or is your idea that at a later stage you then want to create a warning

for manual mysqli_report() calls?


What are manual mysqli_report() calls? You either have that line in your
code or you don't. The idea that Nikita suggested is to meet your concerns.
The only way to figure out if your code needs an adjustment is to have a
warning emitted from any mysqli connect methods if you have not called
mysqli_report before. It would inform users that they are missing the
setting.

   No, I want to know if I have any code which needs fixing so I can see how

I want to change those places e.g. a duplicate key from INSERT to REPLACE
or whatever the correct fix is. Without the code aborting.


You can do that right now. This proposal will not make it easier or more
difficult for you to do that. Just paste mysqli_report(MYSQLI_REPORT_ERROR)
before mysqli_connect() and PHP will generate a warning whenever your SQL
throws an error.

   Otherwise the temptation is big to just add a global

mysqli_report(MYSQLI_REPORT_OFF) everywhere just in case.


I don't get what's wrong with that. This is literally what I said in the
RFC you can do if you don't want mysqli to report any errors. Whoever
doesn't want to see errors can use this line to silence them.
I would even make a counterpoint. If we set  MYSQLI_REPORT_ERROR as the
default then users will definitely silence errors and forget. The warning
mode is so useless and annoying that people will prefer to silence it
seeing no benefit to it. If we want this change to be useful then we should
make SQL errors report as exceptions by default.

Regarding Nikita's proposal: The compromise is reasonable, but this will
not be well-received by the community. Additional warnings will only make
things more confusing without providing outstanding benefits. If we want to
force people to set the error reporting mode that they want, then it would
be better to set a sensible default value and give a clear explanation in
the migration guide how to set it back to the previous value. Of course, I
don't hate Nikita's proposal and I might say I even like it a little bit,
but only in connection with exception mode as the default value. This way
users will not be surprised by an exception if it happens that they forget
to silence. But I would prefer that everyone embraced exceptions.

I'm missing a thing here when we talk about "your code". I mean on our 
site we have third party libraries. They can typically have a release 
schedule on a couple of releases each year. So all code is not up to

us to change. So how does this proposal adress that scenario when it's
not up to us to do the necessary changes? Is it e.g. a blocker to 
upgrade to PHP 8.1 with exceptions vs warnings until the library is updated?


As a side note I'm at the moment migrating our test site to PHP 8 and
the two most prominent hickups are located in a third party library: 
"Undefined array key" and "Attempt to read property "value" on null"

which are warnings. Not sure how it would have been with these as
exceptions, e.g. would the test site stop running and thereby stop
testing of other parts.

And as I have said earlier, I do like exceptions! It's the migration
aspect I'm wondering about...

r//Björn L

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] [RFC]: Change Default mysqli Error Mode

[Kamil Tekiela]

>
> Wouldn't this mean everybody would add mysqli_report() to their code?

That is the idea. That is what everyone should already have in their
codebase.

  And wouldn't this render the default of mysqli_report() entirely useless?

I'm sorry, but I don't follow the train of thought? Why would it make that
useless? You must set the correct error reporting mode anyway and that
function helps in doing so.

  Or is your idea that at a later stage you then want to create a warning
> for manual mysqli_report() calls?

What are manual mysqli_report() calls? You either have that line in your
code or you don't. The idea that Nikita suggested is to meet your concerns.
The only way to figure out if your code needs an adjustment is to have a
warning emitted from any mysqli connect methods if you have not called
mysqli_report before. It would inform users that they are missing the
setting.

  No, I want to know if I have any code which needs fixing so I can see how
> I want to change those places e.g. a duplicate key from INSERT to REPLACE
> or whatever the correct fix is. Without the code aborting.

You can do that right now. This proposal will not make it easier or more
difficult for you to do that. Just paste mysqli_report(MYSQLI_REPORT_ERROR)
before mysqli_connect() and PHP will generate a warning whenever your SQL
throws an error.

  Otherwise the temptation is big to just add a global
> mysqli_report(MYSQLI_REPORT_OFF) everywhere just in case.

I don't get what's wrong with that. This is literally what I said in the
RFC you can do if you don't want mysqli to report any errors. Whoever
doesn't want to see errors can use this line to silence them.
I would even make a counterpoint. If we set  MYSQLI_REPORT_ERROR as the
default then users will definitely silence errors and forget. The warning
mode is so useless and annoying that people will prefer to silence it
seeing no benefit to it. If we want this change to be useful then we should
make SQL errors report as exceptions by default.

Regarding Nikita's proposal: The compromise is reasonable, but this will
not be well-received by the community. Additional warnings will only make
things more confusing without providing outstanding benefits. If we want to
force people to set the error reporting mode that they want, then it would
be better to set a sensible default value and give a clear explanation in
the migration guide how to set it back to the previous value. Of course, I
don't hate Nikita's proposal and I might say I even like it a little bit,
but only in connection with exception mode as the default value. This way
users will not be surprised by an exception if it happens that they forget
to silence. But I would prefer that everyone embraced exceptions.

Re: [PHP-DEV] [RFC]: Change Default mysqli Error Mode

[Nikita Popov]

On Wed, Jan 27, 2021 at 3:35 PM Christian Schneider 
wrote:

> Am 27.01.2021 um 14:34 schrieb Nikita Popov :
> > I think if you wanted to introduce an additional diagnostic step, the
> way to go about it would be to issue a deprecation warning when creating a
> connection without mysqli_report() having been explicitly called beforehand.
>
> Wouldn't this mean everybody would add mysqli_report() to their code?
> And wouldn't this render the default of mysqli_report() entirely useless?
> Or is your idea that at a later stage you then want to create a warning
> for manual mysqli_report() calls?
>

Yes, that would render a call to mysqli_report() required until the version
where the default was switched, and the requirement went away. A call to
mysqli_report() is already required if you use exception-based error
handling. The deprecation would force those people who use manual error
handling by default to make an explicit choice.


> > As others have said, the warning mode is pretty much entirely useless,
> and what you really want (presumably) is to know about the places where you
> need to explicitly call mysqli_report() to preserve the old behavior (or
> explicitly switch to the new one).
>
> No, I want to know if I have any code which needs fixing so I can see how
> I want to change those places e.g. a duplicate key from INSERT to REPLACE
> or whatever the correct fix is. Without the code aborting.
>

Okay, I think I now get what you want. My line of thinking was that people
who use manual error handling likely just want to keep doing that, and
should opt-in to it once the default changes. You apparently want them to
switch to use exceptions, in which case the warning mode might indeed be
useful to identify cases where "expected" errors occur.

However, I don't think this has much to do with the proposal at hand. If
someone wants to migrate their code to use exceptions, that's of course
great, but they could do that any time, regardless of what the default is.
They could have done that ten years ago, and they could do it in ten years.

This proposal is only about switching the default, which is why I mentioned
the possibility of diagnosing cases where it may be observable. That said,
I don't think doing so would actually be particularly useful in practice.
And it wouldn't even address your particular concern. As such, I think the
RFC should stick with the original proposal of simply flipping the default
directly.

Regards,
Nikita

Re: [PHP-DEV] [RFC]: Change Default mysqli Error Mode

[Christian Schneider]

Am 27.01.2021 um 14:34 schrieb Nikita Popov :
> I think if you wanted to introduce an additional diagnostic step, the way to 
> go about it would be to issue a deprecation warning when creating a 
> connection without mysqli_report() having been explicitly called beforehand.

Wouldn't this mean everybody would add mysqli_report() to their code?
And wouldn't this render the default of mysqli_report() entirely useless?
Or is your idea that at a later stage you then want to create a warning for 
manual mysqli_report() calls?

> As others have said, the warning mode is pretty much entirely useless, and 
> what you really want (presumably) is to know about the places where you need 
> to explicitly call mysqli_report() to preserve the old behavior (or 
> explicitly switch to the new one).

No, I want to know if I have any code which needs fixing so I can see how I 
want to change those places e.g. a duplicate key from INSERT to REPLACE or 
whatever the correct fix is. Without the code aborting.

Yes, individual developers can switch on mysqli_report(MYSQLI_REPORT_ERROR) 
*now* to get this behavior but I still think this makes more sense as a 
default. Otherwise the temptation is big to just add a global 
mysqli_report(MYSQLI_REPORT_OFF) everywhere just in case.

- Chris

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] [RFC]: Change Default mysqli Error Mode

[Sebastian Bergmann]

Am 25.01.2021 um 11:03 schrieb Nikita Popov:

Fully support this proposal. Throwing exceptions is the right default, it
matches what PDO does, and restoring the old behavior is one line of code.


I second that emotion.

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] [RFC]: Change Default mysqli Error Mode

[Nikita Popov]

On Tue, Jan 26, 2021 at 8:42 AM Christian Schneider 
wrote:

> Am 26.01.2021 um 07:57 schrieb Alexandru Pătrănescu :
> > There will be other parts that will require testing, whatever you do.
> Trying to optimize this to reduce the 1 line quick fix that could be easily
> read in the documentation at https://www.php.net/manual/en/migration81.php
>  sounds to me like we're
> trying too hard to reduce this cost, ignoring others.
>
> The migration guides are already getting quite long, just have a look at
> https://www.php.net/manual/de/migration80.incompatible.php <
> https://www.php.net/manual/de/migration80.incompatible.php>
> "Just one more line" can add up and that's why I want to be able to
> proactively fix my code, not fixing it at the time of migration.
>
> If the error mode would be changed to warnings instead of exceptions first
> then every developer interested in best practices could and would already
> adapt their code. But they could do it gradually without the disruption of
> hard fails looming over them.
>
> I just think giving developers a warning period (per default, not opt-in)
> is the right thing to do for BC breaks. [ Did I mention round()? ;-) ]
>
> I start to sound like a broken record, I'll leave it at that,
> - Chris
>

I think if you wanted to introduce an additional diagnostic step, the way
to go about it would be to issue a deprecation warning when creating a
connection without mysqli_report() having been explicitly called
beforehand. As others have said, the warning mode is pretty much entirely
useless, and what you really want (presumably) is to know about the places
where you need to explicitly call mysqli_report() to preserve the old
behavior (or explicitly switch to the new one).

Regards,
Nikita

Re: [PHP-DEV] [RFC]: Change Default mysqli Error Mode

[Kamil Tekiela]

I would like to say that I completely agree with George. The BC is minimal.
We are not introducing any new functionality or removing any either. We are
just changing a default setting that was always there. The only code that
would be affected is the one that is currently relying on the default
setting. Most of the mysqli code should already have the setting
enabled/disabled explicitly. If your current code has mysqli error
reporting disabled then nothing will change for you. If your current code
has mysqli error reporting enabled then nothing will change for you. It
will only change in the code that doesn't have it enabled/disabled yet.
Such a project would need to decide whether to enable it or disable it.
This can be done even now, and it should be done now, regardless of whether
this RFC is accepted or not.

> Or (probably more what you have in mind) that they should add
mysqli_report(MYSQLI_REPORT_ERROR) now (to prepare for the change) and
switch to MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT later once they know
they are ready.

IMHO the warning mode is useless as it doesn't stop the code execution
automatically but still throws an error. You either enable error reporting
in the form of exceptions, or you keep it silenced. I believe changing the
default mode to warning mode makes no sense.
Bear in mind that you can do it right now. There's no need for you to wait
for PHP 8.1 to see the change. You can decide on the error reporting mode
right now. However, I fail to see how mysqli_report(MYSQLI_REPORT_ERROR)
could help in migrating the code. As long as your code doesn't have any
errors, the error reporting mode will make no difference.

> This is where you are missing a key component: I want to go through a
phase where I will notice parts where manual error handling (or exception
handling after the change) is missing without having my application abort.
This is what warnings are good for.

Enabling warning mode will not help you in finding places where you are
missing manual error handling. That is the whole point. Manual error
handling means that you have to do it manually. When you enable warnings or
exceptions then manual error handling is already useless. If you think that
your code is missing some manual error handling then enabling
warning/exception mode will not reveal to you where these places are. You
have to go line by line through your code and everwhere you see a mysqli
method/function call you have to add manual error handling.

I want to make things straight. Changing the default error mode is not
forcing anyone to use exception mode out of the sudden. You can still
continue to use whichever mode you are using right now. The change is aimed
at people who start their new projects with PHP 8.1 and want to learn
mysqli.

> I don't see what testing needs to be done, you're reverting to the
previous state which supposedly is already working.

I also do not know what kind of additional testing would be required. The
functionality stays the same. A project code should remain the same unless
its maintainers decide to refactor it. All that is needed is to make sure
that the project has the correct mode enabled and does not rely on the
default setting.

Re: [PHP-DEV] [RFC]: Change Default mysqli Error Mode

[Christian Schneider]

Am 26.01.2021 um 07:57 schrieb Alexandru Pătrănescu :
> There will be other parts that will require testing, whatever you do. Trying 
> to optimize this to reduce the 1 line quick fix that could be easily read in 
> the documentation at https://www.php.net/manual/en/migration81.php 
>  sounds to me like we're 
> trying too hard to reduce this cost, ignoring others.

The migration guides are already getting quite long, just have a look at 
https://www.php.net/manual/de/migration80.incompatible.php 

"Just one more line" can add up and that's why I want to be able to proactively 
fix my code, not fixing it at the time of migration.

If the error mode would be changed to warnings instead of exceptions first then 
every developer interested in best practices could and would already adapt 
their code. But they could do it gradually without the disruption of hard fails 
looming over them.

I just think giving developers a warning period (per default, not opt-in) is 
the right thing to do for BC breaks. [ Did I mention round()? ;-) ]

I start to sound like a broken record, I'll leave it at that,
- Chris

Re: [PHP-DEV] [RFC]: Change Default mysqli Error Mode

[Alexandru Pătrănescu]

On Mon, Jan 25, 2021 at 7:33 PM Christian Schneider 
wrote:

> Am 25.01.2021 um 18:22 schrieb G. P. B. :
> > And yes I'm basically advocating for people to add
> mysqli_report(MYSQLI_REPORT_ERROR) (or even
> mysqli_report(MYSQLI_REPORT_OFF) if they fancy) to their setup, I'm even
> advocating for them to do it right now such that they can catch anything
> ahead of time.
>
> Defaults matter. Most people will *not* do this step and their code will
> fail on upgrade.
>
> > Moreover, this proposal does not prevent you from using the warning mode
> instead of jumping to exceptions directly, the key point is that you need
> to specify it, and I personally believe that having the warning mode as the
> default mode is the worst choice for the reasons I mentioned previously.
>
> Agree to disagree.
>
> If I have the choice to make the smoothen the transition centrally with
> PHP defaults or rely on each user to do it manually by proactively setting
> warning modes I prefer the defaults way.
>
> Remember: I am not advocating staying in the warning mode, I'm just seeing
> it as a useful intermediate step for legacy stuff we want to fix.
>
> - Chris
>
>
  Hi Chris,

I think PHP is far from targeting a zero cost on version upgrade for
projects.
There will be other parts that will require testing, whatever you do.
Trying to optimize this to reduce the 1 line quick fix that could be easily
read in the documentation at https://www.php.net/manual/en/migration81.php
sounds to me like we're trying too hard to reduce this cost, ignoring
others. Other technical costs would be to not switch earlier to new
exception mode as long as the default will cover the cases.

I am thinking about this with a legacy system in mind that is long due on
exception handling for the database layer. I know that having exceptions by
default would spark an interest in developers' minds to do this change
sooner rather than later because they will feel the default is the
recommended way. Due to bandwidth allocation, they might add the line
there, but a task will be created to fix it in the next few months, maybe a
year.

There will be a cost of upgrading. New scenarios and tests that might not
even exist currently, negative flow scenarios.

Regards,
Alex

Re: [PHP-DEV] [RFC]: Change Default mysqli Error Mode

[Christian Schneider]

Am 25.01.2021 um 18:22 schrieb G. P. B. :
> And yes I'm basically advocating for people to add 
> mysqli_report(MYSQLI_REPORT_ERROR) (or even mysqli_report(MYSQLI_REPORT_OFF) 
> if they fancy) to their setup, I'm even advocating for them to do it right 
> now such that they can catch anything ahead of time.

Defaults matter. Most people will *not* do this step and their code will fail 
on upgrade.

> Moreover, this proposal does not prevent you from using the warning mode 
> instead of jumping to exceptions directly, the key point is that you need to 
> specify it, and I personally believe that having the warning mode as the 
> default mode is the worst choice for the reasons I mentioned previously.

Agree to disagree.

If I have the choice to make the smoothen the transition centrally with PHP 
defaults or rely on each user to do it manually by proactively setting warning 
modes I prefer the defaults way.

Remember: I am not advocating staying in the warning mode, I'm just seeing it 
as a useful intermediate step for legacy stuff we want to fix.

- Chris

Re: [PHP-DEV] [RFC]: Change Default mysqli Error Mode

[G. P. B.]

On Mon, 25 Jan 2021 at 16:25, Christian Schneider 
wrote:

> Am 25.01.2021 um 16:59 schrieb G. P. B. :
> > The BC break is totally minimal as it's a one line of code that needs to
> be added (and for all intent and purposes should already be done).
>
> That does not change the fact that it is a BC break which should be
> treated accordingly.
>
> You are basically advocating people should add
> mysqli_report(MYSQLI_REPORT_OFF) to their setup which will totally defeat
> the purpose of the change.
> Or (probably more what you have in mind) that they should add
> mysqli_report(MYSQLI_REPORT_ERROR) now (to prepare for the change) and
> switch to MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT later once they know
> they are ready.
> And this is exactly what I think should be done by the PHP default so
> people don't have to go through this manually.
>
> > Moreover, I'd rather we get rid of the warning modes all together as
> they make the least sense to me.
> > Either you're handling the failures explicitly anyway and you use the
> silent mode, or you don't and want it to throw with the exception mode.
>
> This is where you are missing a key component: I want to go through a
> phase where I will notice parts where manual error handling (or exception
> handling after the change) is missing without having my application abort.
> This is what warnings are good for.
>
> To some of us this is a very valuable step when upgrading.
>
> - Chris
>

I've never said there is no BC break, and frankly this change (and other
similar ones) should probably have been done in PHP 8 at the same time with
PDO.

And yes I'm basically advocating for people to add
mysqli_report(MYSQLI_REPORT_ERROR) (or even mysqli_report(MYSQLI_REPORT_OFF)
if they fancy) to their setup, I'm even advocating for them to do it *right
now* such that they can catch anything ahead of time.
Moreover, this proposal does not prevent you from using the warning mode
instead of jumping to exceptions directly, the key point is that you need
to specify it, and I personally believe that having the warning mode as the
*default* mode is the worst choice for the reasons I mentioned previously.
The removal of the warning modes is completely orthogonal to this proposal,
and I agree it is a very useful tool for transitioning to the exception
mode, but this transition can already be done in PHP 5, 7 and 8.

The grace period for completing this transition is from whenever you want
to whenever, if ever, the warning mode gets deprecated and removed.

On the note that a one line code change is not a good enough justification
for saner defaults, I don't know what to say.
At worst you need to find all usages of a connection start and add the line
to downgrade the error reporting (even to silence if too many warnings are
emitted).
I don't see what testing needs to be done, you're reverting to the previous
state which supposedly is already working.

Best regards,

George P. Banyard

Re: [PHP-DEV] [RFC]: Change Default mysqli Error Mode

[Björn Larsson]

Den 2021-01-25 kl. 17:25, skrev Christian Schneider:

Am 25.01.2021 um 16:59 schrieb G. P. B. :

The BC break is totally minimal as it's a one line of code that needs to be 
added (and for all intent and purposes should already be done).


That does not change the fact that it is a BC break which should be treated 
accordingly.

You are basically advocating people should add mysqli_report(MYSQLI_REPORT_OFF) 
to their setup which will totally defeat the purpose of the change.
Or (probably more what you have in mind) that they should add 
mysqli_report(MYSQLI_REPORT_ERROR) now (to prepare for the change) and switch 
to MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT later once they know they are 
ready.
And this is exactly what I think should be done by the PHP default so people 
don't have to go through this manually.


Moreover, I'd rather we get rid of the warning modes all together as they make 
the least sense to me.
Either you're handling the failures explicitly anyway and you use the silent 
mode, or you don't and want it to throw with the exception mode.


This is where you are missing a key component: I want to go through a phase 
where I will notice parts where manual error handling (or exception handling 
after the change) is missing without having my application abort. This is what 
warnings are good for.

To some of us this is a very valuable step when upgrading.

- Chris


I fully agree on this! Just changing one line of code is not
a good enough justification, since for legacy code that could
be a change that is not needed. Think one should ask if for a
stable released product, changing on line of code incurs cost
for testing and release management. So it's not just about
changing one line of code...

When it comes to warnings vs exceptions for me the main benefit
with warning & grace period is that when migrating legacy code
the timeline is not forced by the application stopping and one
can see all the warnings generated by the app and fixing them.

But of course I fully support the overall strategy on going for
exceptions in Mysli.

r//Björn L

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] [RFC]: Change Default mysqli Error Mode

[Christian Schneider]

Am 25.01.2021 um 16:59 schrieb G. P. B. :
> The BC break is totally minimal as it's a one line of code that needs to be 
> added (and for all intent and purposes should already be done).

That does not change the fact that it is a BC break which should be treated 
accordingly.

You are basically advocating people should add mysqli_report(MYSQLI_REPORT_OFF) 
to their setup which will totally defeat the purpose of the change.
Or (probably more what you have in mind) that they should add 
mysqli_report(MYSQLI_REPORT_ERROR) now (to prepare for the change) and switch 
to MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT later once they know they are 
ready.
And this is exactly what I think should be done by the PHP default so people 
don't have to go through this manually.

> Moreover, I'd rather we get rid of the warning modes all together as they 
> make the least sense to me.
> Either you're handling the failures explicitly anyway and you use the silent 
> mode, or you don't and want it to throw with the exception mode.

This is where you are missing a key component: I want to go through a phase 
where I will notice parts where manual error handling (or exception handling 
after the change) is missing without having my application abort. This is what 
warnings are good for.

To some of us this is a very valuable step when upgrading.

- Chris

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] [RFC]: Change Default mysqli Error Mode

[Benjamin Morel]

>
> Moreover, I'd rather we get rid of the warning modes all together as they
> make the least sense to me.
> Either you're handling the failures explicitly anyway and you use the
> silent mode, or you don't and want it to throw with the exception mode.
> The warning mode is literally the worst of both worlds, it doesn't fail on
> failure so you need to check for failure manually,
> and you need to suppress the warnings because otherwise you get false
> negatives.
>
> Best regards,
>
> George P. Banyard
>

I'd just like to say amen to that.

Please move as much code as possible to throwing exceptions, and get rid of
warnings. Most of the time when a warning is thrown in PHP, what you
actually want is to deal with it, or stop execution.

- Benjamin

Re: [PHP-DEV] [RFC]: Change Default mysqli Error Mode

[G. P. B.]

On Mon, 25 Jan 2021 at 15:41, Christian Schneider 
wrote:

> Am 21.01.2021 um 23:46 schrieb Kamil Tekiela :
> > In fact, I don't really see this as a major breaking change. If any
> project
> > really needs the mysqli errors silenced then they can just set the error
> > mode back to OFF. The change is aimed at new users, not existing
> projects.
> > Silenced error reporting was a source of confusion for plenty of
> beginners
> > who were unaware of the setting.
>
>
> As this definitely is a BC (applications stop running) I think we should
> go through a phase were it throws a warning instead of aborting, i.e.
> changing it to
> mysqli_report(MYSQLI_REPORT_ERROR);
> first and change it to throw an exception after a grace period.
>
> This would be in line with most other newly introduced exceptions for the
> default configuration.
> Plus this would most certainly help people who host other people's
> websites who want to upgrade to PHP 8.x.
>
> PS: IMHO The same should be (retroactively) done with round() and friends!
>
> - Chris
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>
>
The BC break is totally minimal as it's a one line of code that needs to be
added (and for all intent and purposes should already be done).

Moreover, I'd rather we get rid of the warning modes all together as they
make the least sense to me.
Either you're handling the failures explicitly anyway and you use the
silent mode, or you don't and want it to throw with the exception mode.
The warning mode is literally the worst of both worlds, it doesn't fail on
failure so you need to check for failure manually,
and you need to suppress the warnings because otherwise you get false
negatives.

Best regards,

George P. Banyard

Re: [PHP-DEV] [RFC]: Change Default mysqli Error Mode

[Christian Schneider]

Am 21.01.2021 um 23:46 schrieb Kamil Tekiela :
> In fact, I don't really see this as a major breaking change. If any project
> really needs the mysqli errors silenced then they can just set the error
> mode back to OFF. The change is aimed at new users, not existing projects.
> Silenced error reporting was a source of confusion for plenty of beginners
> who were unaware of the setting.


As this definitely is a BC (applications stop running) I think we should go 
through a phase were it throws a warning instead of aborting, i.e. changing it 
to
mysqli_report(MYSQLI_REPORT_ERROR);
first and change it to throw an exception after a grace period.

This would be in line with most other newly introduced exceptions for the 
default configuration.
Plus this would most certainly help people who host other people's websites who 
want to upgrade to PHP 8.x.

PS: IMHO The same should be (retroactively) done with round() and friends!

- Chris

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] [RFC]: Change Default mysqli Error Mode

[Nikita Popov]

On Thu, Jan 21, 2021 at 12:05 AM Kamil Tekiela  wrote:

> Hi Internals,
>
> Following my earlier announcement in
> https://wiki.php.net/rfc/improve_mysqli
> I decided to submit a dedicated RFC for the first proposal.
>
> This RFC proposes to change the default mysqli error reporting mode to
> exception mode. This change follows the same change in PDO. The RFC can be
> found here https://wiki.php.net/rfc/mysqli_default_errmode
>
> Kind Regards,
> Kamil Tekiela
>

Fully support this proposal. Throwing exceptions is the right default, it
matches what PDO does, and restoring the old behavior is one line of code.

Regards,
Nikita

Re: [PHP-DEV] [RFC]: Change Default mysqli Error Mode

[BohwaZ]

We should do the same with SQLite3 extension.

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] [RFC]: Change Default mysqli Error Mode

[Kamil Tekiela]

On Thu, 21 Jan 2021 at 21:51, Björn Larsson 
wrote:

> If I understand this correctly, does it imply that a lot of
> legacy code that works today, will not work any longer and
> needs to change? Like:
> if ("mysqlierror") {
> "error handling"
> }
>
> If so I think that like in other RFC's an investigation should
> be done on how much code that is affected to assess BC impact.
>

The code will still work as it did before, but if there is an error then an
exception will be thrown. This means that by default the custom error
handlers will become redundant. Generally, this should not be a problem,
but I can imagine that some projects heavily rely upon the manual error
checking.

Among the popular PHP projects that I know of, only CodeIgniter and
WordPress still use mysqli.
CI does offer PDO as an option but due to legacy reasons, many people still
chose mysqli. However, they already use the exception mode.
https://github.com/codeigniter4/CodeIgniter4/blob/d5673f4376dc981196607bc159368bee9990033c/system/Database/MySQLi/Connection.php#L94
WordPress ... the black sheep of PHP ... uses mysqli with a fallback to
mysql_* API. They use manual error handling, which would be replaced by
normal exceptions unless they silence it. They should modernize their
codebase at some point as they can't cling on to the past forever. For the
moment if they want to continue using manual error checking then they need
to explicitly disable mysqli errors without relying on the default setting.
That is only 1 additional line of code. This would be the same as what I
have done in PHP internal test cases. I don't see any problems there.

In fact, I don't really see this as a major breaking change. If any project
really needs the mysqli errors silenced then they can just set the error
mode back to OFF. The change is aimed at new users, not existing projects.
Silenced error reporting was a source of confusion for plenty of beginners
who were unaware of the setting.

Kind Regards,
Kamil Tekiela

Re: [PHP-DEV] [RFC]: Change Default mysqli Error Mode

[Björn Larsson]

Den 2021-01-21 kl. 00:04, skrev Kamil Tekiela:

Hi Internals,

Following my earlier announcement in https://wiki.php.net/rfc/improve_mysqli
I decided to submit a dedicated RFC for the first proposal.

This RFC proposes to change the default mysqli error reporting mode to
exception mode. This change follows the same change in PDO. The RFC can be
found here https://wiki.php.net/rfc/mysqli_default_errmode

Kind Regards,
Kamil Tekiela



If I understand this correctly, does it imply that a lot of
legacy code that works today, will not work any longer and
needs to change? Like:
if ("mysqlierror") {
"error handling"
}

If so I think that like in other RFC's an investigation should
be done on how much code that is affected to assess BC impact.

Btw, I liked your proposal on how to improve prepared statements
in mysqli a lot!

Regards //Björn Larsson

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] [RFC]: Change Default mysqli Error Mode

[Larry Garfield]

On Wed, Jan 20, 2021, at 5:04 PM, Kamil Tekiela wrote:
> Hi Internals,
> 
> Following my earlier announcement in https://wiki.php.net/rfc/improve_mysqli
> I decided to submit a dedicated RFC for the first proposal.
> 
> This RFC proposes to change the default mysqli error reporting mode to
> exception mode. This change follows the same change in PDO. The RFC can be
> found here https://wiki.php.net/rfc/mysqli_default_errmode
> 
> Kind Regards,
> Kamil Tekiela

Consistency FTW.  +1

--Larry Garfield

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php