From ea2cd87b5e2aaade20cc2a2e35314ac7d7ad3f66 Mon Sep 17 00:00:00 2001 From: flemming-it Date: Sat, 18 Jul 2026 17:33:11 +0200 Subject: [PATCH] fix(runs): stop claiming the hub is unreachable on RPC-level errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The runs monitor folded every load failure into 'hub not reachable', contradicting the sidebar's green connected dot whenever the hub answered but the RPC failed — most visibly against a pre-0.22 hub whose version predates the ListInvocations RPC (UNIMPLEMENTED). Classify the failure instead (top-level, unit-tested): - UNIMPLEMENTED -> 'this view needs a newer hub version' with a doctor-page link (the update banner lives there) - FAILED_PRECONDITION from the detached gate -> the regular feature-off empty state with the guide button - UNAVAILABLE / DEADLINE_EXCEEDED / socket-level failures -> the honest 'hub not reachable' state (unchanged) - everything else -> a load-failed state with the friendly error and a copyable detail box The error view is a public callback-driven widget so the tests pump each variant without a live hub. New DE+EN strings for the too-old and load-failed states; grpcCodeOf/grpcMessageOf exposed from the friendly-error mapper instead of duplicating the duck-typing. Signed-off-by: flemming-it --- lib/data/friendly_error.dart | 10 ++ lib/l10n/app_de.arb | 4 + lib/l10n/app_en.arb | 4 + lib/l10n/app_localizations.dart | 24 ++++ lib/l10n/app_localizations_de.dart | 14 ++ lib/l10n/app_localizations_en.dart | 13 ++ lib/pages/runs.dart | 146 ++++++++++++++++++- test/runs_error_classification_test.dart | 172 +++++++++++++++++++++++ 8 files changed, 380 insertions(+), 7 deletions(-) create mode 100644 test/runs_error_classification_test.dart diff --git a/lib/data/friendly_error.dart b/lib/data/friendly_error.dart index d12efce..e415485 100644 --- a/lib/data/friendly_error.dart +++ b/lib/data/friendly_error.dart @@ -217,6 +217,16 @@ FriendlyError? _matchHubPattern(String detail, AppLocalizations l) { return null; } +/// Duck-typed `GrpcError.code` reader — public so pages that +/// classify errors themselves (e.g. the runs monitor separating +/// "hub down" from "hub too old") share one accessor instead of +/// re-implementing the duck-typing. +int? grpcCodeOf(Object error) => _intField(error, 'code'); + +/// Duck-typed `GrpcError.message` reader — companion to +/// [grpcCodeOf]. +String? grpcMessageOf(Object error) => _stringField(error, 'message'); + /// Try to read an `int` field by name off an arbitrary object. /// Returns `null` when the field doesn't exist or has another /// runtime type. Used to duck-type `GrpcError.code` without diff --git a/lib/l10n/app_de.arb b/lib/l10n/app_de.arb index 32357d0..3021d67 100644 --- a/lib/l10n/app_de.arb +++ b/lib/l10n/app_de.arb @@ -1773,6 +1773,10 @@ "runsEmptyHint": "Hier erscheinen Läufe, die im Hintergrund weiterlaufen, während Sie anderes tun. Diese Funktion ist optional und standardmäßig ausgeschaltet — die Anleitung zeigt Schritt für Schritt, wie sie eingeschaltet wird.", "runsEmptyEnabledHint": "Läufe im Hintergrund sind eingeschaltet — es wurde nur noch keiner gestartet. Starten Sie einen Flow mit der Option „im Hintergrund ausführen“, dann erscheint er hier.", "runsEmptyGuideButton": "Anleitung öffnen", + "runsHubTooOldTitle": "Diese Ansicht braucht eine neuere Hub-Version", + "runsHubTooOldHint": "Der Hub ist verbunden, aber seine Version kennt die Laufübersicht noch nicht. Aktualisieren Sie den Hub, dann erscheinen die Läufe hier.", + "runsHubTooOldButton": "Diagnose öffnen", + "runsLoadFailedTitle": "Läufe konnten nicht geladen werden", "runsCancelButton": "Abbrechen", "runsCancelSignalled": "Abbruch für {flow} ausgelöst.", "@runsCancelSignalled": {"placeholders": {"flow": {"type": "String"}}}, diff --git a/lib/l10n/app_en.arb b/lib/l10n/app_en.arb index c08eea7..3b15097 100644 --- a/lib/l10n/app_en.arb +++ b/lib/l10n/app_en.arb @@ -1812,6 +1812,10 @@ "runsEmptyHint": "Runs that keep working in the background while you do something else appear here. The feature is optional and off by default — the guide shows step by step how to turn it on.", "runsEmptyEnabledHint": "Background runs are switched on — none has been started yet. Start a flow with the \"run in background\" option and it will appear here.", "runsEmptyGuideButton": "Open the guide", + "runsHubTooOldTitle": "This view needs a newer hub version", + "runsHubTooOldHint": "The hub is connected, but its version does not know the runs monitor yet. Update the hub and the runs will appear here.", + "runsHubTooOldButton": "Open Doctor", + "runsLoadFailedTitle": "Runs could not be loaded", "runsCancelButton": "Cancel", "runsCancelSignalled": "Cancel signalled for {flow}.", "@runsCancelSignalled": {"placeholders": {"flow": {"type": "String"}}}, diff --git a/lib/l10n/app_localizations.dart b/lib/l10n/app_localizations.dart index db946a6..07d265b 100644 --- a/lib/l10n/app_localizations.dart +++ b/lib/l10n/app_localizations.dart @@ -5401,6 +5401,30 @@ abstract class AppLocalizations { /// **'Open the guide'** String get runsEmptyGuideButton; + /// No description provided for @runsHubTooOldTitle. + /// + /// In en, this message translates to: + /// **'This view needs a newer hub version'** + String get runsHubTooOldTitle; + + /// No description provided for @runsHubTooOldHint. + /// + /// In en, this message translates to: + /// **'The hub is connected, but its version does not know the runs monitor yet. Update the hub and the runs will appear here.'** + String get runsHubTooOldHint; + + /// No description provided for @runsHubTooOldButton. + /// + /// In en, this message translates to: + /// **'Open Doctor'** + String get runsHubTooOldButton; + + /// No description provided for @runsLoadFailedTitle. + /// + /// In en, this message translates to: + /// **'Runs could not be loaded'** + String get runsLoadFailedTitle; + /// No description provided for @runsCancelButton. /// /// In en, this message translates to: diff --git a/lib/l10n/app_localizations_de.dart b/lib/l10n/app_localizations_de.dart index f714a53..87f06e3 100644 --- a/lib/l10n/app_localizations_de.dart +++ b/lib/l10n/app_localizations_de.dart @@ -3198,6 +3198,20 @@ class AppLocalizationsDe extends AppLocalizations { @override String get runsEmptyGuideButton => 'Anleitung öffnen'; + @override + String get runsHubTooOldTitle => + 'Diese Ansicht braucht eine neuere Hub-Version'; + + @override + String get runsHubTooOldHint => + 'Der Hub ist verbunden, aber seine Version kennt die Laufübersicht noch nicht. Aktualisieren Sie den Hub, dann erscheinen die Läufe hier.'; + + @override + String get runsHubTooOldButton => 'Diagnose öffnen'; + + @override + String get runsLoadFailedTitle => 'Läufe konnten nicht geladen werden'; + @override String get runsCancelButton => 'Abbrechen'; diff --git a/lib/l10n/app_localizations_en.dart b/lib/l10n/app_localizations_en.dart index 2e6ee2c..2a2fa2a 100644 --- a/lib/l10n/app_localizations_en.dart +++ b/lib/l10n/app_localizations_en.dart @@ -3195,6 +3195,19 @@ class AppLocalizationsEn extends AppLocalizations { @override String get runsEmptyGuideButton => 'Open the guide'; + @override + String get runsHubTooOldTitle => 'This view needs a newer hub version'; + + @override + String get runsHubTooOldHint => + 'The hub is connected, but its version does not know the runs monitor yet. Update the hub and the runs will appear here.'; + + @override + String get runsHubTooOldButton => 'Open Doctor'; + + @override + String get runsLoadFailedTitle => 'Runs could not be loaded'; + @override String get runsCancelButton => 'Cancel'; diff --git a/lib/pages/runs.dart b/lib/pages/runs.dart index 6f458c0..64fdf57 100644 --- a/lib/pages/runs.dart +++ b/lib/pages/runs.dart @@ -3,9 +3,11 @@ import 'dart:async'; import 'package:flutter/material.dart'; import '../data/error_presentation.dart'; +import '../data/friendly_error.dart'; import '../data/hub.dart'; import '../data/workspace.dart'; import '../l10n/app_localizations.dart'; +import '../main.dart' show StudioShellState; import '../theme/tokens.dart'; import '../widgets/widgets.dart'; import 'welcome.dart' show showFaiDoc; @@ -22,10 +24,138 @@ class RunsPage extends StatefulWidget { State createState() => _RunsPageState(); } +/// Why the runs monitor could not load its list. The page used to +/// fold every failure into "hub not reachable", which contradicted +/// the sidebar's green "connected" dot whenever the hub answered +/// with an RPC-level error (usertest finding: a 0.21 hub without +/// the ListInvocations RPC). Top-level so the unit test drives the +/// classification directly. +enum RunsLoadIssue { + /// The hub itself cannot be reached (socket-level failure or + /// gRPC UNAVAILABLE / DEADLINE_EXCEEDED). + unreachable, + + /// The hub answered, but its version predates the + /// ListInvocations RPC (gRPC UNIMPLEMENTED) — connected, just + /// too old for this view. + unsupported, + + /// The hub answered and refused because detached invocations + /// are switched off (gRPC FAILED_PRECONDITION from the detached + /// gate) — show the regular feature-off empty state, not a + /// connection error. + featureDisabled, + + /// Anything else — show the friendly error with copyable detail. + other, +} + +/// Classify a [HubService.listDetachedRuns] failure into the view +/// state the page should render. +RunsLoadIssue classifyRunsLoadError(Object error) { + switch (grpcCodeOf(error)) { + case 12: // UNIMPLEMENTED — hub predates the RPC + return RunsLoadIssue.unsupported; + case 9: // FAILED_PRECONDITION — the hub's detached gate + final msg = (grpcMessageOf(error) ?? '').toLowerCase(); + return msg.contains('detached') + ? RunsLoadIssue.featureDisabled + : RunsLoadIssue.other; + case 4: // DEADLINE_EXCEEDED + case 14: // UNAVAILABLE + return RunsLoadIssue.unreachable; + } + // Non-gRPC failures: only clear socket-level shapes count as + // "unreachable"; everything else keeps its real story. + final s = error.toString().toLowerCase(); + if (s.contains('socketexception') || + s.contains('connection refused') || + s.contains('connection terminated') || + s.contains('failed to connect')) { + return RunsLoadIssue.unreachable; + } + return RunsLoadIssue.other; +} + +/// The error rendition of the runs monitor — public + callback-driven +/// so the widget test can pump each variant without a live hub. +class RunsLoadErrorView extends StatelessWidget { + final Object error; + final RunsLoadIssue issue; + + /// Opens the runs guide (the feature-off empty state's CTA). + final VoidCallback onOpenGuide; + + /// Opens the doctor page (the "hub too old" state's CTA, where + /// the update banner lives). Null hides the button. + final VoidCallback? onOpenDoctor; + + const RunsLoadErrorView({ + super.key, + required this.error, + required this.issue, + required this.onOpenGuide, + this.onOpenDoctor, + }); + + @override + Widget build(BuildContext context) { + final theme = Theme.of(context); + final l = AppLocalizations.of(context)!; + switch (issue) { + case RunsLoadIssue.featureDisabled: + // Same story as the regular feature-off empty state: the + // hub is fine, the operator just has not enabled the + // feature — never claim "not reachable" here. + return ChainEmptyState( + icon: Icons.rocket_launch_outlined, + title: l.runsEmptyTitle, + hint: l.runsEmptyHint, + action: OutlinedButton.icon( + icon: const Icon(Icons.menu_book_outlined, size: 16), + label: Text(l.runsEmptyGuideButton), + onPressed: onOpenGuide, + ), + ); + case RunsLoadIssue.unsupported: + return ChainEmptyState( + icon: Icons.system_update_alt_outlined, + title: l.runsHubTooOldTitle, + hint: l.runsHubTooOldHint, + action: onOpenDoctor == null + ? null + : OutlinedButton.icon( + icon: const Icon(Icons.health_and_safety_outlined, size: 16), + label: Text(l.runsHubTooOldButton), + onPressed: onOpenDoctor, + ), + ); + case RunsLoadIssue.unreachable: + return ChainEmptyState( + icon: Icons.cloud_off_outlined, + iconColor: theme.colorScheme.error, + title: l.hubUnreachable, + hint: l.hubUnreachableHint, + ); + case RunsLoadIssue.other: + return ChainEmptyState( + icon: Icons.error_outline, + iconColor: theme.colorScheme.error, + title: l.runsLoadFailedTitle, + action: ConstrainedBox( + constraints: const BoxConstraints(maxWidth: 420), + child: ChainErrorBox(error: error, isError: true), + ), + ); + } + } +} + class _RunsPageState extends State { List _runs = const []; bool _detachedEnabled = false; - String? _error; + Object? _error; + RunsLoadIssue _issue = RunsLoadIssue.other; bool _loaded = false; Timer? _poll; final Set _cancelling = {}; @@ -63,7 +193,8 @@ class _RunsPageState extends State { } catch (e) { if (!mounted) return; setState(() { - _error = e.toString(); + _error = e; + _issue = classifyRunsLoadError(e); _loaded = true; }); } @@ -118,11 +249,12 @@ class _RunsPageState extends State { body: !_loaded ? const Center(child: CircularProgressIndicator()) : _error != null && _runs.isEmpty - ? ChainEmptyState( - icon: Icons.cloud_off_outlined, - iconColor: theme.colorScheme.error, - title: l.hubUnreachable, - hint: l.hubUnreachableHint, + ? RunsLoadErrorView( + error: _error!, + issue: _issue, + onOpenGuide: () => showFaiDoc(context, 'runs'), + onOpenDoctor: () => + StudioShellState.of(context)?.navigateTo('doctor'), ) : _runs.isEmpty ? ChainEmptyState( diff --git a/test/runs_error_classification_test.dart b/test/runs_error_classification_test.dart new file mode 100644 index 0000000..0249aab --- /dev/null +++ b/test/runs_error_classification_test.dart @@ -0,0 +1,172 @@ +// Runs-monitor error classification — the page used to render +// every load failure as "hub not reachable", contradicting the +// sidebar's green "connected" dot whenever the hub answered but +// the RPC failed (usertest finding: a 0.21 hub without the +// ListInvocations RPC answered UNIMPLEMENTED and the page claimed +// the hub was down). These tests pin the classification and the +// widget rendition of each state. + +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; + +import 'package:chain_studio/l10n/app_localizations.dart'; +import 'package:chain_studio/pages/runs.dart'; + +/// Duck-typed stand-in for `GrpcError` — the classifier reads +/// `.code` and `.message` off whatever object arrives, exactly +/// like the friendly-error mapper does. +class _FakeGrpcError { + final int code; + final String? message; + const _FakeGrpcError(this.code, [this.message]); + + @override + String toString() => 'gRPC Error (code: $code, message: $message)'; +} + +Widget _host(Widget child) => MaterialApp( + localizationsDelegates: AppLocalizations.localizationsDelegates, + supportedLocales: AppLocalizations.supportedLocales, + locale: const Locale('de'), + home: Scaffold(body: child), +); + +void main() { + group('classifyRunsLoadError', () { + test('UNIMPLEMENTED means the hub is too old, not unreachable', () { + expect( + classifyRunsLoadError( + const _FakeGrpcError(12, 'grpc.Hub/ListInvocations unimplemented'), + ), + RunsLoadIssue.unsupported, + ); + }); + + test('FAILED_PRECONDITION from the detached gate is feature-off', () { + expect( + classifyRunsLoadError( + const _FakeGrpcError( + 9, + 'detached invocations are not enabled — set detached.enabled: ' + 'true in the operator config', + ), + ), + RunsLoadIssue.featureDisabled, + ); + }); + + test('other FAILED_PRECONDITION errors keep their own story', () { + expect( + classifyRunsLoadError(const _FakeGrpcError(9, 'store busy')), + RunsLoadIssue.other, + ); + }); + + test('UNAVAILABLE and DEADLINE_EXCEEDED are unreachable', () { + expect( + classifyRunsLoadError(const _FakeGrpcError(14, 'connection refused')), + RunsLoadIssue.unreachable, + ); + expect( + classifyRunsLoadError(const _FakeGrpcError(4, 'deadline exceeded')), + RunsLoadIssue.unreachable, + ); + }); + + test('socket-level failures without a gRPC code are unreachable', () { + expect( + classifyRunsLoadError( + Exception('SocketException: Connection refused (port 50051)'), + ), + RunsLoadIssue.unreachable, + ); + }); + + test('arbitrary errors fall through to other', () { + expect( + classifyRunsLoadError(const FormatException('bad payload')), + RunsLoadIssue.other, + ); + }); + }); + + group('RunsLoadErrorView', () { + testWidgets('feature-off renders the plain-language empty state, ' + 'never "not reachable"', (tester) async { + await tester.pumpWidget( + _host( + RunsLoadErrorView( + error: const _FakeGrpcError( + 9, + 'detached invocations are not enabled', + ), + issue: RunsLoadIssue.featureDisabled, + onOpenGuide: () {}, + ), + ), + ); + await tester.pumpAndSettle(); + expect(find.text('Keine Läufe im Hintergrund'), findsOneWidget); + expect(find.text('Anleitung öffnen'), findsOneWidget); + expect(find.text('Hub nicht erreichbar'), findsNothing); + }); + + testWidgets('too-old hub says so and links to the doctor page', ( + tester, + ) async { + var doctorOpened = false; + await tester.pumpWidget( + _host( + RunsLoadErrorView( + error: const _FakeGrpcError(12, 'unimplemented'), + issue: RunsLoadIssue.unsupported, + onOpenGuide: () {}, + onOpenDoctor: () => doctorOpened = true, + ), + ), + ); + await tester.pumpAndSettle(); + expect( + find.text('Diese Ansicht braucht eine neuere Hub-Version'), + findsOneWidget, + ); + expect(find.text('Hub nicht erreichbar'), findsNothing); + await tester.tap(find.text('Diagnose öffnen')); + expect(doctorOpened, isTrue); + }); + + testWidgets('a genuinely unreachable hub still says unreachable', ( + tester, + ) async { + await tester.pumpWidget( + _host( + RunsLoadErrorView( + error: const _FakeGrpcError(14, 'connection refused'), + issue: RunsLoadIssue.unreachable, + onOpenGuide: () {}, + ), + ), + ); + await tester.pumpAndSettle(); + expect(find.text('Hub nicht erreichbar'), findsOneWidget); + }); + + testWidgets('other errors surface a copyable detail box', (tester) async { + await tester.pumpWidget( + _host( + RunsLoadErrorView( + error: const _FakeGrpcError(13, 'internal boom'), + issue: RunsLoadIssue.other, + onOpenGuide: () {}, + ), + ), + ); + await tester.pumpAndSettle(); + expect(find.text('Läufe konnten nicht geladen werden'), findsOneWidget); + expect(find.text('Hub nicht erreichbar'), findsNothing); + // The copy affordance is the hard rule: the detail box must + // be present so the operator can copy the real message. + expect(find.byIcon(Icons.content_copy), findsWidgets); + }); + }); +}