Hi internals!
I opened the vote on the "Exceptions in the engine" RFC:
https://wiki.php.net/rfc/engine_exceptions#vote
The vote has three options, "Yes", "No" and "Yes, without
E_RECOVERABLE_ERROR changes". The last option is a version of the proposal
without BC issues.
Some further notes:
- I will not be including something like BaseException. Introducing it for
this purpose seems like bad design and will be very hard to get rid of in
the future. As the proposal (without recoverable errors) does not break BC
[1] even without it, I don't see a reason to introduce it. - Some people suggest to use different subclasses of EngineException for
different error types. I'm not against that, but I think it's okay to do
that in a separate proposal, if someone can come up with a good selection
of exception classes. It's much easier (and does not break BC) to add
subclassing later, than to add suboptimal subclass types now and try to fix
something like the SPL exception mess later. - The suggested uncaught exception error message change is just a bit of
cosmetics (and not directly related to this proposal), so I'd like to do
that separately.
And regarding the vote: If you are in favor of the proposal in general, but
want to have it in PHP 6 rather than PHP 5.6, then vote "No" here. If it
fails now, someone can revive this once the time for PHP 6 has come.
Thanks,
Nikita
[1]: We don't have a formal definition for this, but it seems to be general
consensus that relaxing error conditions does not constitute a backwards
compatibility break.
Hi internals!
I opened the vote on the "Exceptions in the engine" RFC:
https://wiki.php.net/rfc/engine_exceptions#voteThe vote has three options, "Yes", "No" and "Yes, without
E_RECOVERABLE_ERRORchanges". The last option is a version of the proposal
without BC issues.Some further notes:
- I will not be including something like BaseException. Introducing it for
this purpose seems like bad design and will be very hard to get rid of in
the future. As the proposal (without recoverable errors) does not break BC
[1] even without it, I don't see a reason to introduce it.- Some people suggest to use different subclasses of EngineException for
different error types. I'm not against that, but I think it's okay to do
that in a separate proposal, if someone can come up with a good selection
of exception classes. It's much easier (and does not break BC) to add
subclassing later, than to add suboptimal subclass types now and try to fix
something like the SPL exception mess later.- The suggested uncaught exception error message change is just a bit of
cosmetics (and not directly related to this proposal), so I'd like to do
that separately.And regarding the vote: If you are in favor of the proposal in general, but
want to have it in PHP 6 rather than PHP 5.6, then vote "No" here. If it
fails now, someone can revive this once the time for PHP 6 has come.Thanks,
Nikita[1]: We don't have a formal definition for this, but it seems to be general
consensus that relaxing error conditions does not constitute a backwards
compatibility break.
personally I have seen catch-all blocks in the wild (try {//do something}
catch(Exception $e) {//do nothing}), which would behave differently (and
some of them would screw something up instead of terminating the app) if
the EngineException is a subclass of Exception.
I can see myself supporting this proposal for 5.6, if we can have it done
in a truly BC-safe manner, but I can understand, if the required
compromises for that would make the feature too "clunky", so maybe it would
be better to introduce it in a major version.
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
personally I have seen catch-all blocks in the wild (try {//do something}
catch(Exception $e) {//do nothing}), which would behave differently (and
some of them would screw something up instead of terminating the app) if
the EngineException is a subclass of Exception.
I can see myself supporting this proposal for 5.6, if we can have it done
in a truly BC-safe manner, but I can understand, if the required
compromises for that would make the feature too "clunky", so maybe it would
be better to introduce it in a major version.
I'm not sure I see how catch-all blocks relate to BC-safety as far as
E_ERROR is concerned. If an engine exception is accidentally caught by a
catch-all block, it means that previously it was throwing a fatal error,
which means that your code didn't work anyway - in all likeliness you were
getting a WSOD or ISE. The catch-all block will not break the code (it is
already broken), it will only change the way in which it fails (or prevent
it from failing altogether). Of course, unintentionally missing an error
is an issue, but it's not an issue of BC.
Nikita
personally I have seen catch-all blocks in the wild (try {//do something}
catch(Exception $e) {//do nothing}), which would behave differently (and
some of them would screw something up instead of terminating the app) if
the EngineException is a subclass of Exception.
I can see myself supporting this proposal for 5.6, if we can have it done
in a truly BC-safe manner, but I can understand, if the required
compromises for that would make the feature too "clunky", so maybe it
would
be better to introduce it in a major version.I'm not sure I see how catch-all blocks relate to BC-safety as far as
E_ERRORis concerned. If an engine exception is accidentally caught by a
catch-all block, it means that previously it was throwing a fatal error,
which means that your code didn't work anyway - in all likeliness you were
getting a WSOD or ISE. The catch-all block will not break the code (it is
already broken), it will only change the way in which it fails (or prevent
it from failing altogether). Of course, unintentionally missing an error
is an issue, but it's not an issue of BC.Nikita
I think that the problem could be code like this:
try {
// do same stuff
} catch (Exception $e) {}
delete_something_important();
I haven't tested the patch but from what I read it seems that
delete_something_important() would be now called even if there was a fatal
error in the try block?
In addition there is a small BC break that should be probably mentioned in
RFC. All apps with defined EngineException will not work. I actually found
one open source app. It's the first version of ORM Propel:
https://github.com/propelorm/Propel/blob/master/generator/lib/exception/EngineException.php
It looks that it's still used quite a few users (even there is a version 2
that resolved this using ns). I believe that there can be more closed
source apps so it should be at least mentioned in RFC...
Jakub
I opened the vote on the "Exceptions in the engine" RFC:
https://wiki.php.net/rfc/engine_exceptions#voteThe vote has three options, "Yes", "No" and "Yes, without
E_RECOVERABLE_ERRORchanges". The last option is a version of the proposal
without BC issues.And regarding the vote: If you are in favor of the proposal in general, but
want to have it in PHP 6 rather than PHP 5.6, then vote "No" here. If it
fails now, someone can revive this once the time for PHP 6 has come.
To be clear: I've voted -1 for exactly this reason, and this reason
alone. I don't think implementing this piecemeal (without the
E_RECOVERABLE_ERROR changes) is the right way to go — I would prefer
to have the whole thing as part of a 6.0 release, rather than
potentially confusing users with a partial implementation.
Notes on the notes:
- I will not be including something like BaseException. Introducing it for
this purpose seems like bad design and will be very hard to get rid of in
the future. As the proposal (without recoverable errors) does not break BC
[1] even without it, I don't see a reason to introduce it.
+1. I think I said last time that it felt like the
mysql_real_escape_string() of exception API design. :)
- Some people suggest to use different subclasses of EngineException for
different error types. I'm not against that, but I think it's okay to do
that in a separate proposal, if someone can come up with a good selection
of exception classes. It's much easier (and does not break BC) to add
subclassing later, than to add suboptimal subclass types now and try to fix
something like the SPL exception mess later.
Also +1.
Adam
To be clear: I've voted -1 for exactly this reason, and this reason
alone. I don't think implementing this piecemeal (without the
E_RECOVERABLE_ERRORchanges) is the right way to go — I would prefer
to have the whole thing as part of a 6.0 release, rather than
potentially confusing users with a partial implementation.
Maybe I'm misunderstanding you here, but if you don't want it piecemeal,
surely just vote "Yes"? Or are you saying you want it in 6.0, and also
that it shouldn't be piecemeal?
--
Andrea Faulds
http://ajf.me/
To be clear: I've voted -1 for exactly this reason, and this reason
alone. I don't think implementing this piecemeal (without the
E_RECOVERABLE_ERRORchanges) is the right way to go — I would prefer
to have the whole thing as part of a 6.0 release, rather than
potentially confusing users with a partial implementation.Maybe I'm misunderstanding you here, but if you don't want it piecemeal,
surely just vote "Yes"? Or are you saying you want it in 6.0, and also that
it shouldn't be piecemeal?
I guess I explained that badly. :) The latter: I want it in 6.0, and
don't think it should be piecemeal.
Adam
I opened the vote on the "Exceptions in the engine" RFC:
https://wiki.php.net/rfc/engine_exceptions#voteThe vote has three options, "Yes", "No" and "Yes, without
E_RECOVERABLE_ERRORchanges". The last option is a version of the
proposal
without BC issues.And regarding the vote: If you are in favor of the proposal in general,
but
want to have it in PHP 6 rather than PHP 5.6, then vote "No" here. If it
fails now, someone can revive this once the time for PHP 6 has come.To be clear: I've voted -1 for exactly this reason, and this reason
alone. I don't think implementing this piecemeal (without the
E_RECOVERABLE_ERRORchanges) is the right way to go — I would prefer
to have the whole thing as part of a 6.0 release, rather than
potentially confusing users with a partial implementation.
If you're concerned about E_RECOVERABLE_ERROR in particular I should point
out that there are very few recoverable fatals in the engine. Actually, the
only common recoverable error is the typehint violation error. The others
are along the lines of "Instantiation of 'Closure' is not allowed" or
"Closure object cannot have properties" - not something you usually
encounter. For a full list see
http://lxr.php.net/search?q=E_RECOVERABLE_ERROR&defs=&refs=&path=Zend&hist=&project=PHP_TRUNK.
So I don't think that E_RECOVERABLE_ERROR is a particularly large loss in
itself - but the "partial implementation" argument presumably still stands
even without it (won't be able to port all fatal errors at once, after all.)
Nikita
Nikita,
Apologies for not having followed the discussion a few weeks ago but I
think this proposal can't be accepted as it is even if everyone likes it.
It breaks a very basic design concept in the way both the engine code as
well as extension code is written - execution will never resume beyond an
E_ERROR.
Allowing execution to resume (through the use of exceptions) opens a huge
can of worms, and sets expectations we can't meet. In fact, the main
difference between E_ERROR and E_WARNING is the recoverability, so this is
not some small detail we can fix - we simply can't assume execution can
continue after an E_ERROR is triggered, because E_ERROR in its very
essence implies no-resumed-execution. That's also the reason error
handlers (that existed before exceptions did) don't handle E_ERROR.
I don't think a vote makes a lot of sense here, since it's asking people
whether they're in favor of the concept of being able to recover from any
kind of error. That's the wrong question (to which, by the way, I'd vote
'yes'). The real question is whether people are OK that the engine will
start randomly crashing through the use of standard PHP code, which is
pretty much out of the question.
If there are E_ERROR's which you think are erroneously tagged as E_ERROR's
and should in fact be E_RECOVERABLE_ERROR, we should change them, rather
than assuming that E_ERRORs can be tagged as recoverable wholesale...
Disclaimer: Perhaps I'm missing something. I also asked Dmitry to take a
closer look at this...
Zeev
-----Original Message-----
From: Nikita Popov [mailto:nikita.ppv@gmail.com]
Sent: Saturday, December 07, 2013 2:58 PM
To: PHP internals
Subject: [PHP-DEV] [VOTE] Allowing use of exceptions in the engineHi internals!
I opened the vote on the "Exceptions in the engine" RFC:
https://wiki.php.net/rfc/engine_exceptions#voteThe vote has three options, "Yes", "No" and "Yes, without
E_RECOVERABLE_ERRORchanges". The last option is a version of the
proposal without BC issues.Some further notes:
- I will not be including something like BaseException. Introducing it
for this
purpose seems like bad design and will be very hard to get rid of in the
future. As the proposal (without recoverable errors) does not break BC
[1]
even without it, I don't see a reason to introduce it.- Some people suggest to use different subclasses of EngineException
for
different error types. I'm not against that, but I think it's okay to do
that in a
separate proposal, if someone can come up with a good selection of
exception classes. It's much easier (and does not break BC) to add
subclassing later, than to add suboptimal subclass types now and try to
fix
something like the SPL exception mess later.- The suggested uncaught exception error message change is just a bit
of
cosmetics (and not directly related to this proposal), so I'd like to do
that
separately.And regarding the vote: If you are in favor of the proposal in general,
but
want to have it in PHP 6 rather than PHP 5.6, then vote "No" here. If it
fails
now, someone can revive this once the time for PHP 6 has come.Thanks,
Nikita[1]: We don't have a formal definition for this, but it seems to be
general
consensus that relaxing error conditions does not constitute a backwards
compatibility break.
Nikita,
Apologies for not having followed the discussion a few weeks ago but I
think this proposal can't be accepted as it is even if everyone likes it.
It breaks a very basic design concept in the way both the engine code as
well as extension code is written - execution will never resume beyond an
E_ERROR.Allowing execution to resume (through the use of exceptions) opens a huge
can of worms, and sets expectations we can't meet. In fact, the main
difference betweenE_ERRORandE_WARNINGis the recoverability, so this is
not some small detail we can fix - we simply can't assume execution can
continue after anE_ERRORis triggered, becauseE_ERRORin its very
essence implies no-resumed-execution. That's also the reason error
handlers (that existed before exceptions did) don't handle E_ERROR.I don't think a vote makes a lot of sense here, since it's asking people
whether they're in favor of the concept of being able to recover from any
kind of error. That's the wrong question (to which, by the way, I'd vote
'yes'). The real question is whether people are OK that the engine will
start randomly crashing through the use of standard PHP code, which is
pretty much out of the question.If there are E_ERROR's which you think are erroneously tagged as E_ERROR's
and should in fact be E_RECOVERABLE_ERROR, we should change them, rather
than assuming that E_ERRORs can be tagged as recoverable wholesale...
Hi Zeev!
I think this is a misunderstanding... I'm not suggesting to simply let the
engine continue after an E_ERROR - as you pointed out, that would likely
just crash it a few lines further down.
This RFC is mainly a policy RFC: The goal is to allow the use of exceptions
in the engine and to allow changing existing fatal errors to exceptions.
The change from fatal errors to exceptions needs to happen manually, by
adjusting the surrounding code to support continued execution (usually that
means freeing resources + returning). A lot of fatal errors are easy to
change, others are very hard or impossible. Changing fatal errors to
exceptions rather than recoverable errors is both more useful to the end
user and technically easier (as recoverable errors need to continue
execution in the same codepath, which is often a lot harder to implement
and find appropriate semantics for), which is why I'm suggesting this
particular course of action.
So, basically what I'm suggest is what you say in the last paragraph, just
going directly to exceptions rather than converting to E_RECOVERABLE_ERROR
:)
Hope this is a bit clearer.
Nikita
Hi Zeev!
I think this is a misunderstanding... I'm not suggesting to simply let the
engine continue after anE_ERROR- as you pointed out, that would likely
just crash it a few lines further down.This RFC is mainly a policy RFC: The goal is to allow the use of
exceptions in the engine and to allow changing existing fatal errors to
exceptions. The change from fatal errors to exceptions needs to happen
manually, by adjusting the surrounding code to support continued execution
(usually that means freeing resources + returning). A lot of fatal errors
are easy to change, others are very hard or impossible. Changing fatal
errors to exceptions rather than recoverable errors is both more useful to
the end user and technically easier (as recoverable errors need to continue
execution in the same codepath, which is often a lot harder to implement
and find appropriate semantics for), which is why I'm suggesting this
particular course of action.So, basically what I'm suggest is what you say in the last paragraph, just
going directly to exceptions rather than converting toE_RECOVERABLE_ERROR
:)Hope this is a bit clearer.
OK, got you. Not using exceptions in the engine was also a design
decision (so that we don't force OO concepts on non-OO people) - need to
think if it still makes sense and get back to the vote.
Thanks!
Zeev
Hi internals!
I opened the vote on the "Exceptions in the engine" RFC:
https://wiki.php.net/rfc/engine_exceptions#voteThe vote has three options, "Yes", "No" and "Yes, without
E_RECOVERABLE_ERRORchanges". The last option is a version of the proposal
without BC issues.
Wow. This is one of the bigger language changes I have seen proposed to date.
I see the framework guys jumping in on this right away wanting pretty
OO interface of course - but hold your horses a bit.
This is a very fundamental change in the language and a vote should
not be casted lightly.
The recoverable error cases is something I argued when breaking type
hinting was decided to be E_REOCVERABLE_ERROR for reasons that still
baffle me (and depending if its extension code or userland code,
effectively continues current context or not, since internals will
most likely bail out on zpp) - but now we are talking about about
changing something much bigger then that...
If this gets accepted, what is the reason we have these error levels
at all, and shouldn't E_* friends be rewritten into more convenient
logging framework and remove the association with "errors"?
-Hannes
Hi Nikita,
The idea makes sense, but it really changes the fundamental design decision.
For now the patch misses more than 300 places where you'll have to find a
way to recover from a error state.
As you said, it's hardly possible in some cases (e.g. memory_limit
overflow).
Anyway, I'm not sure if the effort and risks of bugs and breaks cost the
ability of writing "dirty" code.
Thanks. Dmitry.
Hi internals!
I opened the vote on the "Exceptions in the engine" RFC:
https://wiki.php.net/rfc/engine_exceptions#voteThe vote has three options, "Yes", "No" and "Yes, without
E_RECOVERABLE_ERRORchanges". The last option is a version of the proposal
without BC issues.Some further notes:
- I will not be including something like BaseException. Introducing it for
this purpose seems like bad design and will be very hard to get rid of in
the future. As the proposal (without recoverable errors) does not break BC
[1] even without it, I don't see a reason to introduce it.- Some people suggest to use different subclasses of EngineException for
different error types. I'm not against that, but I think it's okay to do
that in a separate proposal, if someone can come up with a good selection
of exception classes. It's much easier (and does not break BC) to add
subclassing later, than to add suboptimal subclass types now and try to fix
something like the SPL exception mess later.- The suggested uncaught exception error message change is just a bit of
cosmetics (and not directly related to this proposal), so I'd like to do
that separately.And regarding the vote: If you are in favor of the proposal in general, but
want to have it in PHP 6 rather than PHP 5.6, then vote "No" here. If it
fails now, someone can revive this once the time for PHP 6 has come.Thanks,
Nikita[1]: We don't have a formal definition for this, but it seems to be general
consensus that relaxing error conditions does not constitute a backwards
compatibility break.
hi,
Hi internals!
I opened the vote on the "Exceptions in the engine" RFC:
https://wiki.php.net/rfc/engine_exceptions#voteThe vote has three options, "Yes", "No" and "Yes, without
E_RECOVERABLE_ERRORchanges". The last option is a version of the proposal
without BC issues.Some further notes:
- I will not be including something like BaseException. Introducing it for
this purpose seems like bad design and will be very hard to get rid of in
the future. As the proposal (without recoverable errors) does not break BC
[1] even without it, I don't see a reason to introduce it.- Some people suggest to use different subclasses of EngineException for
different error types. I'm not against that, but I think it's okay to do
that in a separate proposal, if someone can come up with a good selection
of exception classes. It's much easier (and does not break BC) to add
subclassing later, than to add suboptimal subclass types now and try to fix
something like the SPL exception mess later.- The suggested uncaught exception error message change is just a bit of
cosmetics (and not directly related to this proposal), so I'd like to do
that separately.And regarding the vote: If you are in favor of the proposal in general, but
want to have it in PHP 6 rather than PHP 5.6, then vote "No" here. If it
fails now, someone can revive this once the time for PHP 6 has come.
I do not mind having it in 5.x or 6. but I do mind introducing
exceptions in the core without defining what we want. Baby steps are
nice but where do we stop at this stage then? Can we use exceptions in
extensions as well? Or in other parts of the engine? This is a very
good move forward but also a very critical move. We need to clarify
how we do it, where it can be done, naming or how we want them
exactly, etc.
That's why I can't vote yes with this exact proposal (and its state).
Cheers,
Pierre
@pierrejoye | http://www.libgd.org
I do not mind having it in 5.x or 6. but I do mind introducing
exceptions in the core without defining what we want. Baby steps are
nice but where do we stop at this stage then? Can we use exceptions in
extensions as well? Or in other parts of the engine? This is a very
good move forward but also a very critical move. We need to clarify
how we do it, where it can be done, naming or how we want them
exactly, etc.That's why I can't vote yes with this exact proposal (and its state).
Is there something particular that you would like to have clarified or
specified? I tried to keep the scope narrow - only engine and only fatal
errors, but if you wish to have stricter specification on some parts,
please say so.
About your particular questions:
Can we use exceptions in extensions as well?
This RFC is only about the engine - it does not affect our standard library
exts. The rules there stay the same: Functions throw warnings/notices,
methods may throw exceptions.
Or in other parts of the engine?
What do you mean by "other parts"? I'm referring to the Zend engine as a
whole, though primarily the executor, because converting compile-time
errors will require more careful planning.
Thanks,
Nikita
I do not mind having it in 5.x or 6. but I do mind introducing
exceptions in the core without defining what we want. Baby steps are
nice but where do we stop at this stage then? Can we use exceptions in
extensions as well? Or in other parts of the engine? This is a very
good move forward but also a very critical move. We need to clarify
how we do it, where it can be done, naming or how we want them
exactly, etc.That's why I can't vote yes with this exact proposal (and its state).
Is there something particular that you would like to have clarified or
specified? I tried to keep the scope narrow - only engine and only fatal
errors, but if you wish to have stricter specification on some parts,
please say so.
I would prefer to see introduce exceptions in the whole core and clearly
define how we want them or where. Adding very good documentation is also a
requirement (see Zeev's reply).
Not doing may lead to half backed support with everyone doing its little
sauce and will end as a big mess. We should avoid that.
About your particular questions:
Can we use exceptions in extensions as well?
This RFC is only about the engine - it does not affect our standard
library exts. The rules there stay the same: Functions throw
warnings/notices, methods may throw exceptions.
Right, everyone does his thing and it is not confusing and inconsistent.
Let solve the whole thing instead of a rather limited and not so useful.
Or in other parts of the engine?
What do you mean by "other parts"? I'm referring to the Zend engine as a
whole, though primarily the executor, because converting compile-time
errors will require more careful planning.
As of other errors, functions, operators, etc.
Cheers,
Pierre
The vote on the Engine Exceptions RFC ended with 24 in favor, 21 against
and 1 in the middle. As this vote had a 2/3-majority requirement, this
means that the RFC is declined.
You can find the voting results on
https://wiki.php.net/rfc/engine_exceptions#vote.
Nikita