Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Throwable PHP7 #26

Open
lon9man opened this issue Oct 14, 2019 · 5 comments
Open

Throwable PHP7 #26

lon9man opened this issue Oct 14, 2019 · 5 comments

Comments

@lon9man
Copy link

lon9man commented Oct 14, 2019

Expected Behavior

PHP7 needs to catch Throwable instead Exception

Current Behavior

try-catch don't catch Throwable instead Exception

Possible Solution

refactor all try-catch

My Environment

  • PHP-Resque version: latest
  • PHP version: 7.1
  • Operating System and version: Ubuntu 18.10
@lon9man lon9man added the bug label Oct 14, 2019
@BenoitDD-URIOS
Copy link

No need to refactor all try/catch.
PHP Resque Exceptions just have to implement Throwable interface.

@lon9man
Copy link
Author

lon9man commented Oct 14, 2019

@Benoit-amo
not sure that understood.
for example:

catch(Exception $e) {

how this example can be refactored in other way than replacing Exception to Throwable?

@BenoitDD-URIOS
Copy link

Ok, I missed that.
How many catch like this ?

@lon9man
Copy link
Author

lon9man commented Oct 15, 2019

@Benoit-amo
with concrete "catch(Exception $e)" it seems only 1 occurence.

but exist some child-exceptions which has extended from Exception
OR
type-hinted parameters in methods:

public static function create($payload, Exception $exception, Resque_Worker $worker, $queue)

i am not sure which one should be updated to Throwable and which one not.

@danhunsaker
Copy link
Member

There are a lot of PHP 7 updates that haven't been done yet. This one is relatively minor, as \Exception extends \Throwable anyway, so only \Errors will be uncaught, which is consistent with PHP 5 behavior. (AIUI, it's illegal to extend or even implement the \Throwable interface directly anyway, so only type hints should be updated even when we version bump to add this change.)

In other words, this will be a BC break, and will need several other PHP 7 changes in place before it makes any sense to do. So this will be an item on the PHP 7 checklist.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants