From 2128d17a2046bd0a7fc3f63a5576076ed4be1882 Mon Sep 17 00:00:00 2001 From: daywalker90 Date: Mon, 28 Sep 2026 17:09:25 +0200 Subject: [PATCH] tests: fix websocket connect races in test_clnrest notifications_received_via_websocket() relied on the socketio client's connect_error callback and its default 1s wait_timeout to detect the namespace handshake result. In the flakes the server logged "Authorized rune" and "Socket.IO connected" well inside that window, yet the client raised socketio.exceptions.ConnectionError: One or more namespaces failed to connect: so the connect_error callback never fired and, for the rejection tests, the expected error was compared against an empty string, e.g. ValueError: Expected error code `Missing rune`, got `One or more namespaces failed to connect: ` instead Why the client does not process the server's namespace reply is not established: it could not be reproduced locally, and the engine.io upgrade path on both sides looks correct. So this works around it rather than fixing a root cause. This affected both the rejection tests (test_websocket_no_rune, test_websocket_wrong_rune, test_websocket_rune_no_listnotifications, test_websocket_upgrade_header) and the successful-connection tests (test_websocket_unrestricted_rune, test_websocket_rune_readonly, test_websocket_rune_listnotifications, test_numeric_msat_notification). Give the namespace handshake a longer timeout, disable the client's background reconnection and retry transient connect failures for the success path. Keep checking the exact text the server sent when the connect_error callback fires, and fall back to the node log when it does not. Also make test_websocket_upgrade_header fail if the connection unexpectedly succeeds, instead of falling through. Changelog-None --- tests/test_clnrest.py | 82 +++++++++++++++++++++++++++---------------- 1 file changed, 51 insertions(+), 31 deletions(-) diff --git a/tests/test_clnrest.py b/tests/test_clnrest.py index bcc9a668e122..65dfe0908f35 100644 --- a/tests/test_clnrest.py +++ b/tests/test_clnrest.py @@ -10,6 +10,7 @@ import pytest import json import os +import re # Skip the entire module if we don't have Rust. pytestmark = pytest.mark.skipif( @@ -277,31 +278,46 @@ def notifications_received_via_websocket(l1, base_url, http_session, rpc_method= - if we couldn't connect to the websocket server, the notification list we return is empty.""" http_session.headers.update({"upgrade": "websocket"}) - sio = socketio.Client(http_session=http_session) notifications = [] connection_error_msg = None - @sio.event - def message(data): - notifications.append(data) - - @sio.event - def connect_error(data): - nonlocal connection_error_msg - connection_error_msg = str(data) - - try: - sio.connect(base_url) - except socketio.exceptions.ConnectionError as e: - # Use the detailed error message if available - error_str = connection_error_msg if connection_error_msg else str(e) - - if expect_error and expect_error in error_str: - return notifications - else: - raise ValueError(f"Expected error code `{expect_error}`, got `{error_str}` instead") - except Exception: - raise + def connect(): + sio = socketio.Client(http_session=http_session, reconnection=False) + + @sio.event + def message(data): + notifications.append(data) + + @sio.event + def connect_error(data): + nonlocal connection_error_msg + connection_error_msg = str(data) + + sio.connect(base_url, wait_timeout=10) + return sio + + sio = None + for attempt in range(3): + try: + sio = connect() + break + except socketio.exceptions.ConnectionError: + if expect_error: + # If the CONNECT_ERROR packet arrived, check the text the + # server sent. Otherwise it may have arrived after the + # client's connect timeout, so confirm the rejection from + # the node log instead. + if connection_error_msg is None: + l1.daemon.wait_for_log(rf'verify_rune failed: .*{re.escape(expect_error)}') + elif expect_error not in connection_error_msg: + raise ValueError(f"Expected error code `{expect_error}`, got `{connection_error_msg}` instead") + return notifications + # The namespace CONNECT reply is not always received before the + # client's connect timeout; give it another go. + if attempt == 2: + raise + time.sleep(1) + assert sio is not None if expect_error: raise Exception(f"did not raise expected error {expect_error}") time.sleep(2) @@ -500,7 +516,7 @@ def test_websocket_upgrade_header(node_factory): http_session = http_session_with_retry() http_session.verify = ca_cert.as_posix() - sio = socketio.Client(http_session=http_session) + sio = socketio.Client(http_session=http_session, reconnection=False) notifications = [] connection_error_msg = None @@ -512,16 +528,20 @@ def message(data): def connect_error(data): nonlocal connection_error_msg connection_error_msg = str(data) + try: sio.connect(base_url) - except socketio.exceptions.ConnectionError as e: - error_str = connection_error_msg if connection_error_msg else str(e) - expect_error = "Missing rune" - - if expect_error not in error_str: - raise ValueError(f"Expected error code `{expect_error}`, got `{error_str}` instead") - except Exception: - raise + except socketio.exceptions.ConnectionError: + # If the CONNECT_ERROR packet arrived, check the text the server sent; + # otherwise it may have arrived after the client's connect timeout, so + # confirm the rejection from the node log instead. + if connection_error_msg is None: + l1.daemon.wait_for_log(r'verify_rune failed: .*Missing rune') + elif 'Missing rune' not in connection_error_msg: + raise ValueError(f"Expected error code `Missing rune`, got `{connection_error_msg}` instead") + else: + sio.disconnect() + raise Exception("did not raise expected error Missing rune") time.sleep(2) # trigger notification by calling method