Everything
php
Learn Community Releases The PHP Project
Home » Project » Newsgroups » php.internals » [RFC] [Discussion] SQLite3: remove warnings, move to exceptions »

[RFC] [Discussion] SQLite3: remove warnings, move to exceptions

php.internals

BohwaZ/PHP

3 years ago
Kia ora, I am proposing that the SQLite3 extension stops using warnings for errors and instead throws exceptions by default: https://wiki.php.net/rfc/sqlite3_exceptions Whether we just deprecate warnings first and default to exceptions, and then remove warnings altogether in a later version, or just disable warnings directly, is probably the important part of the RFC. My personal point of view is that modern code probably already have enabled exceptions in the class, or is transforming warnings into exceptions using an error handler. If not, it's relatively easy to set a exception handler to dismiss the new exceptions if you don't want to see them. So going throught depreciation might not be necessary, but it's open for discussion. Feedback is welcome :) Thanks.

Christian Schneider

3 years ago
Am 24.10.2022 um 04:56 schrieb BohwaZ <php@bohwaz.net>:
> I am proposing that the SQLite3 extension stops using warnings for > errors and instead throws exceptions by default: > > https://wiki.php.net/rfc/sqlite3_exceptions > > Whether we just deprecate warnings first and default to exceptions, and > then remove warnings altogether in a later version, or just disable > warnings directly, is probably the important part of the RFC. > > My personal point of view is that modern code probably already have > enabled exceptions in the class, or is transforming warnings into > exceptions using an error handler. > > If not, it's relatively easy to set a exception handler to dismiss the > new exceptions if you don't want to see them. > > So going throught depreciation might not be necessary, but it's open > for discussion.
To no surprise for anyone reading this list I'm going to strongly advocate for going through a deprecation phase to ease migration of code. As often the code changes needed might not be big but you still have to a) notice all the necessary changes (the SQLite exceptions might just be one of many changes in the new PHP version) and b) adapting the code should be decoupled from the PHP upgrade. Both of these goals are IMHO well handled by a deprecation phase. Regards, - Chris

Kamil Tekiela

3 years ago
Hi, I don't think we should be removing such large chunk of functionality in a minor version. If you want to change the default to throw exceptions, that's ok. But I wouldn't remove the method in 8.3 or 8.4 and only remove it in 9.0. What is the advantage to removing the Warning mode? Regards, Kamil

Larry Garfield

3 years ago
On Sun, Oct 23, 2022, at 9:56 PM, BohwaZ wrote:
> Kia ora, > > I am proposing that the SQLite3 extension stops using warnings for > errors and instead throws exceptions by default: > > https://wiki.php.net/rfc/sqlite3_exceptions > > Whether we just deprecate warnings first and default to exceptions, and > then remove warnings altogether in a later version, or just disable > warnings directly, is probably the important part of the RFC. > > My personal point of view is that modern code probably already have > enabled exceptions in the class, or is transforming warnings into > exceptions using an error handler. > > If not, it's relatively easy to set a exception handler to dismiss the > new exceptions if you don't want to see them. > > So going throught depreciation might not be necessary, but it's open > for discussion. > > Feedback is welcome :) > > Thanks.
As others have said, the correct, responsible way to do this would be to make it gradual and keep warnings working until 9.0. I would recommend: 1. 8.3, enableExceptions(false) raises E_DEPRECATED. 2. 9.0 enableExceptions() defaults to true, and calling it with false is an Error. (Calling it with true still works, but is a no-op.) 3. 10.0 Remove enableExceptions() entirely. The behavior then only changes on major versions. I think we already did this for PDO, didn't we? It makes sense to do it for SQLite, too. --Larry Garfield

BohwaZ/PHP

