Do we really have to catch Guzzle RequestException in getResponse?

[frzsombor]

Hi!

I'm working on a project that uses this library, and I noticed that in the getResponse function, there is a try-catch block (line 288-295) around the Guzzle request, which just re-throws the error message. The problem is that, this way I can't really build a good error handling around the translation process, because I can't tell the difference between ConnectException, ClientException, ServerException, etc., which would be great in my opinion.

Do we really have to catch Guzzle RequestExceptions in getResponse?
Can you tell me, what is the purpose of catching and re-throwing the Exceptions there?

[Stichoza]

Ok, I got the point. The reason was to be more independent from http client package. I mean if someday we will switch from Guzzle to something else, it would be smooth operation without major changes and developers will not have to change their code to handle SomeNewHttpClientExceptions instead of GuzzleRequestException.

[frzsombor]

Thanks for the explanation, I understand now.
Here are the exceptions of Guzzle and how they extends other classes:

\Exception
    \ErrorException (this library)
    \RuntimeException
        SeekException
        TransferException
            RequestException
                ConnectException
                TooManyRedirectsException
                BadResponseException
                    ClientException
                    ServerException

In my situation it is important to check, if the exception is a ConnectException or any BadResponseException, because I'm using proxies to get around Google IP bans (as I even get banned with this library after a few hours - maybe due to large number of requests). If a proxy is temporary down for some reason (which happens many times), I get a ConnectException, but if Google banned any of them (and I shouldn't use it for like 24 hours), it will raise a ClientException (4xx type errors, like 403 Forbidden) or ServerException (5xx type errors, like 503 Service Unavailable).

• I think it would be wrong to switch to Guzzle's exceptions, because
the parent class of Guzzle's exceptions is \RuntimeException, and this library throws \ErrorException, so if someone only catches the second one, an update could break his site/app.

• I think it would be good to switch to Guzzle's exceptions, because
As long as someone catches \Exception, the using of Guzzle's custom exceptions shouldn't cause any problem. And I think catching custom exceptions, but not catching \Exception is a bad programming practice, so hopefully nobody using this library made this mistake.

Of course it's on you to switch or not, I can use my synchronized private fork anytime, just wanted to share my thoughts and ideas :)

[Stichoza]

Sure, thanks for sharing your opinion, appreciated that.
I'll think about this again, because currently both options seem to be correct. Can't decide easily.

[Stichoza]

Leaving ErrorException after two years of thinking 😆 For the same reasons stated above.

[Stichoza]

This issue is about to receive a good update in v5.1.0 🎉

[frzsombor]

Great job, props to you for still maintaining this repo and taking care of the feedbacks!