HTTP/1.1 keep-alive connections not reestablished if remote has gone away

[florimondmanca]

Related:

• Add trio concurrency backend #276 (comment)
• Add trio concurrency backend #276 (comment)

Use case

• Create an HTTPX client.
• Make a request to remote A.
• For some reason, connection to remote A gets lost (e.g. we restart our WiFi, or remote A goes down, or our ISP has issues, …).
• Make another request.

Expected behavior

According to these these 3 tests in tests/dispatch/test_connection_pool.py:

• test_keepalive_connection_closed_by_server_is_reestablished
• test_keepalive_http2_connection_closed_by_server_is_reestablished
• test_connection_closed_free_semaphore_on_acquire

the connection should be automatically reestablished. More precisely, the ConnectionResetError caused by trying to read from a closed socket should be handled correctly.

(It seems that the server restart we do in the above tests does not do what we think it does — and these tests may only be passing thanks to some asyncio magic.)

Actual behavior

We get a ConnectionResetError and then h11 state gets scrambled because of improper teardown — see reproduction script and associated traceback below.

To reproduce

# client.py
import asyncio

import httpx


async def main():
    url = "http://localhost:8000"
    async with httpx.ConnectionPool() as http:
        response = await http.request("GET", url)
        await response.read()

        input("Shutdown the server to close the keep-alive connection")

        response = await http.request("GET", url)
        await response.read()
        assert len(http.active_connections) == 0
        assert len(http.keepalive_connections) == 1


asyncio.run(main())

Traceback

$ python debugflorimond/client.py
Shutdown the server to close the keep-alive connection
Traceback (most recent call last):
  File "/Users/florimond/Developer/python-projects/httpx/venv/lib/python3.7/site-packages/h11/_state.py", line 249, in _fire_event_triggered_transitions
    new_state = EVENT_TRIGGERED_TRANSITIONS[role][state][event_type]
KeyError: <class 'h11._events.ConnectionClosed'>

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "debugflorimond/client.py", line 20, in <module>
    asyncio.run(main())
  File "/Users/florimond/.pyenv/versions/3.7.3/lib/python3.7/asyncio/runners.py", line 43, in run
    return loop.run_until_complete(main)
  File "/Users/florimond/.pyenv/versions/3.7.3/lib/python3.7/asyncio/base_events.py", line 584, in run_until_complete
    return future.result()
  File "debugflorimond/client.py", line 14, in main
    response = await http.request("GET", url)
  File "/Users/florimond/Developer/python-projects/httpx/httpx/dispatch/base.py", line 40, in request
    return await self.send(request, verify=verify, cert=cert, timeout=timeout)
  File "/Users/florimond/Developer/python-projects/httpx/httpx/dispatch/connection_pool.py", line 126, in send
    raise exc
  File "/Users/florimond/Developer/python-projects/httpx/httpx/dispatch/connection_pool.py", line 121, in send
    request, verify=verify, cert=cert, timeout=timeout
  File "/Users/florimond/Developer/python-projects/httpx/httpx/dispatch/connection.py", line 65, in send
    response = await self.h11_connection.send(request, timeout=timeout)
  File "/Users/florimond/Developer/python-projects/httpx/httpx/dispatch/http11.py", line 53, in send
    http_version, status_code, headers = await self._receive_response(timeout)
  File "/Users/florimond/Developer/python-projects/httpx/httpx/dispatch/http11.py", line 133, in _receive_response
    event = await self._receive_event(timeout)
  File "/Users/florimond/Developer/python-projects/httpx/httpx/dispatch/http11.py", line 164, in _receive_event
    event = self.h11_state.next_event()
  File "/Users/florimond/Developer/python-projects/httpx/venv/lib/python3.7/site-packages/h11/_connection.py", line 439, in next_event
    exc._reraise_as_remote_protocol_error()
  File "/Users/florimond/Developer/python-projects/httpx/venv/lib/python3.7/site-packages/h11/_util.py", line 72, in _reraise_as_remote_protocol_error
    raise self
  File "/Users/florimond/Developer/python-projects/httpx/venv/lib/python3.7/site-packages/h11/_connection.py", line 422, in next_event
    self._process_event(self.their_role, event)
  File "/Users/florimond/Developer/python-projects/httpx/venv/lib/python3.7/site-packages/h11/_connection.py", line 238, in _process_event
    self._cstate.process_event(role, type(event), server_switch_event)
  File "/Users/florimond/Developer/python-projects/httpx/venv/lib/python3.7/site-packages/h11/_state.py", line 238, in process_event
    self._fire_event_triggered_transitions(role, event_type)
  File "/Users/florimond/Developer/python-projects/httpx/venv/lib/python3.7/site-packages/h11/_state.py", line 253, in _fire_event_triggered_transitions
    .format(event_type.__name__, role, self.states[role]))
h11._util.RemoteProtocolError: can't handle event type ConnectionClosed when role=SERVER and state=SEND_RESPONSE

Possible causes

It's not immediately obvious from the traceback above why we get a RemoteProtocolError. Here's the relevant snippet of code in HTTP11Connection._receive_response_data():

httpx/httpx/dispatch/http11.py

Lines 171 to 181 in 71648ec

	if event is h11.NEED_DATA:
	try:
	data = await self.stream.read(
	self.READ_NUM_BYTES, timeout, flag=self.timeout_flag
	)
	except OSError: # pragma: nocover
	data = b""
	self.h11_state.receive_data(data)
	else:
	assert event is not h11.NEED_DATA
	break # pragma: no cover

As is visible here, we send an EOF to h11 if we get an OSError, which results in h11 producing a ConnectionClosed event although it was expecting some data, leading to the RemoteProtocolError.

(By removing the except OSError error handling, the ConnectionResetError propagates up to the main program.)

My conclusion here is that we need to rethink the logic of detecting closed connections. The current implementation seems to have been brought in #143.

cc @tomchristie

[florimondmanca]

OK, nevermind. My example wasn't working because of this naive blocking input() call, which prevented asyncio from running its background socket polling and detecting the EOF. Replacing input() with a long asyncio.sleep() showed that the server correctly reconnects using a new connection. So no bug to report here, we're all good!