fix(pnp): distinguish router read failure from no-internet (#1098)

lib/page/instant_setup/models/pnp_state.dart

@@ -71,12 +71,22 @@ class AdminInternetConnected extends PnpPhase {
   List<Object?> get props => [];
 }
 
-/// Critical error in admin phase.
-class AdminError extends PnpPhase {
-  final String message;
-  const AdminError({required this.message});
-  @override
-  List<Object?> 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<Object?> get props => [code, detail];
 }
 
 /// No internet detected — route to troubleshooter.

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<PnpState> {
       );
 
       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<PnpState> {
           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'));
     }
   }
 
@@ -379,27 +396,6 @@ class PnpNotifier extends Notifier<PnpState> {
 
   // ─── No Internet Flow ───────────────────────────────────
 
-  /// Save ISP settings and re-check internet.
-  Future<void> 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<void> retryInternetCheck() async {
     state = state.copyWith(phase: const AdminCheckingInternet());

lib/page/instant_setup/views/pnp_entry_view.dart

@@ -60,7 +60,7 @@ class _PnpEntryViewState extends ConsumerState<PnpEntryView> {
             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<PnpEntryView> {
     );
   }
 
-  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<PnpEntryView> {
           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,

lib/page/instant_setup/views/pnp_isp_settings_view.dart

@@ -39,6 +39,11 @@ class _PnpIspSettingsViewState extends ConsumerState<PnpIspSettingsView> {
     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!);
     }

lib/page/instant_setup/views/pnp_no_internet_view.dart

@@ -34,6 +34,11 @@ class _PnpNoInternetViewState extends ConsumerState<PnpNoInternetView> {
     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);
       }
     });
 

lib/page/instant_setup/views/pnp_pppoe_view.dart

@@ -58,6 +58,11 @@ class _PnpPppoeViewState extends ConsumerState<PnpPppoeView> {
     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) {

lib/page/instant_setup/views/pnp_static_ip_view.dart

@@ -64,6 +64,11 @@ class _PnpStaticIpViewState extends ConsumerState<PnpStaticIpView> {
     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) {

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,18 +128,41 @@ 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(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<AdminReadFailure>());
+      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())
-          .thenThrow(Exception('Network error'));
+          .thenAnswer((_) async => testFactoryResult);
+      when(() => mockPnpService.checkInternetConnected())
+          .thenThrow(const InvalidInputError(code: 9998, detail: 'missing'));
 
       final container = createContainer();
       final notifier = container.read(pnpProvider.notifier);
 
       await notifier.startPostLoginFlow();
 
       final state = container.read(pnpProvider);
-      expect(state.phase, isA<AdminError>());
-      expect((state.phase as AdminError).message, contains('Network error'));
+      expect(state.phase, isA<AdminReadFailure>());
+      expect((state.phase as AdminReadFailure).code, 9998);
       container.dispose();
     });
   });
@@ -334,5 +358,38 @@ void main() {
       expect(state.phase, isA<NoInternet>());
       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<AdminReadFailure>());
+      expect((state.phase as AdminReadFailure).code, 9998);
+      container.dispose();
+    });
   });
 }