Skip to content

Conversation

@lovelydinosaur
Copy link
Contributor

Refs: encode/httpx#1478

Currently if the server is in a listening state, then disconnects without sending a response, then users will see the following error...

httpcore.RemoteProtocolError: can't handle event type ConnectionClosed when role=SERVER and state=SEND_RESPONSE

Which really isn't informative as an end-user. We probably would prefer to special case this particular transition as a ConnectError. It's not quite the same as a network ConnectError, since it occurs in cases where the connection was already established and hanging around, but is closed once it an incoming request is sent.

It's not super obvious if we want to treat this case as a ConnectError or something else. Here's our existing hierarchy in this area...

  • NetworkError
    • ConnectError
    • ReadError
    • WriteError
    • CloseError
  • ProtocolError
    • LocalProtocolError
    • RemoteProtocolError

@lovelydinosaur
Copy link
Contributor Author

Requests treats these as ConnectionError. URLLib3 treats them as ProtocolError.

See: https://github.com/psf/requests/blob/c45a4dfe6bfc6017d4ea7e9f051d6cc30972b310/requests/adapters.py#L497

Having reviewed this it makes more sense to me for use to keep the exception class the same as it otherwise would be, but issue a clearer message. So now...

httpcore.RemoteProtocolError: Server disconnected without sending a response.

@lovelydinosaur
Copy link
Contributor Author

Though we could also introduce a specific new class, eg...

  • NetworkError
    • ConnectError
    • ReadError
    • WriteError
    • CloseError
  • ProtocolError
    • LocalProtocolError
    • RemoteProtocolError
    • ServerDisconnected

The one place where our behaviour is important here is wrt. retry strategies.

@lovelydinosaur lovelydinosaur merged commit 005f57d into master Apr 29, 2021
@lovelydinosaur lovelydinosaur deleted the raise-connect-error-for-h11-case-when-server-does-not-send-a-response branch April 29, 2021 12:00
@lovelydinosaur lovelydinosaur changed the title Raise ConnectError when server disconnects without sending a response Improve message on RemoteProtocolError exception when server disconnects without sending a response Apr 29, 2021
@lovelydinosaur lovelydinosaur mentioned this pull request Apr 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants