From b41caeb5100bafcb8f4611909e4f42bf6dd40faa Mon Sep 17 00:00:00 2001 From: Tom Brow Date: Sun, 1 Nov 2026 07:32:28 -0800 Subject: [PATCH] fix(mobile): time out relay connects that never finish the handshake (#8878) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a phone's network silently stops working (crowded event Wi-Fi, a weak cell signal), Buzz Mobile can stay offline for minutes. In a device test, the app took 30s to notice the dead connection, then hid the channel's messages behind a loading skeleton. It tried to reconnect once, and that attempt hung for 5 minutes (303s). It never retried, because it only retries after an attempt fails. This PR gives every reconnect attempt a 10s limit. When an attempt runs out of time, the app closes it and tries again with its normal backoff. ### What was wrong Opening the relay connection has two code paths. The one used with hosted relay auth already had a 10s limit. The other path had none, so a stuck attempt waited for the operating system to give up, which takes minutes. Both paths now use the same limit. Each attempt also gets its own HTTP client, so a timed-out attempt is actually closed rather than left running in the background. ### Validation I tested on an iPhone SE (iOS 27.0.1) running a profile build, with its relay traffic routed through a Mac proxy that could silently drop everything. - **Before:** during a 5.6 minute outage, the app made one reconnect attempt, which hung for 303s. - **After:** during a 2.5 minute outage, each attempt gave up after 10s and the app kept retrying. It reconnected 36s after the network came back, a delay mostly set by the 30s cap on time between retries. Added tests: `relay_socket_liveness_test` now covers an attempt that never completes, with and without hosted relay auth headers. It checks that the attempt fails and that the abandoned connection is closed. The test fails without the fix. Two existing tests were opening real relay connections by accident or design: - `deep_link_dispatcher_test` now uses a fake channel list, like its neighboring tests. - `remote_agents_test` talks to a real local relay on purpose, so it now uses a real HTTP client instead of the test framework's mock. The full mobile test suite passes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Tom Brow Co-authored-by: Tom Brow --- mobile/lib/shared/relay/relay_socket.dart | 4 ++ mobile/lib/shared/relay/relay_websocket.dart | 22 ++++--- .../channels/deep_link_dispatcher_test.dart | 3 + .../shared/agents/remote_agents_test.dart | 3 + .../relay/relay_socket_liveness_test.dart | 64 +++++++++++++++++++ 5 files changed, 88 insertions(+), 8 deletions(-) diff --git a/mobile/lib/shared/relay/relay_socket.dart b/mobile/lib/shared/relay/relay_socket.dart index 61329e8afd..558fba8865 100644 --- a/mobile/lib/shared/relay/relay_socket.dart +++ b/mobile/lib/shared/relay/relay_socket.dart @@ -40,6 +40,9 @@ class RelaySocket { @visibleForTesting static Duration debugPingInterval = pingInterval; + @visibleForTesting + static Duration debugConnectTimeout = relayWebSocketConnectTimeout; + final String _wsUrl; final String? _nsec; final void Function(List message) _onMessage; @@ -86,6 +89,7 @@ class RelaySocket { Uri.parse(_wsUrl), headers: headers, pingInterval: debugPingInterval, + connectTimeout: debugConnectTimeout, ), ); await _channel!.ready; diff --git a/mobile/lib/shared/relay/relay_websocket.dart b/mobile/lib/shared/relay/relay_websocket.dart index a91f19129a..0e4b91fc77 100644 --- a/mobile/lib/shared/relay/relay_websocket.dart +++ b/mobile/lib/shared/relay/relay_websocket.dart @@ -1,28 +1,34 @@ import 'dart:async'; import 'dart:io'; +/// Bounds how long a relay WebSocket may take to connect and upgrade. +/// +/// The OS gives up on a stalled TCP or TLS handshake only after minutes, and +/// reconnecting starts only once this attempt fails, so an unbounded attempt +/// leaves the app reconnecting for as long as the network stays stalled. +const relayWebSocketConnectTimeout = Duration(seconds: 10); + /// WebSocket's default HTTP client follows redirects, including custom headers. /// Keep key-bound assertions at their original origin and bound upgrade time. +/// +/// Each attempt gets its own HTTP client so a timed-out handshake can be +/// force-closed instead of left running until the OS gives up on it. Future openRelayWebSocket( Uri uri, { required Map headers, required Duration pingInterval, + Duration connectTimeout = relayWebSocketConnectTimeout, }) async { - if (headers.isEmpty) { - return WebSocket.connect( - uri.toString(), - ).then((socket) => socket..pingInterval = pingInterval); - } - final client = _UpgradeClient(); + final HttpClient client = headers.isEmpty ? HttpClient() : _UpgradeClient(); try { return await WebSocket.connect( uri.toString(), - headers: headers, + headers: headers.isEmpty ? null : headers, customClient: client, ) .then((socket) => socket..pingInterval = pingInterval) .timeout( - const Duration(seconds: 10), + connectTimeout, onTimeout: () { client.close(force: true); throw TimeoutException('Relay connection timed out'); diff --git a/mobile/test/features/channels/deep_link_dispatcher_test.dart b/mobile/test/features/channels/deep_link_dispatcher_test.dart index e810aa7cab..b213b925ff 100644 --- a/mobile/test/features/channels/deep_link_dispatcher_test.dart +++ b/mobile/test/features/channels/deep_link_dispatcher_test.dart @@ -871,6 +871,9 @@ void main() { ), ), inviteJoinRecoveryProvider.overrideWithValue((_) => recovery), + channelsProvider.overrideWith( + () => _FakeChannelsNotifier(Future.value(const [])), + ), ], ); addTearDown(container.dispose); diff --git a/mobile/test/shared/agents/remote_agents_test.dart b/mobile/test/shared/agents/remote_agents_test.dart index 0469b2261a..cbae56a746 100644 --- a/mobile/test/shared/agents/remote_agents_test.dart +++ b/mobile/test/shared/agents/remote_agents_test.dart @@ -57,6 +57,9 @@ Future _controller(FakeRegistry registry) async { void main() { TestWidgetsFlutterBinding.ensureInitialized(); + // Registrations go to a real loopback relay. The binding's mocked HTTP + // client would reject the WebSocket upgrade. + HttpOverrides.global = null; setUp(() { FlutterSecureStorage.setMockInitialValues({}); SharedPreferences.setMockInitialValues({}); diff --git a/mobile/test/shared/relay/relay_socket_liveness_test.dart b/mobile/test/shared/relay/relay_socket_liveness_test.dart index e3802f39b7..88ab3f9d0c 100644 --- a/mobile/test/shared/relay/relay_socket_liveness_test.dart +++ b/mobile/test/shared/relay/relay_socket_liveness_test.dart @@ -3,6 +3,7 @@ import 'dart:convert'; import 'dart:io'; import 'package:buzz/shared/relay/relay_socket.dart'; +import 'package:buzz/shared/relay/relay_websocket.dart'; import 'package:crypto/crypto.dart'; import 'package:flutter_test/flutter_test.dart'; @@ -40,17 +41,80 @@ Future _silentAfterHandshakeServer() async { return server; } +/// A server that accepts TCP connections and then never answers, like a +/// stalled TLS or upgrade handshake on a congested network. +/// Accepts TCP and never answers the upgrade. [closed] completes when the +/// client drops the connection. +Future<(ServerSocket server, Future closed)> +_stalledHandshakeServer() async { + final server = await ServerSocket.bind(InternetAddress.loopbackIPv4, 0); + final closed = Completer(); + server.listen( + (client) => client.listen( + (_) {}, + onError: (_) {}, + onDone: () { + if (!closed.isCompleted) closed.complete(); + }, + ), + ); + return (server, closed.future); +} + void main() { const testPingInterval = Duration(milliseconds: 150); + const testConnectTimeout = Duration(milliseconds: 200); setUp(() { RelaySocket.debugPingInterval = testPingInterval; + RelaySocket.debugConnectTimeout = testConnectTimeout; }); tearDown(() { RelaySocket.debugPingInterval = RelaySocket.pingInterval; + RelaySocket.debugConnectTimeout = relayWebSocketConnectTimeout; }); + for (final scoped in [false, true]) { + test('gives up on a handshake that never completes ' + '(${scoped ? 'scoped headers' : 'no headers'})', () async { + final (server, clientClosed) = await _stalledHandshakeServer(); + + final disconnected = Completer(); + final socket = RelaySocket( + wsUrl: 'ws://127.0.0.1:${server.port}', + nsec: null, + onMessage: (_) {}, + onConnected: () {}, + onDisconnected: (error) { + if (!disconnected.isCompleted) disconnected.complete(error); + }, + ); + if (scoped) { + socket.headers = {'Nostr-Federated-Identity': 'Bearer synthetic'}; + } + unawaited(socket.connect()); + + final error = await disconnected.future.timeout( + testConnectTimeout * 10, + onTimeout: () => fail( + 'RelaySocket must fail a stalled handshake so the session can ' + 'schedule a reconnect', + ), + ); + expect(socket.state, SocketState.disconnected); + expect(error.toString(), contains('Relay connection timed out')); + // The abandoned handshake is closed, not left for the OS to time out. + await clientClosed.timeout( + testConnectTimeout * 10, + onTimeout: () => fail('the timed-out handshake was left open'), + ); + + socket.dispose(); + await server.close(); + }); + } + test('detects a peer that stops answering pings', () async { final server = await _silentAfterHandshakeServer();