Re: [VOTE] Allowing use of exceptions in the engine
On Sun, Dec 8, 2013 at 11:48 AM, Nikita Popov <nikita.ppv@gmail.com> wrote:
> On Sat, Dec 7, 2013 at 6:05 PM, Ferenc Kovacs <tyra3l@gmail.com> wrote:
>
> > 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
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
Thread (19 messages)