diff --git a/packages/devtools_app/lib/src/shared/editor/editor_client.dart b/packages/devtools_app/lib/src/shared/editor/editor_client.dart index 496f860d970..2c2136abda0 100644 --- a/packages/devtools_app/lib/src/shared/editor/editor_client.dart +++ b/packages/devtools_app/lib/src/shared/editor/editor_client.dart @@ -29,8 +29,6 @@ class EditorClient extends DisposableController String get gaId => EditorSidebar.id; - bool get isDtdClosed => _dtd.isClosed; - Future _initialize() async { autoDisposeStreamSubscription( _dtd.onEvent(CoreDtdServiceConstants.servicesStreamId).listen((data) { diff --git a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_controller.dart b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_controller.dart index 684fae414ed..bad9c16e6aa 100644 --- a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_controller.dart +++ b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_controller.dart @@ -54,20 +54,13 @@ class PropertyEditorController extends DisposableController String? get fileUri => _editableWidgetData.value?.fileUri; EditorRange? get widgetRange => _editableWidgetData.value?.range; - ValueListenable get shouldReconnect => _shouldReconnect; - final _shouldReconnect = ValueNotifier(false); - bool get waitingForFirstEvent => _waitingForFirstEvent; bool _waitingForFirstEvent = true; late final Debouncer _requestDebouncer; - late final Timer _checkConnectionTimer; - static const _requestDebounceDuration = Duration(milliseconds: 600); - static const _checkConnectionInterval = Duration(minutes: 1); - static const _setPropertiesFilterId = 'set-properties-filter'; @visibleForTesting @@ -84,9 +77,6 @@ class PropertyEditorController extends DisposableController void init() { super.init(); _requestDebouncer = Debouncer(duration: _requestDebounceDuration); - _checkConnectionTimer = _periodicallyCheckConnection( - _checkConnectionInterval, - ); // Update in response to ActiveLocationChanged events. autoDisposeStreamSubscription( @@ -133,7 +123,6 @@ class PropertyEditorController extends DisposableController @override void dispose() { _requestDebouncer.dispose(); - _checkConnectionTimer.cancel(); super.dispose(); } @@ -266,16 +255,6 @@ class PropertyEditorController extends DisposableController List _extractRefactors(CodeActionResult? result) => (result?.actions ?? []).toList(); - Timer _periodicallyCheckConnection(Duration interval) { - return Timer.periodic(interval, (timer) { - final isClosed = editorClient.isDtdClosed; - if (isClosed) { - _shouldReconnect.value = true; - timer.cancel(); - } - }); - } - bool _filteredOutBySettings( EditableProperty property, { required Filter filter, diff --git a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_panel.dart b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_panel.dart index fafac06fba0..bf6e8893bb3 100644 --- a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_panel.dart +++ b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/property_editor_panel.dart @@ -17,7 +17,6 @@ import '../../../shared/primitives/query_parameters.dart'; import '../../../shared/ui/common_widgets.dart'; import 'property_editor_controller.dart'; import 'property_editor_view.dart'; -import 'reconnecting_overlay.dart'; /// The side panel for the Property Editor. class PropertyEditorPanel extends StatefulWidget { @@ -106,43 +105,31 @@ class _PropertyEditorConnectedPanelState @override Widget build(BuildContext context) { - return ValueListenableBuilder( - valueListenable: widget.controller.shouldReconnect, - builder: (context, shouldReconnect, _) { - return Stack( - children: [ - Scrollbar( + return Scrollbar( + controller: scrollController, + thumbVisibility: true, + child: Column( + children: [ + Expanded( + child: SingleChildScrollView( controller: scrollController, - thumbVisibility: true, - child: Column( - children: [ - Expanded( - child: SingleChildScrollView( - controller: scrollController, - child: Padding( - padding: const EdgeInsets.fromLTRB( - denseSpacing, - defaultSpacing, - defaultSpacing, // Additional right padding for scroll bar. - defaultSpacing, - ), - child: Column( - crossAxisAlignment: CrossAxisAlignment.start, - children: [ - PropertyEditorView(controller: widget.controller), - ], - ), - ), - ), - ), - const _PropertyEditorFooter(), - ], + child: Padding( + padding: const EdgeInsets.fromLTRB( + denseSpacing, + defaultSpacing, + defaultSpacing, // Additional right padding for scroll bar. + defaultSpacing, + ), + child: Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [PropertyEditorView(controller: widget.controller)], + ), ), ), - if (shouldReconnect) const ReconnectingOverlay(), - ], - ); - }, + ), + const _PropertyEditorFooter(), + ], + ), ); } } diff --git a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/reconnecting_overlay.dart b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/reconnecting_overlay.dart index 66f9f3394b2..aac701f0390 100644 --- a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/reconnecting_overlay.dart +++ b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/reconnecting_overlay.dart @@ -2,70 +2,63 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file or at https://developers.google.com/open-source/licenses/bsd. -import 'dart:async'; - +import 'package:devtools_app_shared/service.dart'; import 'package:devtools_app_shared/ui.dart'; import 'package:flutter/material.dart'; +import '../../../shared/globals.dart'; import '../../../shared/ui/common_widgets.dart'; -import 'utils/utils.dart'; -class ReconnectingOverlay extends StatefulWidget { - const ReconnectingOverlay({super.key}); +// TODO(dantup): Rename and move this file one level up. Leaving as-is to +// make the review/diff simpler. - @override - State createState() => _ReconnectingOverlayState(); -} +/// An overlay to show when we are not connected to DTD based on the +/// [DTDConnectionState] classes. +class NotConnectedOverlay extends StatefulWidget { + const NotConnectedOverlay(this.connectionState, {super.key}); -class _ReconnectingOverlayState extends State { - static const _countdownInterval = Duration(seconds: 1); - late final Timer _countdownTimer; - int _secondsUntilReconnection = 3; + final DTDConnectionState connectionState; @override - void initState() { - super.initState(); - _countdownTimer = Timer.periodic(_countdownInterval, _onTick); - } - - @override - void dispose() { - _countdownTimer.cancel(); - super.dispose(); - } + State createState() => _NotConnectedOverlayState(); +} +class _NotConnectedOverlayState extends State { @override Widget build(BuildContext context) { + final connectionState = widget.connectionState; final theme = Theme.of(context); + + final showSpinner = connectionState is! ConnectionFailedDTDState; + final showReconnectButton = connectionState is ConnectionFailedDTDState; + final stateLabel = switch (connectionState) { + NotConnectedDTDState() => 'Waiting to connect...', + ConnectingDTDState() => 'Connecting...', + WaitingToRetryDTDState(seconds: final seconds) => + 'Reconnecting in $seconds...', + ConnectionFailedDTDState() => 'Connection Failed', + // We should never present this widget when connected, but provide a label + // for debugging if it happens. + ConnectedDTDState() => 'Connected', + }; + return DevToolsOverlay( fullScreen: true, content: Column( mainAxisAlignment: MainAxisAlignment.center, children: [ - const CircularProgressIndicator(), - const SizedBox(height: defaultSpacing), - Text( - _secondsUntilReconnection > 0 - ? 'Reconnecting in $_secondsUntilReconnection' - : 'Reconnecting...', - style: theme.textTheme.headlineMedium, - ), + if (showSpinner) ...const [ + CircularProgressIndicator(), + SizedBox(height: defaultSpacing), + ], + Text(stateLabel, style: theme.textTheme.headlineMedium), + if (showReconnectButton) + ElevatedButton( + onPressed: () => dtdManager.reconnect(), + child: const Text('Retry'), + ), ], ), ); } - - void _onTick(Timer timer) { - setState(() { - _secondsUntilReconnection--; - if (_secondsUntilReconnection == 0) { - timer.cancel(); - _reconnect(); - } - }); - } - - void _reconnect() { - forceReload(); - } } diff --git a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/utils/utils.dart b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/utils/utils.dart index c8877868dee..cb824b077c4 100644 --- a/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/utils/utils.dart +++ b/packages/devtools_app/lib/src/standalone_ui/ide_shared/property_editor/utils/utils.dart @@ -3,7 +3,6 @@ // found in the LICENSE file or at https://developers.google.com/open-source/licenses/bsd. import 'package:flutter/widgets.dart'; -import '_utils_desktop.dart' if (dart.library.js_interop) '_utils_web.dart'; /// Converts a [dartDocText] String into a [Text] widget. class DartDocConverter { @@ -133,10 +132,3 @@ class DartDocConverter { return result; } } - -/// Workaround to force reload the Property Editor when it disconnects. -/// -/// See https://github.com/flutter/devtools/issues/9028 for details. -void forceReload() { - reloadIframe(); -} diff --git a/packages/devtools_app/lib/src/standalone_ui/standalone_screen.dart b/packages/devtools_app/lib/src/standalone_ui/standalone_screen.dart index b1e1bfaff3d..6a1f64a7de1 100644 --- a/packages/devtools_app/lib/src/standalone_ui/standalone_screen.dart +++ b/packages/devtools_app/lib/src/standalone_ui/standalone_screen.dart @@ -2,12 +2,14 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file or at https://developers.google.com/open-source/licenses/bsd. +import 'package:devtools_app_shared/service.dart'; import 'package:dtd/dtd.dart'; import 'package:flutter/material.dart'; import '../shared/globals.dart'; import '../shared/ui/common_widgets.dart'; import 'ide_shared/property_editor/property_editor_panel.dart'; +import 'ide_shared/property_editor/reconnecting_overlay.dart'; import 'vs_code/flutter_panel.dart'; /// "Screens" that are intended for standalone use only, likely for embedding @@ -33,26 +35,13 @@ enum StandaloneScreenType { 'newer of the Dart VS Code extension', ), ), - StandaloneScreenType.editorSidebar => ValueListenableBuilder( - // TODO(dantup): Add a timeout here so if dtdManager.connection - // doesn't complete after some period we can give some kind of - // useful message. - valueListenable: dtdManager.connection, - builder: (context, data, _) { - return _DtdConnectedScreen( - dtd: data, - screenProvider: (dtd) => EditorSidebarPanel(dtd), - ); - }, + StandaloneScreenType.editorSidebar => _DtdConnectedScreen( + dtdManager: dtdManager, + builder: (dtd) => EditorSidebarPanel(dtd), ), - StandaloneScreenType.propertyEditor => ValueListenableBuilder( - valueListenable: dtdManager.connection, - builder: (context, data, _) { - return _DtdConnectedScreen( - dtd: data, - screenProvider: (dtd) => PropertyEditorPanel(dtd), - ); - }, + StandaloneScreenType.propertyEditor => _DtdConnectedScreen( + dtdManager: dtdManager, + builder: (dtd) => PropertyEditorPanel(dtd), ), }; } @@ -61,18 +50,45 @@ enum StandaloneScreenType { values.any((value) => value.name == screenName); } -/// Widget that returns a [CenteredCircularProgressIndicator] while it waits for -/// a [DartToolingDaemon] connection. +/// Widget that show progress while connecting to [DartToolingDaemon] and then +/// the result of calling [builder] when a connection is available. +/// +/// If the DTD connection is dropped, a reconnecting progress will be shown. class _DtdConnectedScreen extends StatelessWidget { - const _DtdConnectedScreen({required this.dtd, required this.screenProvider}); + const _DtdConnectedScreen({required this.dtdManager, required this.builder}); - final DartToolingDaemon? dtd; - final Widget Function(DartToolingDaemon) screenProvider; + final DTDManager dtdManager; + final Widget Function(DartToolingDaemon) builder; @override Widget build(BuildContext context) { - return dtd == null - ? const CenteredCircularProgressIndicator() - : screenProvider(dtd!); + return ValueListenableBuilder( + valueListenable: dtdManager.connectionState, + builder: (context, connectionState, child) { + return ValueListenableBuilder( + valueListenable: dtdManager.connection, + builder: (context, connection, _) { + return Stack( + children: [ + if (connection != null) + // Use a keyed subtree on the connection, so if the connection + // changes (eg. we reconnect), we reset the state because it's + // not safe to assume the existing state is still valid. + // + // This allows us to still keep rendering the old state under + // the overlay (rather than a blank background) until the + // reconnect occurs. + KeyedSubtree( + key: ValueKey(connection), + child: builder(connection), + ), + if (connectionState is! ConnectedDTDState) + NotConnectedOverlay(connectionState), + ], + ); + }, + ); + }, + ); } } diff --git a/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md b/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md index 5e4353dd692..d5d4a6c8a09 100644 --- a/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md +++ b/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md @@ -15,7 +15,9 @@ To learn more about DevTools, check out the ## General updates -TODO: Remove this section if there are not any updates. +- Dropped connections to DTD will now automatically be retried to improve the + experience when your machine is resumed from sleep. + [#9587](https://github.com/flutter/devtools/pull/9587) ## Inspector updates diff --git a/packages/devtools_app/test/test_infra/scenes/standalone_ui/editor_service/simulated_editor.dart b/packages/devtools_app/test/test_infra/scenes/standalone_ui/editor_service/simulated_editor.dart index 41c7eeec437..4a18e78421a 100644 --- a/packages/devtools_app/test/test_infra/scenes/standalone_ui/editor_service/simulated_editor.dart +++ b/packages/devtools_app/test/test_infra/scenes/standalone_ui/editor_service/simulated_editor.dart @@ -21,12 +21,18 @@ import 'package:web_socket_channel/web_socket_channel.dart'; /// carefully to ensure they are not breaking changes to already-shipped /// editors. class SimulatedEditor { - SimulatedEditor(this._dtdUri) { - // Set up some default devices. - connectDevices(); + SimulatedEditor._(this._dtdUri); + /// Creates and connects a simulated editor with some default devices. + static Future connect(Uri dtdUri) async { + final editor = SimulatedEditor._(dtdUri); // Connect editor automatically at launch. - unawaited(connectEditor()); + await editor.connectEditor(); + + // Set up some default devices. + editor.connectDevices(); + + return editor; } /// The URI of the DTD instance we are connecting/connected to. @@ -116,12 +122,19 @@ class SimulatedEditor { DTDServiceCallback callback, { Map? capabilities, }) async { - await _dtd?.registerService( - editorServiceName, - method.name, - callback, - capabilities: capabilities, - ); + try { + await _dtd?.registerService( + editorServiceName, + method.name, + callback, + capabilities: capabilities, + ); + } catch (e) { + // Just log but don't fail everything, because this is normal if we try + // to connect to a real DTD that already has an editor connected. We + // should just fill in any missing services. + _logger.add('Failed to register "$method": $e'); + } } static const _successResponse = {'type': 'Success'}; diff --git a/packages/devtools_app/test/test_infra/scenes/standalone_ui/editor_sidebar.dart b/packages/devtools_app/test/test_infra/scenes/standalone_ui/editor_sidebar.dart index 2a331cb3811..57cbd4fa9e4 100644 --- a/packages/devtools_app/test/test_infra/scenes/standalone_ui/editor_sidebar.dart +++ b/packages/devtools_app/test/test_infra/scenes/standalone_ui/editor_sidebar.dart @@ -5,11 +5,10 @@ import 'dart:async'; import 'package:devtools_app/devtools_app.dart'; -import 'package:devtools_app/src/standalone_ui/vs_code/flutter_panel.dart'; +import 'package:devtools_app/src/standalone_ui/standalone_screen.dart'; import 'package:devtools_app_shared/service.dart'; import 'package:devtools_app_shared/ui.dart'; import 'package:devtools_app_shared/utils.dart'; -import 'package:devtools_test/devtools_test.dart'; import 'package:dtd/dtd.dart'; import 'package:flutter/material.dart'; import 'package:stager/stager.dart'; @@ -35,7 +34,7 @@ class EditorSidebarScene extends Scene { body: MockEditorWidget( editor: editor, clientLog: clientLog, - child: EditorSidebarPanel(clientDtd), + child: StandaloneScreenType.editorSidebar.screen, ), ), ); @@ -46,12 +45,20 @@ class EditorSidebarScene extends Scene { @override Future setUp() async { + final logStream = StreamController(); + final dtdManager = TestingDTDManager( + logStream.sink, + // Set this variable to similate a number of failed connections for + // testing. + failConnectionCount: 3, + ); + setStagerMode(); setGlobal( DevToolsEnvironmentParameters, ExternalDevToolsEnvironmentParameters(), ); - setGlobal(DTDManager, MockDTDManager()); + setGlobal(DTDManager, dtdManager); setGlobal(IdeTheme, IdeTheme()); setGlobal(PreferencesController, PreferencesController()); @@ -60,9 +67,10 @@ class EditorSidebarScene extends Scene { // TODO(dantup): Add a way for the mock editor to set workspace roots so // the extensions parts can work in the sidebar. final dtdUri = Uri.parse('ws://127.0.0.1:8500/'); - final connection = await createLoggedWebSocketChannel(dtdUri); - clientLog = connection.log; - clientDtd = DartToolingDaemon.fromStreamChannel(connection.channel); - editor = SimulatedEditor(dtdUri); + clientLog = logStream.stream; + editor = await SimulatedEditor.connect(dtdUri); + + // Start connecting to DTD after 1s, so we see the progress indicators. + unawaited(dtdManager.connect(dtdUri)); } } diff --git a/packages/devtools_app/test/test_infra/scenes/standalone_ui/mock_editor_widget.dart b/packages/devtools_app/test/test_infra/scenes/standalone_ui/mock_editor_widget.dart index 196401e8bad..08329bab45c 100644 --- a/packages/devtools_app/test/test_infra/scenes/standalone_ui/mock_editor_widget.dart +++ b/packages/devtools_app/test/test_infra/scenes/standalone_ui/mock_editor_widget.dart @@ -6,6 +6,7 @@ import 'dart:async'; import 'dart:collection'; import 'dart:convert'; +import 'package:devtools_app/devtools_app.dart'; import 'package:devtools_app/src/shared/primitives/list_queue_value_notifier.dart'; import 'package:devtools_app_shared/ui.dart'; import 'package:devtools_app_shared/utils.dart'; @@ -258,6 +259,23 @@ class _MockEditorWidgetState extends State ), child: const Text('Stop All'), ), + const SizedBox(height: defaultSpacing * 5), + Text('DTD Manager', style: theme.textTheme.headlineMedium), + const SizedBox(height: defaultSpacing), + const Text( + 'Use these buttons to simulate actions on the DTD Manager that the sidebar panel is using', + ), + const SizedBox(height: defaultSpacing), + Row( + children: [ + const Text('DTD Connection: '), + ElevatedButton( + onPressed: () => + dtdManager.disconnectImpl(allowReconnect: true), + child: const Text('Drop Connection'), + ), + ], + ), ], ), ), diff --git a/packages/devtools_app/test/test_infra/scenes/standalone_ui/property_editor_sidebar.dart b/packages/devtools_app/test/test_infra/scenes/standalone_ui/property_editor_sidebar.dart index 6346811359d..889c170f248 100644 --- a/packages/devtools_app/test/test_infra/scenes/standalone_ui/property_editor_sidebar.dart +++ b/packages/devtools_app/test/test_infra/scenes/standalone_ui/property_editor_sidebar.dart @@ -5,12 +5,10 @@ import 'dart:async'; import 'package:devtools_app/devtools_app.dart'; -import 'package:devtools_app/src/standalone_ui/ide_shared/property_editor/property_editor_panel.dart'; +import 'package:devtools_app/src/standalone_ui/standalone_screen.dart'; import 'package:devtools_app_shared/service.dart'; import 'package:devtools_app_shared/ui.dart'; import 'package:devtools_app_shared/utils.dart'; -import 'package:devtools_test/devtools_test.dart'; -import 'package:dtd/dtd.dart'; import 'package:flutter/material.dart'; import 'package:stager/stager.dart'; @@ -24,9 +22,11 @@ import 'shared/utils.dart'; /// /// flutter run -t test/test_infra/scenes/standalone_ui/property_editor_sidebar.stager_app.g.dart -d chrome class PropertyEditorSidebarScene extends Scene { + late Stream clientLog; + @override Widget build(BuildContext context) { - return const _PropertyEditorSidebar(); + return _PropertyEditorSidebar(clientLog); } @override @@ -34,27 +34,36 @@ class PropertyEditorSidebarScene extends Scene { @override Future setUp() async { + final logStream = StreamController(); + clientLog = logStream.stream; + final dtdManager = TestingDTDManager( + logStream.sink, + // Set this variable to similate a number of failed connections for + // testing. + failConnectionCount: 3, + ); + setStagerMode(); setGlobal( DevToolsEnvironmentParameters, ExternalDevToolsEnvironmentParameters(), ); - setGlobal(DTDManager, MockDTDManager()); + setGlobal(DTDManager, dtdManager); setGlobal(IdeTheme, IdeTheme()); setGlobal(PreferencesController, PreferencesController()); } } class _PropertyEditorSidebar extends StatefulWidget { - const _PropertyEditorSidebar(); + const _PropertyEditorSidebar(this.clientLog); + + final Stream clientLog; @override State<_PropertyEditorSidebar> createState() => _PropertyEditorState(); } class _PropertyEditorState extends State<_PropertyEditorSidebar> { - Stream? clientLog; - DartToolingDaemon? clientDtd; SimulatedEditor? editor; @override @@ -66,11 +75,11 @@ class _PropertyEditorState extends State<_PropertyEditorSidebar> { Widget build(BuildContext context) { return IdeThemedMaterialApp( home: Scaffold( - body: clientLog != null && clientDtd != null && editor != null + body: editor != null ? MockEditorWidget( editor: editor!, - clientLog: clientLog!, - child: PropertyEditorPanel(clientDtd!), + clientLog: widget.clientLog, + child: StandaloneScreenType.propertyEditor.screen, ) : _DtdUriForm( onSaved: _connectToDtd, @@ -82,12 +91,12 @@ class _PropertyEditorState extends State<_PropertyEditorSidebar> { Future _connectToDtd(String? dtdUri) async { if (dtdUri == null) return; + final uri = Uri.parse(dtdUri); - final connection = await createLoggedWebSocketChannel(uri); + final editor = await SimulatedEditor.connect(uri); + unawaited(dtdManager.connect(uri)); setState(() { - clientLog = connection.log; - clientDtd = DartToolingDaemon.fromStreamChannel(connection.channel); - editor = SimulatedEditor(uri); + this.editor = editor; }); } } diff --git a/packages/devtools_app/test/test_infra/scenes/standalone_ui/shared/utils.dart b/packages/devtools_app/test/test_infra/scenes/standalone_ui/shared/utils.dart index 1edbd6f3b68..0156a7f2603 100644 --- a/packages/devtools_app/test/test_infra/scenes/standalone_ui/shared/utils.dart +++ b/packages/devtools_app/test/test_infra/scenes/standalone_ui/shared/utils.dart @@ -4,6 +4,8 @@ import 'dart:async'; +import 'package:devtools_app_shared/service.dart'; +import 'package:dtd/dtd.dart'; import 'package:stream_channel/stream_channel.dart'; import 'package:web_socket_channel/web_socket_channel.dart'; @@ -11,11 +13,13 @@ import 'package:web_socket_channel/web_socket_channel.dart'; /// in both directions across it. typedef LoggedChannel = ({StreamChannel channel, Stream log}); -/// Connects to the websocket at [wsUri] and returns a channel along with -/// a log stream that includes all protocol traffic. -Future createLoggedWebSocketChannel(Uri wsUri) async { - final logController = StreamController(); - +/// Connects to the websocket at [wsUri] and returns a [StreamSink]. +/// +/// All traffic is logged into [sink]. +Future> createLoggedWebSocketChannel( + Uri wsUri, + StreamSink sink, +) async { final rawChannel = WebSocketChannel.connect(wsUri); await rawChannel.ready; final rawStringChannel = rawChannel.cast(); @@ -24,7 +28,7 @@ Future createLoggedWebSocketChannel(Uri wsUri) async { /// traffic with a prefix. String Function(String) logTraffic(String prefix) { return (String s) { - logController.add('$prefix $s'.trim()); + sink.add('$prefix $s'.trim()); return s; }; } @@ -38,8 +42,37 @@ Future createLoggedWebSocketChannel(Uri wsUri) async { .pipe(rawStringChannel.sink), ); - return ( - channel: StreamChannel(loggedInput, loggedOutputController.sink), - log: logController.stream, - ); + return StreamChannel(loggedInput, loggedOutputController.sink); +} + +/// An implementation of DTD Manager that logs traffic to [logSink] and can +/// optionally delay or fail connections to DTD for testing. +class TestingDTDManager extends DTDManager { + TestingDTDManager( + this.logSink, { + this.failConnectionCount = 0, + this.connectionDelay = const Duration(seconds: 1), + }); + + /// The number of connections to fail before connecting. + var failConnectionCount = 0; + + /// The delay for each connection attempt. + final Duration connectionDelay; + + /// The sink to write protocol traffic to. + final StreamSink logSink; + + @override + Future connectDtdImpl(Uri uri) async { + await Future.delayed(connectionDelay); + + if (failConnectionCount > 0) { + failConnectionCount--; + throw 'Connection failed'; + } + + final channel = await createLoggedWebSocketChannel(uri, logSink); + return DartToolingDaemon.fromStreamChannel(channel); + } } diff --git a/packages/devtools_app_shared/lib/service.dart b/packages/devtools_app_shared/lib/service.dart index 43e8e8d83bb..608adefbeb3 100644 --- a/packages/devtools_app_shared/lib/service.dart +++ b/packages/devtools_app_shared/lib/service.dart @@ -5,6 +5,7 @@ export 'src/service/connected_app.dart'; export 'src/service/constants.dart'; export 'src/service/dtd_manager.dart'; +export 'src/service/dtd_manager_connection_state.dart'; export 'src/service/eval_on_dart_library.dart'; export 'src/service/flutter_version.dart'; export 'src/service/isolate_manager.dart' hide TestIsolateManager; diff --git a/packages/devtools_app_shared/lib/src/service/dtd_manager.dart b/packages/devtools_app_shared/lib/src/service/dtd_manager.dart index e2e95085c88..183ca530e13 100644 --- a/packages/devtools_app_shared/lib/src/service/dtd_manager.dart +++ b/packages/devtools_app_shared/lib/src/service/dtd_manager.dart @@ -2,10 +2,15 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file or at https://developers.google.com/open-source/licenses/bsd. +import 'dart:async'; +import 'dart:math' as math; + import 'package:dtd/dtd.dart'; import 'package:flutter/foundation.dart'; import 'package:logging/logging.dart'; +import 'dtd_manager_connection_state.dart'; + final _log = Logger('dtd_manager'); /// Manages a connection to the Dart Tooling Daemon. @@ -18,39 +23,225 @@ class DTDManager { /// Whether the [DTDManager] is connected to a running instance of the DTD. bool get hasConnection => connection.value != null; + /// The current state of the connection. + ValueListenable get connectionState => _connectionState; + final _connectionState = + ValueNotifier(NotConnectedDTDState()); + /// The URI of the current DTD connection. Uri? get uri => _uri; Uri? _uri; - /// Sets the Dart Tooling Daemon connection to point to [uri]. + /// Whether or not to automatically reconnect if disconnected. /// - /// Before connecting to [uri], if a current connection exists, then - /// [disconnect] is called to close it. - Future connect( + /// This will happen by default as long as the disconnect wasn't + /// explicitly requested. + bool _automaticallyReconnect = true; + + Timer? _periodicConnectionCheck; + static const _periodicConnectionCheckInterval = Duration(minutes: 1); + + /// A function that replays the last connection attempt. + /// + /// This is used by [reconnect] to reconnect to the last server with the same + /// settings if the connection was dropped and failed to reconnect within the + /// specified retry period. + Future Function()? _lastConnectFunc; + + /// A wrapper around connecting to DTD to allow tests to intercept the + /// connection. + @visibleForTesting + Future connectDtdImpl(Uri uri) async { + // Cancel any previous timer. + _periodicConnectionCheck?.cancel(); + + final dtd = await DartToolingDaemon.connect(uri); + + // Set up a periodic connection check to detect if the connection has + // dropped even if `done` doesn't fire. + // + // If this happens, just disconnect (without disabling reconnect) so the + // done event fires and then the usual handling occurs. + _periodicConnectionCheck = + Timer.periodic(_periodicConnectionCheckInterval, (timer) async { + if (_dtd.isClosed) { + _log.warning('The DTD connection has dropped'); + await disconnectImpl(allowReconnect: true); + } + }); + + return dtd; + } + + /// Triggers a reconnect to the last connected URI if the current state is + /// [ConnectionFailedDTDState] (and there was a pervious connection). + Future reconnect() { + final reconnectFunc = _lastConnectFunc; + if (_connectionState.value is! ConnectionFailedDTDState || + reconnectFunc == null) { + return Future.value(); + } + + return reconnectFunc(); + } + + /// Tries to connect to DTD at [uri] with automatic retries and exponential + /// backoff. + /// + /// When a computer sleeps, the WebSocket connection may be dropped and it + /// may take some time for a browser to allow network connections without + /// ERR_NETWORK_IO_SUSPENDED. + Future _connectWithRetries( + Uri uri, { + required int maxRetries, + }) async { + for (var attempt = 1; attempt <= maxRetries; attempt++) { + try { + _connectionState.value = ConnectingDTDState(); + // The await here is important so errors are handled by this catch! + return await connectDtdImpl(uri); + } catch (e, s) { + // On last attempt, fail and propagate the error. + if (attempt == maxRetries) { + _connectionState.value = ConnectionFailedDTDState(); + _log.severe('Failed to connect to DTD after $attempt attempts', e, s); + rethrow; + } + + // Otherwise, retry after a delay. + var delay = math.pow(2, attempt - 1).toInt(); + _log.info( + 'Failed to connect to DTD on attempt $attempt, ' + 'will retry in ${delay}s', + ); + while (delay > 0) { + _connectionState.value = WaitingToRetryDTDState(delay); + await Future.delayed(const Duration(seconds: 1)); + delay--; + } + } + } + + // We can't get here because of the logic above, but the analyzer can't + // tell that. + _connectionState.value = NotConnectedDTDState(); + throw StateError('Failed to connect to DTD'); + } + + /// Connects Dart Tooling Daemon connection to [uri]. + /// + /// Before connecting to [uri], unless [disconnectBeforeConnecting] is + /// `false`, will call [disconnect] to disconnect any existing connection. + /// + /// If the connection fails, will retry with exponential backoff up to + /// [maxRetries]. + Future _connectImpl( Uri uri, { void Function(Object, StackTrace?)? onError, + int maxRetries = 5, + bool disconnectBeforeConnecting = true, }) async { - await disconnect(); + if (disconnectBeforeConnecting) { + await disconnect(); + } + // Enable automatic reconnect on any new connection. + _automaticallyReconnect = true; try { - final connection = await DartToolingDaemon.connect(uri); + final connection = await _connectWithRetries(uri, maxRetries: maxRetries); + _uri = uri; // Set this after setting the value of [_uri] so that [_uri] can be used // by any listeners of the [_connection] notifier. _connection.value = connection; + _connectionState.value = ConnectedDTDState(); _log.info('Successfully connected to DTD at: $uri'); + + // If a connection drops (and we hadn't disabled auto-reconnect, such + // as by explicitly calling disconnect/dispose), we should attempt to + // reconnect. + unawaited(connection.done + .then((_) => _reconnectAfterDroppedConnection(uri, onError: onError)) + .catchError((_) { + // TODO(dantup): Create a devtools_app_shared version of safeUnawaited. + // https://github.com/flutter/devtools/pull/9587#discussion_r2624306047 + })); } catch (e, st) { onError?.call(e, st); } } + /// Triggers a reconnect without first disconnecting. This allows existing + /// state to be retained in the background while reconnect is in progress so + /// that the content the user could previously see is not hidden. + Future _reconnectAfterDroppedConnection( + Uri uri, { + void Function(Object, StackTrace?)? onError, + }) async { + // Trigger disconnect to ensure we emit a `null` connection to + // listeners. + await disconnectImpl(allowReconnect: true); + if (_automaticallyReconnect) { + await _connectImpl( + uri, + onError: onError, + // We've already disconnected above, in a way that doesn't disable + // reconnect and does not set connection to null (allowing screens + // to remain visible under connection overlays). + disconnectBeforeConnecting: false, + ); + } + } + + /// Sets the Dart Tooling Daemon connection to point to [uri]. + /// + /// Before connecting to [uri], if a current connection exists, then + /// [disconnect] is called to close it. + /// + /// If the connection fails, will retry with exponential backoff up to + /// [maxRetries]. + Future connect( + Uri uri, { + void Function(Object, StackTrace?)? onError, + int maxRetries = 5, + }) { + // On explicit connections, we capture the connect function so that we + // can call it again if [reconnect()] is called. + final connectFunc = _lastConnectFunc = + () => _connectImpl(uri, onError: onError, maxRetries: maxRetries); + return connectFunc(); + } + + /// Closes and unsets the Dart Tooling Daemon connection, if one is set. + Future disconnect() => disconnectImpl(); + /// Closes and unsets the Dart Tooling Daemon connection, if one is set. - Future disconnect() async { - if (_connection.value != null) { - await _connection.value!.close(); + /// + /// [allowReconnect] controls whether reconnection is allowed. This is + /// generally false for an explicit disconnect/dispose, but allowed if we + /// are called as part of a dropped connection. Reconnecting being allowed + /// does not necessarily mean it will happen, because there might have been + /// an explicit disconnect (or dispose) call before we got here. + @visibleForTesting + Future disconnectImpl({bool allowReconnect = false}) async { + if (!allowReconnect) { + // If we're not allowed to reconnect, disable this. `allowReconnect` being + // true does NOT mean we can enable this, because we might get here after + // an explicit disconnect. + _automaticallyReconnect = false; + + // We only clear the connection if we are explicitly disconnecting. In the + // case where the connection just dropped, we leave it so that we can + // continue to render a page (usually with an overlay). + _connection.value = null; + } + + _periodicConnectionCheck?.cancel(); + if (_connection.value case final connection?) { + await connection.close(); } - _connection.value = null; + _connectionState.value = NotConnectedDTDState(); _uri = null; _workspaceRoots = null; _projectRoots = null; diff --git a/packages/devtools_app_shared/lib/src/service/dtd_manager_connection_state.dart b/packages/devtools_app_shared/lib/src/service/dtd_manager_connection_state.dart new file mode 100644 index 00000000000..0f3d6008f76 --- /dev/null +++ b/packages/devtools_app_shared/lib/src/service/dtd_manager_connection_state.dart @@ -0,0 +1,33 @@ +// Copyright 2025 The Flutter Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file or at https://developers.google.com/open-source/licenses/bsd. + +/// A class representing the current state of a DTD connection. +sealed class DTDConnectionState {} + +/// DTD is not connected and has not started to connect. +class NotConnectedDTDState extends DTDConnectionState {} + +/// Attempting to connect to DTD. +class ConnectingDTDState extends DTDConnectionState {} + +/// A connection failed and we are waiting for [seconds] before +/// trying again. +/// +/// This state is emitted every second during a retry countdown. +class WaitingToRetryDTDState extends DTDConnectionState { + WaitingToRetryDTDState(this.seconds); + + /// The remaining number of seconds to wait. + /// + /// This value does not update, but the state is emitted for each second of + /// a retry countdown. + final int seconds; +} + +/// We are connected to DTD. +class ConnectedDTDState extends DTDConnectionState {} + +/// We failed to connect to DTD in the maximum number of retries and are no +/// longer trying to connect. +class ConnectionFailedDTDState extends DTDConnectionState {}