PSR-18: Separate ConnectException from RequestException

[davidgrayston]

Guzzle version affected: 7.0.0-beta.1

Relates to #2507 / php-http/httplug#155

Description

Update exception hierarchy to conform with https://www.php-fig.org/psr/psr-18/meta/

The domain exceptions NetworkExceptionInterface and RequestExceptionInterface define a contract very similar to each other. The chosen approach is to not let them extend each other because inheritance does not make sense in the domain model. A RequestExceptionInterface is simply not a NetworkExceptionInterface.

Changed

• ConnectException now extends TransferException instead of RequestException

  Note: request, response and handler context methods have been moved into traits for reuse - there might be a neater/preferable way

New hierarchy:

\RuntimeException
-- TransferException
-- -- ConnectException (Psr\Http\Client\NetworkExceptionInterface)
-- -- RequestException (Psr\Http\Client\RequestExceptionInterface)
-- -- -- TooManyRedirectsException
-- -- -- BadResponseException
-- -- -- -- ClientException
-- -- -- -- ServerException

I've tried to do this with as few breaking changes as possible - you could instead introduce a new exception that implements Psr\Http\Client\RequestExceptionInterface which would extend RequestException like this (which would be non-breaking):

\RuntimeException
-- TransferException
-- -- RequestException
-- -- -- ConnectException (Psr\Http\Client\NetworkExceptionInterface)
-- -- -- RequestFailedException (Psr\Http\Client\RequestExceptionInterface)
-- -- -- -- TooManyRedirectsException
-- -- -- -- BadResponseException
-- -- -- -- -- ClientException
-- -- -- -- -- ServerException

[gmponos]

I am not sure about the traits.. because the functions are pretty simple.. besides that in general I agree that separating the exceptions is a necessary evil step that we need to apply towards 7.0 and supporting PSR-18

[davidgrayston]

@gmponos I can remove use of traits if preferable?

I was also wondering if the following methods could be removed with a note added to UPGRADING.md?

• GuzzleHttp\Exception\ConnectException::getResponse()
• GuzzleHttp\Exception\ConnectException::hasResponse()

[Nyholm]

Thank you for this PR.
I agree with this change.

Could you please remove the Traits?
Could you also remove the hasResponse() and getResponse() from the ConnectException.

[davidgrayston]

Would you prefer the PHPDoc for getHandlerContext() be duplicated in each exception, or could this be moved into a handler context interface, with each implementation using inheritdoc?

[Nyholm]

You can duplicate it.
It is "just exceptions" and it is just twice.

[Nyholm]

Thank you

[gmponos]

Can we update changelog as well.. In general I am happy with it.

[Nyholm]

Generally, change log updates should not be done in a PR. That will lead to merge conflicts.

[Tobion]

With these changes, we should also change RequestException to make the response non-optional. The response was only optional for ConnectException. But now ConnectException does not extend RequestException anymore. All sub-classes of RequestException have a response now AFAIK.

[Tobion]

Looks like TooManyRedirectsException does not have a response but should have. The response is just not forwarded from what I see.

[Nyholm]

Thank you @Tobion

[sergejostir]

The docs haven't been updated and still display the old hiearchy: https://docs.guzzlephp.org/en/stable/quickstart.html#exceptions