From 66e9a9c7bea88576fdd17f1d747c4d350440bd03 Mon Sep 17 00:00:00 2001 From: Hank Yu Date: Tue, 14 Jul 2026 09:37:25 +0800 Subject: [PATCH 1/3] fix(pnp): distinguish router read failure from no-internet (#1098) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PnP internet check collapsed two distinct outcomes into NoInternet: a genuine "no internet" (WAN read succeeds, Status != 'Up') and a read failure (USP GET returns empty / missing fields → WanStatus.fetch throws). The two are already separated by control flow — return false vs throw — but the catch block discarded that distinction, so "Try again" could never recover while GETs stayed empty, and the no-internet troubleshooter options (restart modem / enter ISP settings) were shown for a read failure where they are meaningless. Split the two paths: return false → NoInternet (unchanged); throw → new AdminReadFailure phase, carrying the ServiceError code/detail for diagnostics and rendered as a plain error card + retry on the entry view (no redirect to the no-internet hub). Both startPostLoginFlow and _checkInternet now branch on ServiceError, so a read failure at either the SystemInfo or WanStatus step lands in the same phase. AdminReadFailure fully replaces the former AdminError. --- lib/page/instant_setup/models/pnp_state.dart | 22 +++++++++---- .../instant_setup/providers/pnp_notifier.dart | 23 +++++++++++-- .../instant_setup/views/pnp_entry_view.dart | 9 ++++-- .../providers/pnp_notifier_test.dart | 32 ++++++++++++++++--- 4 files changed, 70 insertions(+), 16 deletions(-) diff --git a/lib/page/instant_setup/models/pnp_state.dart b/lib/page/instant_setup/models/pnp_state.dart index 5b5f7920d..49e9f8d6a 100644 --- a/lib/page/instant_setup/models/pnp_state.dart +++ b/lib/page/instant_setup/models/pnp_state.dart @@ -71,12 +71,22 @@ class AdminInternetConnected extends PnpPhase { List get props => []; } -/// Critical error in admin phase. -class AdminError extends PnpPhase { - final String message; - const AdminError({required this.message}); - @override - List get props => [message]; +/// Router state could not be read (USP GET returned empty / missing fields). +/// +/// Distinct from [NoInternet]: the router did not confirm "no internet" — the +/// read itself failed, so we cannot tell the WAN state at all. The no-internet +/// troubleshooter options (restart modem / enter ISP settings) are meaningless +/// here, so this phase renders its own error card with a plain retry instead. +/// +/// [code] / [detail] carry the underlying [ServiceError] diagnostics (e.g. the +/// codegen 9998 "required fields missing" fault) for logging only — the UI +/// derives its message from the phase itself. +class AdminReadFailure extends PnpPhase { + final int? code; + final String? detail; + const AdminReadFailure({this.code, this.detail}); + @override + List get props => [code, detail]; } /// No internet detected — route to troubleshooter. diff --git a/lib/page/instant_setup/providers/pnp_notifier.dart b/lib/page/instant_setup/providers/pnp_notifier.dart index e77e9b145..ef6317453 100644 --- a/lib/page/instant_setup/providers/pnp_notifier.dart +++ b/lib/page/instant_setup/providers/pnp_notifier.dart @@ -1,6 +1,7 @@ import 'dart:async'; import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:privacy_gui/core/errors/service_error.dart'; import 'package:privacy_gui/core/usp/providers/usp_auth_coordinator.dart'; import 'package:privacy_gui/core/usp/providers/usp_mutation_lock.dart'; import 'package:privacy_gui/core/session/providers/session_provider.dart'; @@ -46,10 +47,17 @@ class PnpNotifier extends Notifier { ); await _checkInternet(); + } on ServiceError catch (e) { + // Reading device info failed (e.g. USP GET returned empty). This is a + // read failure, not "no internet" — surface it as such. + logger.e('[PnP] startPostLoginFlow read failure: $e (code=${e.code})'); + state = state.copyWith( + phase: AdminReadFailure(code: e.code, detail: '$e'), + ); } catch (e) { logger.e('[PnP] startPostLoginFlow error: $e'); state = state.copyWith( - phase: AdminError(message: '$e'), + phase: AdminReadFailure(detail: '$e'), ); } } @@ -71,9 +79,18 @@ class PnpNotifier extends Notifier { phase: NoInternet(ssid: ssid, currentWanSettings: wanSettings), ); } + } on ServiceError catch (e) { + // The WAN read threw — we could NOT determine the WAN state (router + // unreachable / USP GET returned empty). This is distinct from the router + // confirming "no internet" (which returns false above, no throw), so we + // must not collapse it into NoInternet. + logger.e('[PnP] Internet check read failure: $e (code=${e.code})'); + state = state.copyWith( + phase: AdminReadFailure(code: e.code, detail: '$e'), + ); } catch (e) { - logger.e('[PnP] Internet check failed: $e'); - state = state.copyWith(phase: const NoInternet()); + logger.e('[PnP] Internet check unexpected error: $e'); + state = state.copyWith(phase: AdminReadFailure(detail: '$e')); } } diff --git a/lib/page/instant_setup/views/pnp_entry_view.dart b/lib/page/instant_setup/views/pnp_entry_view.dart index 14988ad2c..12d093d79 100644 --- a/lib/page/instant_setup/views/pnp_entry_view.dart +++ b/lib/page/instant_setup/views/pnp_entry_view.dart @@ -60,7 +60,7 @@ class _PnpEntryViewState extends ConsumerState { child: switch (pnpState.phase) { AdminCheckingInternet() => _buildCheckingInternet(context), AdminInternetConnected() => _buildLoading(context), - AdminError(message: final msg) => _buildErrorCard(context, msg), + AdminReadFailure() => _buildErrorCard(context), WizardInitializing() => _buildLoading(context), _ => _buildLoading(context), }, @@ -93,7 +93,7 @@ class _PnpEntryViewState extends ConsumerState { ); } - Widget _buildErrorCard(BuildContext context, String message) { + Widget _buildErrorCard(BuildContext context) { return AppCard( child: Padding( padding: const EdgeInsets.all(AppSpacing.lg), @@ -102,7 +102,10 @@ class _PnpEntryViewState extends ConsumerState { children: [ AppIcon.font(Icons.error_outline, size: 48, color: Colors.red), AppGap.lg(), - AppText.bodyMedium(message), + AppText.bodyMedium( + loc(context).unableToGatherDeviceInfo, + textAlign: TextAlign.center, + ), AppGap.xl(), AppButton.text( label: loc(context).tryAgain, diff --git a/test/page/instant_setup/providers/pnp_notifier_test.dart b/test/page/instant_setup/providers/pnp_notifier_test.dart index e435b69b0..b3f67d1a0 100644 --- a/test/page/instant_setup/providers/pnp_notifier_test.dart +++ b/test/page/instant_setup/providers/pnp_notifier_test.dart @@ -1,6 +1,7 @@ import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:mocktail/mocktail.dart'; +import 'package:privacy_gui/core/errors/service_error.dart'; import 'package:privacy_gui/core/usp/providers/usp_mutation_lock.dart'; import 'package:privacy_gui/core/usp/providers/usp_client_provider.dart'; import 'package:privacy_gui/core/usp/services/usp_client.dart'; @@ -127,9 +128,32 @@ void main() { container.dispose(); }); - test('error transitions to AdminError phase', () async { + test('device info read failure transitions to AdminReadFailure phase', + () async { when(() => mockPnpService.checkFactoryDefault()) - .thenThrow(Exception('Network error')); + .thenThrow(const NetworkError(detail: 'Network error')); + + final container = createContainer(); + final notifier = container.read(pnpProvider.notifier); + + await notifier.startPostLoginFlow(); + + final state = container.read(pnpProvider); + expect(state.phase, isA()); + expect( + (state.phase as AdminReadFailure).detail, contains('Network error')); + container.dispose(); + }); + + // Regression for #1098: a WAN read FAILURE (USP GET returned empty → + // WanStatus.fetch throws) must NOT collapse into NoInternet. That state is + // reserved for the router *confirming* no internet (returns false, no throw). + test('WAN read failure transitions to AdminReadFailure, not NoInternet', + () async { + when(() => mockPnpService.checkFactoryDefault()) + .thenAnswer((_) async => testFactoryResult); + when(() => mockPnpService.checkInternetConnected()) + .thenThrow(const InvalidInputError(code: 9998, detail: 'missing')); final container = createContainer(); final notifier = container.read(pnpProvider.notifier); @@ -137,8 +161,8 @@ void main() { await notifier.startPostLoginFlow(); final state = container.read(pnpProvider); - expect(state.phase, isA()); - expect((state.phase as AdminError).message, contains('Network error')); + expect(state.phase, isA()); + expect((state.phase as AdminReadFailure).code, 9998); container.dispose(); }); }); From 7695ff7f0d22a7ff9e8ff3925682eec89588ddde Mon Sep 17 00:00:00 2001 From: Hank Yu Date: Wed, 15 Jul 2026 10:03:39 +0800 Subject: [PATCH 2/3] fix(pnp): route read failures back to entry from all check callers (#1098) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The classification fix landed AdminReadFailure at the notifier, but _checkInternet() has multiple callers and only the entry view rendered the new phase. The no-internet retry and the ISP-save (PPPoE / Static IP / DHCP) flows had no branch for it, so a read failure there would stall silently. Route every read-failure path back to the entry view (RoutePath.pnp). Because /pnp and the no-internet subtree are independent top-level route trees, go() mounts a fresh PnpEntryView whose initState re-runs startPostLoginFlow — an implicit retry that settles on the read-failure card only if it still fails. The ISP-save WRITE failure path is deliberately left on NoInternet + snackbar: _checkInternet does not rethrow, so a trailing check read-failure is handled inside it (AdminReadFailure), and only a genuine save-write failure reaches the saveIspWithProgress catch — the user should stay on the form to fix it. Also delete saveIspSettingsAndCheck (dead code, zero callers; superseded by saveIspWithProgress) and add a regression test for the save-succeeds-but-check- read-fails path. --- .../instant_setup/providers/pnp_notifier.dart | 21 ------------ .../views/pnp_isp_settings_view.dart | 5 +++ .../views/pnp_no_internet_view.dart | 5 +++ .../instant_setup/views/pnp_pppoe_view.dart | 5 +++ .../views/pnp_static_ip_view.dart | 5 +++ .../providers/pnp_notifier_test.dart | 32 +++++++++++++++++++ 6 files changed, 52 insertions(+), 21 deletions(-) diff --git a/lib/page/instant_setup/providers/pnp_notifier.dart b/lib/page/instant_setup/providers/pnp_notifier.dart index ef6317453..67b424021 100644 --- a/lib/page/instant_setup/providers/pnp_notifier.dart +++ b/lib/page/instant_setup/providers/pnp_notifier.dart @@ -396,27 +396,6 @@ class PnpNotifier extends Notifier { // ─── No Internet Flow ─────────────────────────────────── - /// Save ISP settings and re-check internet. - Future saveIspSettingsAndCheck(PnpIspConfig config) async { - try { - await ref.read(uspMutationLockProvider).withLock(() async { - await _svc.saveIspSettings(config); - }); - - // Wait for WAN interface to come up - await Future.delayed(const Duration(seconds: 5)); - - state = state.copyWith(phase: const AdminCheckingInternet()); - await _checkInternet(); - } catch (e) { - logger.e('[PnP] ISP save failed: $e'); - state = state.copyWith( - phase: const NoInternet(), - errorMessage: '$e', - ); - } - } - /// Retry internet check after modem restart flow. Future retryInternetCheck() async { state = state.copyWith(phase: const AdminCheckingInternet()); diff --git a/lib/page/instant_setup/views/pnp_isp_settings_view.dart b/lib/page/instant_setup/views/pnp_isp_settings_view.dart index 9c7e11097..b4a58df17 100644 --- a/lib/page/instant_setup/views/pnp_isp_settings_view.dart +++ b/lib/page/instant_setup/views/pnp_isp_settings_view.dart @@ -39,6 +39,11 @@ class _PnpIspSettingsViewState extends ConsumerState { final phase = state.phase; if (phase is WizardConfiguring || phase is WizardInitializing) { context.go(RoutePath.pnp); + } else if (phase is AdminReadFailure) { + // Save succeeded but the trailing internet check could not read router + // state. Route back to the entry view, which re-runs the flow (implicit + // retry) and renders the read-failure card if it still fails. + context.go(RoutePath.pnp); } else if (state.errorMessage != null) { showFailedSnackBar(context, state.errorMessage!); } diff --git a/lib/page/instant_setup/views/pnp_no_internet_view.dart b/lib/page/instant_setup/views/pnp_no_internet_view.dart index 9de02b9aa..e2873ed64 100644 --- a/lib/page/instant_setup/views/pnp_no_internet_view.dart +++ b/lib/page/instant_setup/views/pnp_no_internet_view.dart @@ -34,6 +34,11 @@ class _PnpNoInternetViewState extends ConsumerState { ref.listen(pnpProvider, (prev, next) { if (next.phase is WizardConfiguring || next.phase is WizardInitializing) { context.go(RoutePath.pnp); + } else if (next.phase is AdminReadFailure) { + // "Try again" hit a read failure (router state unreadable) rather than a + // confirmed no-internet. Route back to the entry view, which re-runs the + // flow (implicit retry) and renders the read-failure card if it persists. + context.go(RoutePath.pnp); } }); diff --git a/lib/page/instant_setup/views/pnp_pppoe_view.dart b/lib/page/instant_setup/views/pnp_pppoe_view.dart index 1d9623d13..6782330e0 100644 --- a/lib/page/instant_setup/views/pnp_pppoe_view.dart +++ b/lib/page/instant_setup/views/pnp_pppoe_view.dart @@ -58,6 +58,11 @@ class _PnpPppoeViewState extends ConsumerState { ref.listen(pnpProvider, (prev, next) { if (next.phase is WizardConfiguring || next.phase is WizardInitializing) { context.go(RoutePath.pnp); + } else if (next.phase is AdminReadFailure) { + // Save succeeded but the trailing internet check could not read router + // state. Route back to the entry view, which re-runs the flow (implicit + // retry) and renders the read-failure card if it still fails. + context.go(RoutePath.pnp); } else if (prev?.phase is IspSaving && next.phase is NoInternet && next.errorMessage != null) { diff --git a/lib/page/instant_setup/views/pnp_static_ip_view.dart b/lib/page/instant_setup/views/pnp_static_ip_view.dart index d092fd978..cc9804a55 100644 --- a/lib/page/instant_setup/views/pnp_static_ip_view.dart +++ b/lib/page/instant_setup/views/pnp_static_ip_view.dart @@ -64,6 +64,11 @@ class _PnpStaticIpViewState extends ConsumerState { ref.listen(pnpProvider, (prev, next) { if (next.phase is WizardConfiguring || next.phase is WizardInitializing) { context.go(RoutePath.pnp); + } else if (next.phase is AdminReadFailure) { + // Save succeeded but the trailing internet check could not read router + // state. Route back to the entry view, which re-runs the flow (implicit + // retry) and renders the read-failure card if it still fails. + context.go(RoutePath.pnp); } else if (prev?.phase is IspSaving && next.phase is NoInternet && next.errorMessage != null) { diff --git a/test/page/instant_setup/providers/pnp_notifier_test.dart b/test/page/instant_setup/providers/pnp_notifier_test.dart index b3f67d1a0..1b0fb6941 100644 --- a/test/page/instant_setup/providers/pnp_notifier_test.dart +++ b/test/page/instant_setup/providers/pnp_notifier_test.dart @@ -358,5 +358,37 @@ void main() { expect(state.phase, isA()); container.dispose(); }); + + // Regression for #1098: the ISP save WRITE succeeds, but the trailing + // internet check READ fails (USP GET returned empty). This must land in + // AdminReadFailure (read failure), NOT NoInternet — distinct from a genuine + // no-internet (checkInternetConnected returns false) and from a save write + // failure (which stays on NoInternet + errorMessage, tested above). + test('ISP save success but check read failure → AdminReadFailure', () async { + when(() => mockPnpService.saveIspSettings(any())) + .thenAnswer((_) async {}); + when(() => mockPnpService.checkInternetConnected()) + .thenThrow(const InvalidInputError(code: 9998, detail: 'missing')); + + final container = createContainer(); + final notifier = container.read(pnpProvider.notifier); + + notifier.setDemoPhase(const NoInternet(ssid: 'Test')); + + const config = PnpIspConfig( + type: IspConnectionType.staticIp, + staticIpAddress: '10.0.0.5', + subnetMask: '255.255.255.0', + defaultGateway: '10.0.0.1', + ); + + await notifier.saveIspWithProgress(config); + + verify(() => mockPnpService.saveIspSettings(any())).called(1); + final state = container.read(pnpProvider); + expect(state.phase, isA()); + expect((state.phase as AdminReadFailure).code, 9998); + container.dispose(); + }); }); } From 78b5ecee301c698203041bd6fb73c288c27cb822 Mon Sep 17 00:00:00 2001 From: Hank Yu Date: Wed, 15 Jul 2026 10:44:28 +0800 Subject: [PATCH 3/3] style(pnp): apply dart format to pnp_notifier_test --- test/page/instant_setup/providers/pnp_notifier_test.dart | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/page/instant_setup/providers/pnp_notifier_test.dart b/test/page/instant_setup/providers/pnp_notifier_test.dart index 1b0fb6941..bdfb1fd94 100644 --- a/test/page/instant_setup/providers/pnp_notifier_test.dart +++ b/test/page/instant_setup/providers/pnp_notifier_test.dart @@ -364,7 +364,8 @@ void main() { // AdminReadFailure (read failure), NOT NoInternet — distinct from a genuine // no-internet (checkInternetConnected returns false) and from a save write // failure (which stays on NoInternet + errorMessage, tested above). - test('ISP save success but check read failure → AdminReadFailure', () async { + test('ISP save success but check read failure → AdminReadFailure', + () async { when(() => mockPnpService.saveIspSettings(any())) .thenAnswer((_) async {}); when(() => mockPnpService.checkInternetConnected())