mirror of
https://github.com/block/buzz.git
synced 2026-11-02 15:31:27 +00:00
fix(mobile): time out relay connects that never finish the handshake (#8878)
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 <tomb@squareup.com> Co-authored-by: Tom Brow <tomb@squareup.com>
This commit is contained in:
@@ -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<dynamic> message) _onMessage;
|
||||
@@ -86,6 +89,7 @@ class RelaySocket {
|
||||
Uri.parse(_wsUrl),
|
||||
headers: headers,
|
||||
pingInterval: debugPingInterval,
|
||||
connectTimeout: debugConnectTimeout,
|
||||
),
|
||||
);
|
||||
await _channel!.ready;
|
||||
|
||||
@@ -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<WebSocket> openRelayWebSocket(
|
||||
Uri uri, {
|
||||
required Map<String, String> 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');
|
||||
|
||||
@@ -871,6 +871,9 @@ void main() {
|
||||
),
|
||||
),
|
||||
inviteJoinRecoveryProvider.overrideWithValue((_) => recovery),
|
||||
channelsProvider.overrideWith(
|
||||
() => _FakeChannelsNotifier(Future.value(const [])),
|
||||
),
|
||||
],
|
||||
);
|
||||
addTearDown(container.dispose);
|
||||
|
||||
@@ -57,6 +57,9 @@ Future<RelayFakeController> _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({});
|
||||
|
||||
@@ -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<ServerSocket> _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<void> closed)>
|
||||
_stalledHandshakeServer() async {
|
||||
final server = await ServerSocket.bind(InternetAddress.loopbackIPv4, 0);
|
||||
final closed = Completer<void>();
|
||||
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<Object?>();
|
||||
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();
|
||||
|
||||
|
||||
Reference in New Issue
Block a user