Skip to content

Conversation

@ivodvb
Copy link

@ivodvb ivodvb commented Aug 25, 2021

I'd like to propose the change to support Throwables next to Exceptions, so that they can be handled properly as well.

Throwables within PHP are supported since PHP 7.0. Currently only Exceptions are supported and Throwables will lead to an UnexpectedValueException. With this change Throwables are supported as well.

Given the direction reactphp/promise goes I think it would be good to make this change.

@clue
Copy link
Owner

clue commented Sep 3, 2021

@ivodvb Thanks for looking into this and filing this PR!

It looks like a somewhat similar changeset has been proposed via #52. The changes look perfectly reasonable to me, but unfortunately this introduces a small BC break with regards to what types this project will throw. The original feature was introduced via #7/#27/#42.

What do you think about this?

@ivodvb
Copy link
Author

ivodvb commented Sep 6, 2021

Thanks for looking into the PR @clue.

I definitely agree on the BC break. I think there are two options:

  1. Release a new version (2.0) with this breaking change;
  2. Add a parameter $rejectedInstanceOf to await in which the name of accepted class/interface can be passed.

The first option is the most sane to me, I suggest to go with that one.
The second option feels hacky, but prevents a BC break.

@clue
Copy link
Owner

clue commented Oct 17, 2021

@ivodvb I agree introducing a BC break is the way forward in the long run. I've discussed future options for this package with a number of people, so expect some major changes not too far in the future anyway 🤫

As an intermediary solution, I've just filed #57 which at least eases debugging by appending the original message for Throwables. What do you think about this?

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants