Make GuzzleException extend Throwable whereever it's available #2273
Conversation
src/Exception/GuzzleException.php
Outdated
| * @method string getFile() | ||
| * @method int getLine() | ||
| * @method array getTrace() | ||
| * @method string getTraceAsString() |
There was a problem hiding this comment.
these annotations should not be needed anymore because the method signatures are provided by throwable
|
just stumbled into this because of phpstan complaining about |
There was a problem hiding this comment.
LGTM except the comment from @dbu
See also symfony/symfony#28307
|
@dbu @Tobion Thanks for the review and the feedback! I've updated the PR, but a unit test is now failing because of a hardcoded date in the I have created a separate PR to fix that and as soon as that's merged or the issue is otherwise fixed, I'll rebase this PR to make it pass inspection. |
|
@ErikBooijCB thanks a lot for the PR. Can you please rebase it? |
|
@sagikazarmark Absolutely, done 👍 |
|
Thanks! |
|
Hello @ErikBooijCB, i create a fork of this repo and release this marge request into the new version(6.4.0). Namespace compatibility is 100%, and you can use my changes if you patch composer.json in your projects
Fork link: https://github.com/SomeBlackMagic/guzzle/ |
| interface GuzzleException {} | ||
| use Throwable; | ||
|
|
||
| if (interface_exists(Throwable::class)) { |
There was a problem hiding this comment.
interface_exists has a second parameter autoload that defaults to true: https://www.php.net/manual/en/function.interface-exists.php
Can we explicitely pass this as false? It makes no sense imo to try autoloading a SPL class/interface, and causes issues with bad autoloader implementations that include/require everything passed to them even if such a file does not exist.
I can make a PR if needed.
Our static analyzer is giving us some issues when catching Guzzles exceptions, because they don't extend throwable and thus aren't technically catchable.
I get that we don't want to extend it by default because it would break BC with PHP5.x, but I think the proposed solution would give us the best of both worlds.