From c1cccb6e0758591dd348af091880a1bc9dad3b7c Mon Sep 17 00:00:00 2001 From: Carlos Date: Mon, 23 Oct 2023 13:28:48 -0300 Subject: [PATCH 01/12] Makes agent recovery automatic if unreachable When the GUI finds the addr file, but the agent doesn't respond The GUI assumes the agent crashed or was killed some time in the past. A button was offered to allow the user to request a recovery action. The recovery action consists in: - deleting the addr file and - reset internal state to try again. What changed: now that action is taken automatically. --- .../ubuntupro/lib/pages/startup/startup_model.dart | 7 +++++++ .../ubuntupro/lib/pages/startup/startup_page.dart | 13 ++++--------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/gui/packages/ubuntupro/lib/pages/startup/startup_model.dart b/gui/packages/ubuntupro/lib/pages/startup/startup_model.dart index 55183e499..149f5ad0e 100644 --- a/gui/packages/ubuntupro/lib/pages/startup/startup_model.dart +++ b/gui/packages/ubuntupro/lib/pages/startup/startup_model.dart @@ -43,6 +43,8 @@ class StartupModel extends ChangeNotifier { ViewState get view => _view; StreamSubscription? _subs; + static const _retryLimit = 5; + int _retries = 0; /// Starts the monitor and subscribes to its events. Returns a future that /// completes when the agent monitor startup routine completes. @@ -67,6 +69,11 @@ class StartupModel extends ChangeNotifier { _view == ViewState.retry, "resetAgent only if it's possible to retry", ); + if (_retries >= _retryLimit) { + _view = ViewState.crash; + return; + } + ++_retries; await monitor.reset(); await _subs?.cancel(); return init(); diff --git a/gui/packages/ubuntupro/lib/pages/startup/startup_page.dart b/gui/packages/ubuntupro/lib/pages/startup/startup_page.dart index b5ede7079..eed3f29d8 100644 --- a/gui/packages/ubuntupro/lib/pages/startup/startup_page.dart +++ b/gui/packages/ubuntupro/lib/pages/startup/startup_page.dart @@ -54,26 +54,21 @@ class _StartupAnimatedChildState extends State { if (model.view == ViewState.ok) { Navigator.of(context).pushReplacementNamed(widget.nextRoute); } + if (model.view == ViewState.retry) { + model.resetAgent(); + } }); } Widget buildChild(ViewState view, String message) { switch (view) { case ViewState.inProgress: + case ViewState.retry: return StartupInProgressWidget(message); case ViewState.ok: return const SizedBox.shrink(); - case ViewState.retry: - return StartupRetryWidget( - message: message, - retry: OutlinedButton( - onPressed: context.read().resetAgent, - child: Text(AppLocalizations.of(context).agentRetryButton), - ), - ); - case ViewState.crash: return StartupErrorWidget(message); } From 32bf3d12c70238c65ac00c58ad5ee2a69e03b845 Mon Sep 17 00:00:00 2001 From: Carlos Date: Mon, 23 Oct 2023 13:41:32 -0300 Subject: [PATCH 02/12] Update l10n strings --- gui/packages/ubuntupro/lib/l10n/app_en.arb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gui/packages/ubuntupro/lib/l10n/app_en.arb b/gui/packages/ubuntupro/lib/l10n/app_en.arb index 2dbd6441b..7906d03a3 100644 --- a/gui/packages/ubuntupro/lib/l10n/app_en.arb +++ b/gui/packages/ubuntupro/lib/l10n/app_en.arb @@ -24,7 +24,7 @@ "agentStateCannotStart": "Ubuntu Pro background agent cannot be started.", "agentStateUnknownEnv": "Ubuntu Pro background agent state cannot be verified. Check your environment settings.", "agentStateQuerying": "Checking Ubuntu Pro background agent's state.", - "agentStateUnreachable": "Ubuntu Pro background agent is unreachable.", + "agentStateUnreachable": "Attempting to recover Ubuntu Pro background agent.", "agentRetryButton": "Click to restart it", "proHeading": "The most comprehensive subscription\nfor open-source software security\nnow available on WSL", From b5c1f0366fee4b025467d2f8580c3c4d1da9d009 Mon Sep 17 00:00:00 2001 From: Carlos Date: Mon, 23 Oct 2023 13:34:48 -0300 Subject: [PATCH 03/12] Removes no longer applicable test case --- .../test/startup/startup_page_test.dart | 26 ------------------- 1 file changed, 26 deletions(-) diff --git a/gui/packages/ubuntupro/test/startup/startup_page_test.dart b/gui/packages/ubuntupro/test/startup/startup_page_test.dart index 98f009744..73069a5cc 100644 --- a/gui/packages/ubuntupro/test/startup/startup_page_test.dart +++ b/gui/packages/ubuntupro/test/startup/startup_page_test.dart @@ -58,32 +58,6 @@ void main() { expect(find.text(lastText), findsOneWidget); }); - testWidgets('button for retry', (tester) async { - final monitor = MockAgentStartupMonitor(); - when(monitor.start()).thenAnswer( - (_) => Stream.fromIterable( - [ - AgentState.querying, - AgentState.starting, - AgentState.invalid, - AgentState.unreachable, - ], - ), - ); - final model = StartupModel(monitor); - await tester.pumpWidget(buildApp(model)); - - await model.init(); - await tester.pumpAndSettle(); - - // no success - expect(find.text(lastText), findsNothing); - // show error icon - expect(find.byIcon(Icons.error_outline), findsOneWidget); - // and a retry button - expect(find.byType(OutlinedButton), findsOneWidget); - }); - testWidgets('terminal error no button', (tester) async { final monitor = MockAgentStartupMonitor(); when(monitor.start()).thenAnswer( From 12a88c131a1d09c8a3aad8abd9abc4b4ec849f05 Mon Sep 17 00:00:00 2001 From: Carlos Date: Mon, 23 Oct 2023 13:36:34 -0300 Subject: [PATCH 04/12] Prevents pumpAndSettle timeout Since we now retry many times --- gui/packages/ubuntupro/test/startup/startup_page_test.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/gui/packages/ubuntupro/test/startup/startup_page_test.dart b/gui/packages/ubuntupro/test/startup/startup_page_test.dart index 73069a5cc..20e27b874 100644 --- a/gui/packages/ubuntupro/test/startup/startup_page_test.dart +++ b/gui/packages/ubuntupro/test/startup/startup_page_test.dart @@ -108,7 +108,6 @@ void main() { ); await tester.pumpWidget(app); - await tester.pumpAndSettle(); final context = tester.element(find.byType(StartupAnimatedChild)); final model = Provider.of(context, listen: false); From 52894d86e1d687302eb4b28f95407bd8904ff17b Mon Sep 17 00:00:00 2001 From: Carlos Date: Mon, 23 Oct 2023 14:06:16 -0300 Subject: [PATCH 05/12] Makes window decoration visible when in progress That allows the window to resize and move around. --- gui/packages/ubuntupro/lib/pages/startup/startup_widgets.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/gui/packages/ubuntupro/lib/pages/startup/startup_widgets.dart b/gui/packages/ubuntupro/lib/pages/startup/startup_widgets.dart index 88c0e0e3f..a3b4dd670 100644 --- a/gui/packages/ubuntupro/lib/pages/startup/startup_widgets.dart +++ b/gui/packages/ubuntupro/lib/pages/startup/startup_widgets.dart @@ -57,7 +57,6 @@ class StartupInProgressWidget extends StatelessWidget { message: message, bottom: const LinearProgressIndicator(), ), - showTitleBar: false, ); } } From 579caa23dc2490a806d3a7add29a5c82696e50e4 Mon Sep 17 00:00:00 2001 From: Carlos Date: Mon, 23 Oct 2023 14:06:50 -0300 Subject: [PATCH 06/12] Removes no longer needed StartupRetry widget --- .../lib/pages/startup/startup_widgets.dart | 23 ------------------- 1 file changed, 23 deletions(-) diff --git a/gui/packages/ubuntupro/lib/pages/startup/startup_widgets.dart b/gui/packages/ubuntupro/lib/pages/startup/startup_widgets.dart index a3b4dd670..866591f5b 100644 --- a/gui/packages/ubuntupro/lib/pages/startup/startup_widgets.dart +++ b/gui/packages/ubuntupro/lib/pages/startup/startup_widgets.dart @@ -77,26 +77,3 @@ class StartupErrorWidget extends StatelessWidget { ); } } - -/// Displays an error icon followed by the [errorMessage] and a button allowing -/// users to manually request a reset/retry operation. -class StartupRetryWidget extends StatelessWidget { - const StartupRetryWidget({ - super.key, - required this.message, - required this.retry, - }); - final String message; - final Widget retry; - - @override - Widget build(BuildContext context) { - return Pro4WindowsPage( - body: StatusColumn( - top: const Icon(Icons.error_outline, size: 64), - message: message, - bottom: retry, - ), - ); - } -} From 09d1dbb3436815e998e27e97854db583a270ddc9 Mon Sep 17 00:00:00 2001 From: Carlos Date: Mon, 23 Oct 2023 14:07:10 -0300 Subject: [PATCH 07/12] Removes no longer relevant tests --- .../test/startup/startup_widgets_test.dart | 32 ------------------- 1 file changed, 32 deletions(-) delete mode 100644 gui/packages/ubuntupro/test/startup/startup_widgets_test.dart diff --git a/gui/packages/ubuntupro/test/startup/startup_widgets_test.dart b/gui/packages/ubuntupro/test/startup/startup_widgets_test.dart deleted file mode 100644 index 9c2d49dd2..000000000 --- a/gui/packages/ubuntupro/test/startup/startup_widgets_test.dart +++ /dev/null @@ -1,32 +0,0 @@ -import 'package:flutter/material.dart'; -import 'package:flutter_gen/gen_l10n/app_localizations.dart'; -import 'package:flutter_test/flutter_test.dart'; -import 'package:ubuntupro/pages/startup/startup_widgets.dart'; - -void main() { - const message = 'Hello'; - MaterialApp buildApp(Widget home) => MaterialApp( - home: home, - localizationsDelegates: AppLocalizations.localizationsDelegates, - ); - testWidgets('inprogress no appbar', (tester) async { - await tester.pumpWidget(buildApp(const StartupInProgressWidget(message))); - expect(find.byType(AppBar), findsNothing); - }); - testWidgets('retry shows appbar', (tester) async { - await tester.pumpWidget( - buildApp( - const StartupRetryWidget( - message: message, - retry: Icon(Icons.check), - ), - ), - ); - expect(find.byType(AppBar), findsOneWidget); - }); - - testWidgets('error also shows appbar', (tester) async { - await tester.pumpWidget(buildApp(const StartupErrorWidget(message))); - expect(find.byType(AppBar), findsOneWidget); - }); -} From 9b98d1994cd799276cd9dff55594ddf34be11a69 Mon Sep 17 00:00:00 2001 From: Carlos Date: Tue, 24 Oct 2023 14:49:53 -0300 Subject: [PATCH 08/12] Notify if retries exceed --- gui/packages/ubuntupro/lib/pages/startup/startup_model.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/gui/packages/ubuntupro/lib/pages/startup/startup_model.dart b/gui/packages/ubuntupro/lib/pages/startup/startup_model.dart index 149f5ad0e..8a68a6395 100644 --- a/gui/packages/ubuntupro/lib/pages/startup/startup_model.dart +++ b/gui/packages/ubuntupro/lib/pages/startup/startup_model.dart @@ -71,6 +71,7 @@ class StartupModel extends ChangeNotifier { ); if (_retries >= _retryLimit) { _view = ViewState.crash; + notifyListeners(); return; } ++_retries; From e26a0b5bb9869d4247f7d728fa5d80cc948603a2 Mon Sep 17 00:00:00 2001 From: Carlos Date: Tue, 24 Oct 2023 14:51:05 -0300 Subject: [PATCH 09/12] Make the listener async --- gui/packages/ubuntupro/lib/pages/startup/startup_page.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gui/packages/ubuntupro/lib/pages/startup/startup_page.dart b/gui/packages/ubuntupro/lib/pages/startup/startup_page.dart index eed3f29d8..e04f2fffb 100644 --- a/gui/packages/ubuntupro/lib/pages/startup/startup_page.dart +++ b/gui/packages/ubuntupro/lib/pages/startup/startup_page.dart @@ -50,12 +50,12 @@ class _StartupAnimatedChildState extends State { super.initState(); final model = context.read(); model.init(); - model.addListener(() { + model.addListener(() async { if (model.view == ViewState.ok) { - Navigator.of(context).pushReplacementNamed(widget.nextRoute); + await Navigator.of(context).pushReplacementNamed(widget.nextRoute); } if (model.view == ViewState.retry) { - model.resetAgent(); + await model.resetAgent(); } }); } From 72eb26a5925d36731f23da0b75f43f0cbc79eb50 Mon Sep 17 00:00:00 2001 From: Carlos Date: Mon, 30 Oct 2023 01:19:27 -0300 Subject: [PATCH 10/12] Ok to race with the agent when deleting addr file Filesystem on Windows can be tricky. Autorestart the agent in loop opens possibility for not found exceptions That shouldn't be a problem. --- .../ubuntupro/lib/pages/startup/agent_monitor.dart | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/gui/packages/ubuntupro/lib/pages/startup/agent_monitor.dart b/gui/packages/ubuntupro/lib/pages/startup/agent_monitor.dart index 962557e6f..7d2b2ee68 100644 --- a/gui/packages/ubuntupro/lib/pages/startup/agent_monitor.dart +++ b/gui/packages/ubuntupro/lib/pages/startup/agent_monitor.dart @@ -161,7 +161,15 @@ class AgentStartupMonitor { /// Thus, we delete the existing `addr` file and retry launching the agent. Future reset() async { if (_addrFilePath != null) { - await File(_addrFilePath!).delete(); + try { + await File(_addrFilePath!).delete(); + } on PathNotFoundException { + // TODO: Log + // ignore: avoid_print + print( + 'Port file expected but not found. Likely a race with the agent at this point, not an issue.', + ); + } } } } From 841041a95307d417fda6548f65726fd657cec9b0 Mon Sep 17 00:00:00 2001 From: Carlos Date: Tue, 31 Oct 2023 00:29:29 -0300 Subject: [PATCH 11/12] Removes no longer needed button string --- gui/packages/ubuntupro/lib/l10n/app_en.arb | 1 - 1 file changed, 1 deletion(-) diff --git a/gui/packages/ubuntupro/lib/l10n/app_en.arb b/gui/packages/ubuntupro/lib/l10n/app_en.arb index 7906d03a3..9fd8d0581 100644 --- a/gui/packages/ubuntupro/lib/l10n/app_en.arb +++ b/gui/packages/ubuntupro/lib/l10n/app_en.arb @@ -25,7 +25,6 @@ "agentStateUnknownEnv": "Ubuntu Pro background agent state cannot be verified. Check your environment settings.", "agentStateQuerying": "Checking Ubuntu Pro background agent's state.", "agentStateUnreachable": "Attempting to recover Ubuntu Pro background agent.", - "agentRetryButton": "Click to restart it", "proHeading": "The most comprehensive subscription\nfor open-source software security\nnow available on WSL", "subscribeNow": "Subscribe Now", From 797cb02b105501914c92aeeddd24410bf97780e7 Mon Sep 17 00:00:00 2001 From: Carlos Date: Tue, 31 Oct 2023 00:41:46 -0300 Subject: [PATCH 12/12] Adds an attempt to kill a stuck agent Before launching it. This way we further ensure there will never be two agent's started by the same GUI process. --- gui/packages/ubuntupro/lib/launch_agent.dart | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gui/packages/ubuntupro/lib/launch_agent.dart b/gui/packages/ubuntupro/lib/launch_agent.dart index aa39197f5..cb2d088db 100644 --- a/gui/packages/ubuntupro/lib/launch_agent.dart +++ b/gui/packages/ubuntupro/lib/launch_agent.dart @@ -15,6 +15,8 @@ import 'core/environment.dart'; Future launchAgent(String agentRelativePath) async { final agentPath = p.join(msixRootDir().path, agentRelativePath); try { + // Attempts to kill a possibly stuck agent. Failure is desirable in this case. + await Process.run('taskkill.exe', ['/f', '/im', p.basename(agentPath)]); await Process.start( agentPath, ['-vv'],