cakephp: What are we going to do about PHP7 "throwable"

This is a (multiple allowed):

  • bug
  • enhancement
  • feature-discussion (RFC)
  • CakePHP Version: 3.6.10
  • Platform and Target: Apache 2.4, PHP 7.0

What you did

Keep running into cases where CakePHP is not handling PHP 7’s new “Error” and “Throwable” exceptions.

Expected Behavior

Cells trigger a server 500 if there is a Error thrown during render. Transactions not commit and leak if there is an Error class thrown. Basically, anywhere there is a catch(Exception $ex) the code does not work as expected in PHP 7.

Actual Behavior

PHP 7 introduced a new error type Error, and gave Error and Exception a common interface Throwable.

All try/catch methods that do a broad catch all catch(Exception $e) should be doing catch(\Throwable $e) in PHP 7.

I don’t know how to address this issue in a backward compatible way.

Maybe use a global function that wraps a closure in the proper try/catch type for the current PHP version?

As this been discussed anywhere else by the Cake team?

About this issue

  • Original URL
  • State: closed
  • Created 8 years ago
  • Comments: 16 (16 by maintainers)

Most upvoted comments

500 errors are a good thing! How would you otherwise know that you have a syntax error, an uncommitted file, a type mismatch, etc?

If we catch the errors and deprive the users from things as basic as a stacktrace and some log output, those errors may go unnoticed and left unfixed.

This would be the equivalent of PHP 5.2 notices and using silencing the error output to make the problem go away.

There may be, though, cases where it is a good idea to catch the error, but I would like to take them case by base.

Would something like this work without issues in PHP 5.x?

I found this while googling, but wouldn’t Throwable be a undeclared type in PHP 5?

try {
    // Code that may throw an Exception or Error.
} catch (Throwable $t) {
    // Executed only in PHP 7, will not match in PHP 5.x
} catch (Exception $e) {
    // Executed only in PHP 5.x, will not be reached in PHP 7
}

It looks like we need this for at least the error handler and some low level logging etc, otherwise php7 errors (Throwable) happening in those will not be properly handled.

It should in those cases always look like this:

    try {
        ...
    } catch (Throwable $exception) {
        ...
    } catch (Exception $exception) {
        ...
    }

In case of PHP5.6 those will just be ignored and the fallback will be used. We do not need to add this for high level exceptions where it is clear only “old” kind of exceptions are expected.

@dereuromark just to add.

I’ve had a lot of experience since this post with PHP 7, and I’m under the impression that a developer should not be catching the new Throwable interface. Only Exceptions, as catching these fatal PHP errors is a major change in behavior from PHP 5.6

I’ve since changed my project to allow the Error type to be uncaught, and as a result closing the transaction is no longer an issue since the app is going to crash anyway, and the DB connection should be closed (which is an auto rollback).

I am not in favor of the closure approach. Almost all exceptions shouldn’t occur in normal case. Creating two closures and calling user defined functions everytime everywhere is not a good idea.

<del> Can't we catch Throwable after catching Exception? ```php try { } catch (Exception $e) {

} catch (Throwable $t) { // Never come here in PHP 5, as only instances of Exception can be thrown. // They should be caught the above catch block. }

</del>

Never mind. I forgot about [how instanceof triggers autoloader in PHP](https://bugs.php.net/bug.php?id=55475#1314007968).