[VOTE]: Change Default mysqli Error Mode

3 years ago by Christian Schneider — view source

unread

Am 11.02.2021 um 00:35 schrieb Kamil Tekiela tekiela246@gmail.com:

I have started voting on https://wiki.php.net/rfc/mysqli_default_errmode
The voting period is 2020-02-11 -- 2020-02-28

For the record: I do not think the wording fo the "Backward Incompatible Changes" section is appropriate, especially the only in "To bring back the old behaviour one needs to add only this line before instantiating mysqli class". This IMHO downplays the impact for people hosting legacy code.

Also: If there would have been an intermediate PHP version with default MYSQLI_REPORT_ERROR I would have voted "Yes", but in the current form I have to say "No".

Chris

3 years ago by Peter Bowyer — view source

unread

On Thu, 11 Feb 2021 at 13:31, Christian Schneider cschneid@cschneid.com
wrote:

For the record: I do not think the wording fo the "Backward Incompatible
Changes" section is appropriate, especially the only in "To bring back
the old behaviour one needs to add only this line before instantiating
mysqli class". This IMHO downplays the impact for people hosting legacy
code.

Also: If there would have been an intermediate PHP version with default
MYSQLI_REPORT_ERROR I would have voted "Yes", but in the current form I
have to say "No".

I voted "No" for the same reasons.

Peter

3 years ago by Craig Francis — view source

unread

[...] If there would have been an intermediate PHP version with default
MYSQLI_REPORT_ERROR I would have voted "Yes", but in the current form I
have to say "No".

I voted "No" for the same reasons.

If you want an intermediate PHP version, can it be done via Nikita's solution.

As in, for 8.1, issue a deprecation warning if mysqli_report() hasn't been called before opening a connection, then change the default to exceptions in 8.2.

This would get everyone (who upgrades to every 0.x) to notice that they haven't specified how they want reporting to work, and effectively gives them a year to notice and specify that choice.

Switching the default from OFF to ERROR to ERROR | STRICT isn't an obvious step, as that still means you're still having a single step to change from error reporting to exceptions.

Thanks,
Craig

3 years ago by Christian Schneider — view source

unread

Am 12.02.2021 um 15:30 schrieb Craig Francis craig@craigfrancis.co.uk:

Switching the default from OFF to ERROR to ERROR | STRICT isn't an obvious step, as that still means you're still having a single step to change from error reporting to exceptions.

It is the usual step for (almost) all BC breaking changes in PHP: Warn first (leaving time to find and adapt code), explode later.
The question is not if the change has to be made (or how big it is) but when (and how to find places to be changed).

Point in case: We just made the transition to PHP 8 and stuff like round(null) and abs(null) going directly from silence to Exception resulted in e.g. data importers suddenly stopping to work. Yes, each individual fix was easy but if there would have been a period where this is a warning we could have fixed those places without time pressure or a time window where users saw missing data.

Chris

3 years ago by Craig Francis — view source

unread

Am 12.02.2021 um 15:30 schrieb Craig Francis craig@craigfrancis.co.uk:

Switching the default from OFF to ERROR to ERROR | STRICT isn't an obvious step, as that still means you're still having a single step to change from error reporting to exceptions.

It is the usual step for (almost) all BC breaking changes in PHP: Warn first (leaving time to find and adapt code), explode later.
The question is not if the change has to be made (or how big it is) but when (and how to find places to be changed).

Oh, I completely agree, but Nikita's solution gives a better warning about the change to using exceptions.

As in, it would warn that mysqli_report() needs to be used, to specify the reporting mode you want, before the default changes.

Going via MYSQLI_REPORT_ERROR first, well, it doesn't exactly warn about that (it kinda moves forward a bit, but it doesn't warn that exceptions will be the new default in 8.2).

Craig

I know you know this already, I'm only including this as a note to self, to check how these changes will affect this example code:

$db = new mysqli('localhost', 'user', 'password', 'db');
$result = $db->query('SELECT COUNT(id) FROM invalid_table');
if ($result) {
    print_r(mysqli_fetch_assoc($result));
} else {
    print_r('Error');
}

Currently this just prints 'Error'.

Step 1) Using MYSQLI_REPORT_ERROR would add a warning; and then continue to print 'Error' - it's not really that different, maybe more error messages are shown or logged, but it still works in the same way.

Step 2) Using MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT is the big-ish change, because it will now throw an exception, and the error (which ideally won't happen in the first place) needs to be handled in a completely different way.

That said, we are talking about what happens when something goes wrong, and generally speaking, that shouldn't happen on an already running system... which is why I'm fairly happy with the change that Kamil is proposing.

And I should note, maybe phpMyAdmin is an oddity here, because I will make typos, and those need to be handled properly, and, erm, phpMyAdmin doesn't currently use mysqli_report().

3 years ago by Benjamin Eberlei — view source

unread

Hi internals,

I have started voting on https://wiki.php.net/rfc/mysqli_default_errmode
The voting period is 2020-02-11 -- 2020-02-28

While the change in itself is something I want to see, I voted no on this
proposal because its a BC break that will break a lot of software out there
and there is no attempt at deprecating the old way and guiding users to the
new behavior.

I wish this would be more gradual by requiring the reporting setting in the
constructor, throwing a deprecation warning when none is provided and then
changing this behavior in 9.0

Kind Regards,
Kamil

3 years ago by johannes@schlueters.de — view source

unread

Hi,

I have started voting on
https://wiki.php.net/rfc/mysqli_default_errmode
The voting period is 2020-02-11 -- 2020-02-28

Sorry, I didn't see this before. I agree that Exceptions are the way to
go and it were good if that had been done decades ago where we argued a
lot whether exceptions should be default.

But now there is tons of valid and good code depending on the default
behavior. Also many tutorials, books, ... don't use the exception
paths, but "classic" error handling. All those will become incorrect!

Doing this change will break such could and that breakage will only be
in an error case, thus many users won't notice this for quite a while.

This requires a smarter migration way. (The spontanous idea might be to
give a deprecation note if users don't set a mode at all and after a
while switch default ... but that isn't unproblematic either)

johannes

3 years ago by Kamil Tekiela — view source

unread

Hi Internals,

The RFC was accepted with 20 Yes and 9 No votes.

During the voting phase, a discussion has arisen whether this change should
land in PHP 8.1 or 8.2 following a deprecation phase. At present, the RFC
was accepted to include it in 8.1, but if someone has a proposal on how to
introduce a deprecation phase I would appreciate it if they could make a
counter-proposal to make the adjustment. A deprecation error was not part
of the initial RFC.

Kind Regards,
Kamil

3 years ago by Craig Francis — view source

unread

if someone has a proposal on how to introduce a deprecation phase

If the extra step is necessary... I liked Nikita's suggestion to "issue a
deprecation warning when creating a connection without mysqli_report()
having been explicitly called beforehand".

This will hopefully prompt old systems, who want to keep the old behaviour,
to proactively add mysqli_report(MYSQLI_REPORT_OFF).

Keeping in mind that anyone using exceptions today is already using this
function; and when the default has changed, we can drop this check,
so mysqli_report() is no longer required by those using exceptions.

Chris, while I liked your thinking, and appreciate your caution about the
change, I'm not sure MYSQLI_REPORT_ERROR would clearly show exceptions
will be the new default.

Craig