Guzzle exceptions

[Nyholm]

I dont believe we have the best structure of our exceptions.

The exception hierarchy currently look like this:

\InvalidArgumentException
-- InvalidArgumentException

\RuntimeException
-- TransferException 
-- -- RequestException
-- -- -- TooManyRedirectsException
-- -- -- ConnectException
-- -- -- BadResponseException
-- -- -- -- ClientException
-- -- -- -- ServerException

I suggest we update it to:

\InvalidArgumentException
-- InvalidArgumentException

\RuntimeException
-- RequestException
-- TooManyRedirectsException
-- NetworkException
-- BadResponseException
-- -- ClientException
-- -- ServerException

[GrahamCampbell]

Maybe we should keep TransferException. Useful for catching stuff that went wrong with the http request, but not other errors, probably caused by bad code.

[Nyholm]

I did not see a point in keeping both ConnectException and TransferException. I agree with you that we should have such exception. I think it is an issue with naming. In this proposal I think ConnectException, TransferException and NetworkException are all doing the same thing.

[GrahamCampbell]

What about:

\InvalidArgumentException
-- InvalidArgumentException

\RuntimeException
-- RuntimeException
-- -- TransferException
-- -- -- RequestException
-- -- -- TooManyRedirectsException
-- -- -- ConnectException
-- -- -- BadResponseException
-- -- -- -- ClientException
-- -- -- -- ServerException

[GrahamCampbell]

The naming of TransferException matches what HTTPlug does, and their RequestException extends it.

[Nyholm]

Hehe. Im not super happy with HTTPlug's exceptions either =)

What is the point of TransferException? All exceptions implements the GuzzleException interface so you can use it if you want to "catch everything".

[GrahamCampbell]

But what if you don't want to catch Guzzle exceptions not caused by the actual HTTP request-response stuff?

[Nyholm]

But what if you don't want to catch Guzzle exceptions not caused by the actual HTTP request-response stuff?

Im not sure I 100% understand. Could you rephrase that without the double negative? And what is the "HTTP request-response stuff"? Do you mean the network layer?

[sagikazarmark]

TransferException is something that happened after a request is passed to the client, but before/after the request is sent. It's an error related to exceptional events outside of the HTTP context (if they can be identified). Non-HTTP timeouts, connection failures belong to this category.

I can see why it might be useful, but TBH I never used it. I don't see any harm in keeping it though.

[Nyholm]

And FYI:

The TransferException class is never used in Guzzle. It is only referred to here:

class RequestException extends TransferException implements RequestExceptionInterface

---

@sagikazarmark:
So if the request is valid and we still cannot send the Request, that sounds like you are describing PSR18's NetworkException.

[sagikazarmark]

Yeah, those two sound identical.

[Nyholm]

I've updated my first entry to this issue. It is now:

\InvalidArgumentException
-- InvalidArgumentException

\RuntimeException
-- RequestException
-- TooManyRedirectsException
-- NetworkException
-- BadResponseException
-- -- ClientException
-- -- ServerException

[GrahamCampbell]

Kinda odd that we have a custom InvalidArg exception but not for Runtime?

[Nyholm]

I saw that you added that in your suggestion.

I dont think we should have our own RuntimeException because we are never throwing one, we do however throw a InvalidArgumentException.

[gmponos]

Before I come up with my suggestion would you guys consider having an InvalidOptionException ?

[mfn]

Is GuzzleException kept?

It's just it's not mentioned in the issue description (probably because it's an interface) but it is the "number 1" exception I use to catch "all that could go wrong with guzzle" and later decide how to specifically handle stuff.

[gmponos]

Is GuzzleException kept?

This is a discussion with suggestions for v8.0.. no bc change regarding exception structure has been applied to upcoming v7.0

It is good to have your feedback about how you handle exceptions...

Note that there is no suggestions about removing this interface :)

[davidgrayston]

ConnectException currently extends RequestException, which means it implements both \Psr\Http\Client\NetworkExceptionInterface and \Psr\Http\Client\RequestExceptionInterface - this seems to contradict https://www.php-fig.org/psr/psr-18/meta/#exception-model

Should this be treated as a bug? (as per php-http/httplug#158 / php-http/httplug#155)

Perhaps ConnectException could extend TransferException instead of RequestException?

\RuntimeException
-- TransferException
-- -- ConnectException
-- -- RequestException
-- -- -- TooManyRedirectsException
-- -- -- BadResponseException
-- -- -- -- ClientException
-- -- -- -- ServerException

I can open a separate issue if this is to be treated as a bug in v7?
(if not, please ignore this as you have already suggested fixing the hierarchy in v8 #2507 (comment))

[brettmc]

PSR-18 mentions a TimeoutException, and I think it is worth implementing in guzzle - I would like to know if a request failed due to not completing within the timeout I have set without having to parse an exception message.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 2 weeks if no further activity occurs. Thank you for your contributions.