Skip to content

Conversation

@TotalyEnglizLitrate
Copy link

@TotalyEnglizLitrate TotalyEnglizLitrate commented Nov 28, 2025

When HTTPConnection.getresponse() encounters an exception/error (like TimeoutError) while reading the response, the outer exception handler calls response.close() but not self.close(). This closes the socket but leaves the connection's internal state stuck at _CS_REQ_SENT, causing subsequent requests on the same connection object to raise CannotSendRequest.

This change correctly closes the connection instead of the response.

… response.begin() raises exceptions

When response.begin() raises an exception other than ConnectionError
(such as TimeoutError), the outer exception handler calls
response.close() but not self.close(), leaving the HTTPConnection's
internal state stuck at _CS_REQ_SENT instead of being reset to _CS_IDLE.

This causes subsequent requests on the same connection to raise
CannotSendRequest even though the underlying socket has been closed.

The fix changes the outer exception handler to call self.close() instead
of just response.close(). This properly resets the connection state and
closes the socket, consistent with the inner ConnectionError handler.
@TotalyEnglizLitrate TotalyEnglizLitrate changed the title gh-141938: Fix HTTPConnection being stuck in a dirty state when respo… gh-141938: Fix HTTPConnection being stuck in a dirty state when HTTPResponse.begin() raises exceptions Nov 28, 2025
@TotalyEnglizLitrate TotalyEnglizLitrate changed the title gh-141938: Fix HTTPConnection being stuck in a dirty state when HTTPResponse.begin() raises exceptions gh-141938: Fix HTTPConnection being stuck in a dirty state when HTTPResponse.begin() raises exceptions/errors Nov 28, 2025
@python-cla-bot
Copy link

python-cla-bot bot commented Nov 28, 2025

All commit authors signed the Contributor License Agreement.

CLA signed

@TotalyEnglizLitrate TotalyEnglizLitrate marked this pull request as draft November 28, 2025 20:28
@picnixz
Copy link
Member

picnixz commented Nov 28, 2025

I will close this PR until (1) tests pass (2) we decide to change this behavior. As you can see this change broke the test suite.

@picnixz picnixz closed this Nov 28, 2025
@TotalyEnglizLitrate
Copy link
Author

TotalyEnglizLitrate commented Nov 28, 2025

I seem to have ran the tests on the main branch locally and thought it passed 😅

The erroneous behavior stemmed from the fact that self.__response was not set to response and was None, that should be fixed by either setting self.__response before calling self.close() or by closing response and connection separately, however the (same) test still fails locally because the test assumes self.sock will not be None to check whether it is closed.

If the decision is made to change this behavior the test would also need to be modified as closing the connection will set self.sock to None.

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