3 years ago
> As others have said, the correct, responsible way to do this would be > to make it gradual and keep warnings working until 9.0. I would > recommend: > > 1. 8.3, enableExceptions(false) raises E_DEPRECATED. > 2. 9.0 enableExceptions() defaults to true, and calling it with false > is an Error. (Calling it with true still works, but is a no-op.) 3. > 10.0 Remove enableExceptions() entirely. > > The behavior then only changes on major versions.
Thank you, I updated the RFC in that direction. The sad part is that it will take a very long time before PHP 10.0 is released :(
> I think we already did this for PDO, didn't we? It makes sense to do > it for SQLite, too.
Slightly different: https://wiki.php.net/rfc/pdo_default_errmode PDO made the change to throwing exceptions without going through a deprecation. Maybe what I would prefer for SQLite3 is a middle-ground: * we keep ''enableExceptions(false)'' with no plan to remove it -> thinking about it, it doesn't really matter if you prefer warnings, and it would be easy to keep it that way * but we make SQLite3 throw exceptions by default directly in PHP 8.3, like it was done with PDO 2 years ago. That way you can keep the same behaviour as before just by adding enableExceptions(false) to your code, and it will not break. That seems like a better compromise no?

Christoph Becker

3 years ago
On 25.10.2022 at 14:10, BohwaZ wrote:
>> As others have said, the correct, responsible way to do this would be >> to make it gradual and keep warnings working until 9.0. I would >> recommend: >> >> 1. 8.3, enableExceptions(false) raises E_DEPRECATED. >> 2. 9.0 enableExceptions() defaults to true, and calling it with false >> is an Error. (Calling it with true still works, but is a no-op.) 3. >> 10.0 Remove enableExceptions() entirely. >> >> The behavior then only changes on major versions. > > Thank you, I updated the RFC in that direction. The sad part is that it > will take a very long time before PHP 10.0 is released :(
Cf. <https://wiki.php.net/rfc/deprecate-and-remove-intl_idna_variant_2003>, where we changed the behavior in a minor release. Not quite the same as in this case, but might still be a precendence.
>> I think we already did this for PDO, didn't we? It makes sense to do >> it for SQLite, too. > > Slightly different: > https://wiki.php.net/rfc/pdo_default_errmode > > PDO made the change to throwing exceptions without going through a > deprecation. > > Maybe what I would prefer for SQLite3 is a middle-ground: > * we keep ''enableExceptions(false)'' with no plan to remove it -> > thinking about it, it doesn't really matter if you prefer warnings, > and it would be easy to keep it that way
No, please get rid of the choice for users whether there is an exception or a warning. As it is now, it is *very* hard to document the behavior; what to put in the errors/exceptions section, for instance. Several other extensions have the same issue, and we should try to fade that out.
> * but we make SQLite3 throw exceptions by default directly in PHP 8.3, > like it was done with PDO 2 years ago. > > That way you can keep the same behaviour as before just by adding > enableExceptions(false) to your code, and it will not break. > > That seems like a better compromise no?
-- Christoph M. Becker

Larry Garfield

3 years ago
On Tue, Oct 25, 2022, at 7:10 AM, BohwaZ wrote:
>> As others have said, the correct, responsible way to do this would be >> to make it gradual and keep warnings working until 9.0. I would >> recommend: >> >> 1. 8.3, enableExceptions(false) raises E_DEPRECATED. >> 2. 9.0 enableExceptions() defaults to true, and calling it with false >> is an Error. (Calling it with true still works, but is a no-op.) 3. >> 10.0 Remove enableExceptions() entirely. >> >> The behavior then only changes on major versions. > > Thank you, I updated the RFC in that direction. The sad part is that it > will take a very long time before PHP 10.0 is released :(
Yes, but as of 9.0 it won't do anything. The cost of leaving it there for an extra few years as a no-op is very small compared to the benefit of an easier upgrade path. This way, people can leave enableExceptions(true) in their codebase and it will work without difference from 9.4 back to... ever? Those are the people doing it right. Be nice to those people. :-) It would only be something they need care about in 10.0, by which point dropping support for PHP 8 and earlier would be a no-brainer.
>> I think we already did this for PDO, didn't we? It makes sense to do >> it for SQLite, too. > > Slightly different: > https://wiki.php.net/rfc/pdo_default_errmode > > PDO made the change to throwing exceptions without going through a > deprecation.
Well that was silly.
> Maybe what I would prefer for SQLite3 is a middle-ground: > * we keep ''enableExceptions(false)'' with no plan to remove it -> > thinking about it, it doesn't really matter if you prefer warnings, > and it would be easy to keep it that way > * but we make SQLite3 throw exceptions by default directly in PHP 8.3, > like it was done with PDO 2 years ago. > > That way you can keep the same behaviour as before just by adding > enableExceptions(false) to your code, and it will not break. > > That seems like a better compromise no?
No, because the goal should be to get everything consistent on using exceptions always. The mixed-mode error handling is a mess that benefits no one and makes collaboration harder. Moving toward all-exceptions is the right way to do it, just in a graceful fashion. --Larry Garfield

Derick Rethans

3 years ago
On 25 October 2022 14:10:14 CEST, BohwaZ <php@bohwaz.net> wrote:
>* but we make SQLite3 throw exceptions by default directly in PHP 8.3, > like it was done with PDO 2 years ago.
I am not in favour, as it would mean that existing code suddenly may start breaking, as exceptions aren't expected (and hence not catered for). We can't do that in an 8.x release.

Christoph Becker

3 years ago
On 26.10.2022 at 08:57, Derick Rethans wrote:
> On 25 October 2022 14:10:14 CEST, BohwaZ <php@bohwaz.net> wrote: > >> * but we make SQLite3 throw exceptions by default directly in PHP 8.3, >> like it was done with PDO 2 years ago. > > I am not in favour, as it would mean that existing code suddenly may start breaking, as exceptions aren't expected (and hence not catered for). We can't do that in an 8.x release.
I'm not in favor of this either, but we did it for PDO (okay, PHP 8.0) and mysqli (PHP 8.1), and it can be counteracted with a single statement. So yes, it would constitute a BC break, but that could be solved easily (if all else fails, you could auto_prepend_file).
-- Christoph M. Becker

BohwaZ/PHP

3 years ago
Hi, I updated the RFC I added a second proposal where the only change would be to throw exceptions by default in 8.3 instead of 9.0: https://wiki.php.net/rfc/sqlite3_exceptions I also changed the plan for 9.0: "Calling SQLite3::enableExceptions(true) raises E_DEPRECATED, to alert that the method will get removed" As for people using enableExceptions(true) there was previously no depreciation notice, and the method would just "disappear" without any prior notice.