bryancall commented on code in PR #13873:
URL: https://github.com/apache/trafficserver/pull/13873#discussion_r4340374987
##########
tests/uranium_tests/h2/h2active_timeout.py:
##########
@@ -78,65 +90,46 @@ def makerequest(port: int, path: str, delay: int) -> None:
h2_connection.send_headers(1, headers, end_stream=True)
tls_socket.sendall(h2_connection.data_to_send())
- # delay, triggering ATS timeout.
- time.sleep(delay)
-
- # The following should fail due to the timeout.
+ # Read the response, then keep reading until ATS closes the connection.
+ closed = False
try:
- # Send a second request.
- h2_connection.send_headers(3, headers, end_stream=True)
- tls_socket.sendall(h2_connection.data_to_send())
-
- response_stream_ended = False
- body = b''
- while not response_stream_ended:
- # read raw data from the socket
- data = tls_socket.recv(65536 * 1024)
+ while True:
+ remaining = start + max_wait - time.monotonic()
+ if remaining <= 0:
+ break
+ tls_socket.settimeout(remaining)
+ data = tls_socket.recv(65536)
if not data:
+ closed = True
break
-
- # feed raw data into h2, and process resulting events
- events = h2_connection.receive_data(data)
- for event in events:
- if isinstance(event, h2.events.ResponseReceived):
- # response headers received
- print("Response received:")
- for header in event.headers:
- print(f' {header[0].decode()}: {header[1].decode()}')
+ for event in h2_connection.receive_data(data):
if isinstance(event, h2.events.DataReceived):
- # update flow control so the server doesn't starve us
h2_connection.acknowledge_received_data(event.flow_controlled_length,
event.stream_id)
- # more response body data received
- body += event.data
- if isinstance(event, h2.events.StreamEnded):
- # response body completed, let's exit the loop
- response_stream_ended = True
- break
- # send any pending data to the server
tls_socket.sendall(h2_connection.data_to_send())
Review Comment:
This check is about when ATS closes the connection, not about the response.
A close that comes no sooner than the 3 second active timeout and within the
wait is the timeout firing, whether or not the response arrived first. The
other tests in test_http2.py already check the GET response itself, so I've
left this as is.
##########
tests/uranium_tests/h2/h2active_timeout.py:
##########
@@ -78,65 +90,46 @@ def makerequest(port: int, path: str, delay: int) -> None:
h2_connection.send_headers(1, headers, end_stream=True)
tls_socket.sendall(h2_connection.data_to_send())
- # delay, triggering ATS timeout.
- time.sleep(delay)
-
- # The following should fail due to the timeout.
+ # Read the response, then keep reading until ATS closes the connection.
+ closed = False
try:
- # Send a second request.
- h2_connection.send_headers(3, headers, end_stream=True)
- tls_socket.sendall(h2_connection.data_to_send())
-
- response_stream_ended = False
- body = b''
- while not response_stream_ended:
- # read raw data from the socket
- data = tls_socket.recv(65536 * 1024)
+ while True:
+ remaining = start + max_wait - time.monotonic()
+ if remaining <= 0:
+ break
+ tls_socket.settimeout(remaining)
+ data = tls_socket.recv(65536)
if not data:
+ closed = True
break
-
- # feed raw data into h2, and process resulting events
- events = h2_connection.receive_data(data)
- for event in events:
- if isinstance(event, h2.events.ResponseReceived):
- # response headers received
- print("Response received:")
- for header in event.headers:
- print(f' {header[0].decode()}: {header[1].decode()}')
+ for event in h2_connection.receive_data(data):
if isinstance(event, h2.events.DataReceived):
- # update flow control so the server doesn't starve us
h2_connection.acknowledge_received_data(event.flow_controlled_length,
event.stream_id)
- # more response body data received
- body += event.data
- if isinstance(event, h2.events.StreamEnded):
- # response body completed, let's exit the loop
- response_stream_ended = True
- break
- # send any pending data to the server
tls_socket.sendall(h2_connection.data_to_send())
-
- print(f"Response fully received: {len(body)} bytes")
-
- body_str = body.decode('utf-8')
-
- # tell the server we are closing the h2 connection
- h2_connection.close_connection()
- tls_socket.sendall(h2_connection.data_to_send())
-
- # close the socket
- tls_socket.close()
- except Exception:
+ except socket.timeout:
+ pass
+ except (ConnectionError, ssl.SSLError):
+ closed = True
+ elapsed = time.monotonic() - start
+ tls_socket.close()
Review Comment:
h2active_timeout.py runs as its own short-lived process, so an unexpected
exception exits non-zero with a traceback and the OS closes the socket. The
test then fails on its return code assert with that traceback in the output, so
nothing is left open.
##########
tests/uranium_tests/h2/h2active_timeout.py:
##########
@@ -78,65 +90,46 @@ def makerequest(port: int, path: str, delay: int) -> None:
h2_connection.send_headers(1, headers, end_stream=True)
tls_socket.sendall(h2_connection.data_to_send())
- # delay, triggering ATS timeout.
- time.sleep(delay)
-
- # The following should fail due to the timeout.
+ # Read the response, then keep reading until ATS closes the connection.
+ closed = False
try:
- # Send a second request.
- h2_connection.send_headers(3, headers, end_stream=True)
- tls_socket.sendall(h2_connection.data_to_send())
-
- response_stream_ended = False
- body = b''
- while not response_stream_ended:
- # read raw data from the socket
- data = tls_socket.recv(65536 * 1024)
+ while True:
+ remaining = start + max_wait - time.monotonic()
+ if remaining <= 0:
+ break
+ tls_socket.settimeout(remaining)
+ data = tls_socket.recv(65536)
if not data:
+ closed = True
break
-
- # feed raw data into h2, and process resulting events
- events = h2_connection.receive_data(data)
- for event in events:
- if isinstance(event, h2.events.ResponseReceived):
- # response headers received
- print("Response received:")
- for header in event.headers:
- print(f' {header[0].decode()}: {header[1].decode()}')
+ for event in h2_connection.receive_data(data):
if isinstance(event, h2.events.DataReceived):
- # update flow control so the server doesn't starve us
h2_connection.acknowledge_received_data(event.flow_controlled_length,
event.stream_id)
- # more response body data received
- body += event.data
- if isinstance(event, h2.events.StreamEnded):
- # response body completed, let's exit the loop
- response_stream_ended = True
- break
- # send any pending data to the server
tls_socket.sendall(h2_connection.data_to_send())
-
- print(f"Response fully received: {len(body)} bytes")
-
- body_str = body.decode('utf-8')
-
- # tell the server we are closing the h2 connection
- h2_connection.close_connection()
- tls_socket.sendall(h2_connection.data_to_send())
-
- # close the socket
- tls_socket.close()
- except Exception:
+ except socket.timeout:
+ pass
+ except (ConnectionError, ssl.SSLError):
+ closed = True
+ elapsed = time.monotonic() - start
+ tls_socket.close()
+
+ if not closed:
+ print(f"Connection still open after {max_wait} seconds")
+ elif elapsed < active_timeout:
+ print(f"Connection closed after {elapsed:.1f} seconds, before the
{active_timeout} second active timeout")
+ else:
print("CONNECTION_TIMEOUT")
Review Comment:
test_http2.py asserts that CONNECTION_TIMEOUT is in the output, and pytest
prints that output when the assert fails, so a failure already shows which case
it hit: still open, or closed after N seconds. A non-zero exit would fail one
assert earlier with the same text.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
