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