[pve-devel] [PATCH proxmox_dart_api_client/pve_flutter_frontend_null 0/2] fix #4976

[pve-devel] [PATCH proxmox_dart_api_client/pve_flutter_frontend_null 0/2] fix #4976

[Alexander Abraham]

This patches fixes bug 4976 in which the UI for nodes that are
offline was reporting a "null" error. In the API client, error-handling
for POST requests was changed to be more explicit and in the app's UI
the case of a node suddenly being offline was handled. Viewing the
information for a node that is offline was made impossible.


proxmox_dart_api_client:

Alexander Abraham (1):
  fix #4976: dart_api_client: Improved error-handling for POST requests.

 lib/src/client.dart | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)


pve_flutter_frontend:

Alexander Abraham (1):
  fix #4976: ui: Offline-status handled adequately in the UI.

 lib/bloc/pve_node_overview_bloc.dart | 79 ++++++++++++++++++----------
 lib/main.dart                        |  3 ++
 lib/pages/main_layout_slim.dart      | 58 ++++++++++++++++----
 3 files changed, 101 insertions(+), 39 deletions(-)


Summary over all repositories:
  4 files changed, 110 insertions(+), 44 deletions(-)

-- 
Generated by git-murpp 0.8.1


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH dart_api_client 1/1] fix #4976: Request errors are

[Alexander Abraham]

This commit adds more explicit error-handling for when a host
is possibly offline.

Signed-off-by: Alexander Abraham <a.abraham@proxmox.com>
---
 lib/src/client.dart | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/lib/src/client.dart b/lib/src/client.dart
index f597c28..7c118d8 100644
--- a/lib/src/client.dart
+++ b/lib/src/client.dart
@@ -1,6 +1,5 @@
 import 'dart:convert';
 import 'dart:io';
-
 import 'package:http/http.dart' as http;
 //import 'package:proxmox_dart_api_client/src/models/pve_access_user_model.dart';
 import 'package:proxmox_dart_api_client/src/models/pve_models.dart';
@@ -127,7 +126,7 @@ class ProxmoxApiClient extends http.BaseClient {
       () => get(url),
       retryIf: (e) => e is http.ClientException || e is SocketException,
     ))
-        .validate(extensiveResponseValidation);
+         .validate(extensiveResponseValidation);
   }
 
   Future<http.Response> _postWithValidation(
@@ -398,9 +397,14 @@ class ProxmoxApiClient extends http.BaseClient {
       'typefilter': typefilter,
       'userfilter': userfilter
     };
-    final response = await _getWithValidation(path, queryParameters);
+    var response;
+    try {
+     response = await _getWithValidation(path, queryParameters);
+    }
+    catch(e) {
+      throw "Host unreachable, possibly offline.";
+    }
     final decoded = json.decode(response.body);
-
     var data = (decoded['data'] as List).map((f) {
       return serializers.deserializeWith(PveClusterTasksModel.serializer, f);
     });
@@ -563,7 +567,7 @@ class ProxmoxApiClient extends http.BaseClient {
     var data = (json.decode(response.body)['data'] as List).map((f) {
       return serializers.deserializeWith(PveNodeRRDDataModel.serializer, f);
     });
-    return data.whereType<PveNodeRRDDataModel>().toList();
+   return data.whereType<PveNodeRRDDataModel>().toList();
   }
 
   Future<List<PveNodeServicesModel>> getNodeServices(
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH flutter_frontend 1/1] fix #4976: ui: Nodes offline

[Alexander Abraham]

This commit adds functionality for not letting users click on nodes
that are offline in the general overview screen. Furthermore,
functionality was added to send the user back to the general overview
screen if a node is suddenly offline while the user is viewing
information on that node.

Signed-off-by: Alexander Abraham <a.abraham@proxmox.com>
---
 lib/bloc/pve_node_overview_bloc.dart | 79 ++++++++++++++++++----------
 lib/main.dart                        |  3 ++
 lib/pages/main_layout_slim.dart      | 58 ++++++++++++++++----
 3 files changed, 101 insertions(+), 39 deletions(-)

diff --git a/lib/bloc/pve_node_overview_bloc.dart b/lib/bloc/pve_node_overview_bloc.dart
index 19d6563..98005fd 100644
--- a/lib/bloc/pve_node_overview_bloc.dart
+++ b/lib/bloc/pve_node_overview_bloc.dart
@@ -1,5 +1,5 @@
 import 'dart:async';
-
+import 'package:flutter/material.dart';
 import 'package:proxmox_dart_api_client/proxmox_dart_api_client.dart';
 import 'package:pve_flutter_frontend/bloc/proxmox_base_bloc.dart';
 import 'package:pve_flutter_frontend/states/pve_node_overview_state.dart';
@@ -8,12 +8,13 @@ class PveNodeOverviewBloc
     extends ProxmoxBaseBloc<PveNodeOverviewEvent, PveNodeOverviewState> {
   final ProxmoxApiClient apiClient;
   final String nodeID;
+  final BuildContext context;
   final PveNodeOverviewState init;
   @override
   PveNodeOverviewState get initialState => init;
 
   PveNodeOverviewBloc(
-      {required this.apiClient, required this.nodeID, required this.init});
+      {required this.apiClient, required this.nodeID, required this.init, required this.context});
 
   Timer? sTimer;
   @override
@@ -32,36 +33,56 @@ class PveNodeOverviewBloc
   @override
   Stream<PveNodeOverviewState> processEvents(
       PveNodeOverviewEvent event) async* {
-    if (event is UpdateNodeStatus) {
-      final status = await apiClient.getNodeStatus(nodeID);
-      yield latestState.rebuild((b) => b..status.replace(status));
-      final rrdData =
-          await apiClient.getNodeRRDdata(nodeID, PveRRDTimeframeType.hour);
-      yield latestState.rebuild((b) => b..rrdData.replace(rrdData));
-      final services = await apiClient.getNodeServices(nodeID);
-      yield latestState.rebuild((b) => b..services.replace(services));
-      try {
-        final updates = await apiClient.getNodeAptUpdate(nodeID);
-        yield latestState.rebuild((b) => b..updates.replace(updates));
-        yield latestState
-            .rebuild((b) => b..updatesQueryPermissionFailure = false);
-      } on ProxmoxApiException catch (e) {
-        // only throw on non permission related errors
-        if (e.statusCode != 403) {
-          rethrow;
-        } else {
+    PveNodeStatusModel? status;
+    try {
+      status = await apiClient.getNodeStatus(nodeID);
+      if (event is UpdateNodeStatus) {
+        yield latestState.rebuild((b) => b..status.replace(status!));
+        final rrdData =
+        await apiClient.getNodeRRDdata(nodeID, PveRRDTimeframeType.hour);
+        yield latestState.rebuild((b) => b..rrdData.replace(rrdData));
+        final services = await apiClient.getNodeServices(nodeID);
+        yield latestState.rebuild((b) => b..services.replace(services));
+        try {
+          final updates = await apiClient.getNodeAptUpdate(nodeID);
+          yield latestState.rebuild((b) => b..updates.replace(updates));
           yield latestState
-              .rebuild((b) => b..updatesQueryPermissionFailure = true);
+              .rebuild((b) => b..updatesQueryPermissionFailure = false);
+        } 
+        on ProxmoxApiException catch (e) {
+            // only throw on non permission related errors
+            if (e.statusCode != 403) {
+              rethrow;
+            } 
+            else {
+              yield latestState
+                .rebuild((b) => b..updatesQueryPermissionFailure = true);
+            }
+          }
+          final disks = await apiClient.getNodeDisksList(nodeID);
+          yield latestState.rebuild((b) => b..disks.replace(disks));
+        }
+
+        if (event is PerformNodeAction) {
+          await apiClient.doResourceAction(
+            nodeID,
+            '',
+            'node',
+            event.action,
+            parameters: <String, String>{});
+          yield latestState;
         }
       }
-      final disks = await apiClient.getNodeDisksList(nodeID);
-      yield latestState.rebuild((b) => b..disks.replace(disks));
-    }
-    if (event is PerformNodeAction) {
-      await apiClient.doResourceAction(nodeID, '', 'node', event.action,
-          parameters: <String, String>{});
-      yield latestState;
-    }
+      catch(e){
+        yield latestState.rebuild(
+          (b) => b
+            ..updatesQueryPermissionFailure = true
+        );
+        if (context.mounted){
+          Navigator.of(context).pop();
+        }
+        else {}
+      }
   }
 }
 
diff --git a/lib/main.dart b/lib/main.dart
index a985e3a..bbd3c97 100644
--- a/lib/main.dart
+++ b/lib/main.dart
@@ -330,15 +330,18 @@ class MyApp extends StatelessWidget {
                   final rbloc = Provider.of<PveResourceBloc>(context);
                   return MultiProvider(
                     providers: [
+
                       Provider<PveNodeOverviewBloc>(
                         create: (context) => PveNodeOverviewBloc(
                           apiClient: state.apiClient,
                           nodeID: nodeID,
+                          context: context,
                           init: PveNodeOverviewState.init(
                               rbloc.latestState.isStandalone),
                         )..events.add(UpdateNodeStatus()),
                         dispose: (context, bloc) => bloc.dispose(),
                       ),
+
                       Provider<PveTaskLogBloc>(
                         create: (context) => PveTaskLogBloc(
                             apiClient: state.apiClient,
diff --git a/lib/pages/main_layout_slim.dart b/lib/pages/main_layout_slim.dart
index 322bcac..5fa6199 100644
--- a/lib/pages/main_layout_slim.dart
+++ b/lib/pages/main_layout_slim.dart
@@ -254,7 +254,7 @@ class MobileDashboard extends StatelessWidget {
                                 fontWeight: FontWeight.bold,
                                 color: Colors.white),
                           ),
-                          onPressed: () => showDialog(
+                          onPressed: () => (
                             context: context,
                             builder: (c) => const PveSubscriptionAlertDialog(),
                           ),
@@ -556,7 +556,7 @@ class MobileDashboard extends StatelessWidget {
   }
 }
 
-class PveNodeListTile extends StatelessWidget {
+class PveNodeListTile extends StatelessWidget{
   final String name;
   final bool online;
   final String type;
@@ -564,21 +564,58 @@ class PveNodeListTile extends StatelessWidget {
   final String? ip;
   const PveNodeListTile(
       {super.key,
-      required this.name,
-      required this.online,
-      required this.type,
-      this.level,
-      this.ip = ''});
+        required this.name,
+        required this.online,
+        required this.type,
+        this.level,
+        this.ip = ''});
+
   @override
   Widget build(BuildContext context) {
+    late Color splashColor;
+    if (online){
+      splashColor = ProxmoxColors.greyShade40;
+    }
+    else {
+      splashColor = Colors.transparent;
+    }
     return ListTile(
       leading: Icon(
         Renderers.getDefaultResourceIcon(type),
       ),
       title: Text(name),
+      splashColor: splashColor,
       subtitle: Text(getNodeTileSubtitle(online, level, ip)),
       trailing: Icon(Icons.power, color: online ? Colors.green : Colors.grey),
-      onTap: () => Navigator.pushNamed(context, '/nodes/$name'),
+      onTap: () => {
+        if (online){
+          Navigator.pushNamed(context, '/nodes/$name')
+        }
+        else{
+          showDialog(
+            context: context,
+            builder: (context) {
+              return AlertDialog(
+                contentPadding: const EdgeInsets.fromLTRB(24.0, 12.0, 24.0, 16.0),
+                title: Row(
+                  mainAxisAlignment: MainAxisAlignment.spaceBetween,
+                  children: <Widget>[
+                    Text('Error'),
+                    Icon(Icons.warning),
+                  ],
+                ),
+                content: Text("Node offline."),
+                actions: [
+                  TextButton(
+                    onPressed: () => Navigator.of(context).pop(false),
+                    child: const Text("Ok")
+                  ),
+                ],
+              );
+            }
+          )
+        }  
+      }
     );
   }
 
@@ -588,6 +625,7 @@ class PveNodeListTile extends StatelessWidget {
     }
     return 'offline';
   }
+
 }
 
 class MobileResourceOverview extends StatelessWidget {
@@ -621,7 +659,7 @@ class MobileResourceOverview extends StatelessWidget {
               separatorBuilder: (context, index) => const Divider(),
               itemBuilder: (context, index) {
                 final resource = fResources[index];
-                StatelessWidget listWidget = const ListTile(
+                Widget listWidget = const ListTile(
                   title: Text('Unkown resource type'),
                 );
                 if (const ['lxc', 'qemu'].contains(resource.type)) {
@@ -1191,7 +1229,7 @@ class MobileAccessManagement extends StatelessWidget {
                               borderRadius: BorderRadius.vertical(
                                   top: Radius.circular(10))),
                           context: context,
-                          builder: (context) {
+                          builder: (context) { 
                             return SizedBox(
                               height: MediaQuery.of(context).size.height * 0.5,
                               child: Column(
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH dart_api_client 1/1] fix #4976: Request errors are

[Michael Köppl]

Hi, thanks for tackling this. Noticed a few small things. Find the 
comments inline. Generally, please run dart format on these changes. 
Every changed block contains wrong indentation. Also, the commit message 
needs to be fixed.

On 4/29/25 15:16, Alexander Abraham wrote:
> This commit adds more explicit error-handling for when a host
> is possibly offline.
> 
> Signed-off-by: Alexander Abraham <a.abraham@proxmox.com>
> ---
>   lib/src/client.dart | 14 +++++++++-----
>   1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/src/client.dart b/lib/src/client.dart
> index f597c28..7c118d8 100644
> --- a/lib/src/client.dart
> +++ b/lib/src/client.dart
> @@ -1,6 +1,5 @@
>   import 'dart:convert';
>   import 'dart:io';
> -
>   import 'package:http/http.dart' as http;
>   //import 'package:proxmox_dart_api_client/src/models/pve_access_user_model.dart';
>   import 'package:proxmox_dart_api_client/src/models/pve_models.dart';
> @@ -127,7 +126,7 @@ class ProxmoxApiClient extends http.BaseClient {
>         () => get(url),
>         retryIf: (e) => e is http.ClientException || e is SocketException,
>       ))
> -        .validate(extensiveResponseValidation);
> +         .validate(extensiveResponseValidation);
>     }
>   
>     Future<http.Response> _postWithValidation(
> @@ -398,9 +397,14 @@ class ProxmoxApiClient extends http.BaseClient {
>         'typefilter': typefilter,
>         'userfilter': userfilter
>       };
> -    final response = await _getWithValidation(path, queryParameters);
> +    var response;
> +    try {
> +     response = await _getWithValidation(path, queryParameters);
> +    }
> +    catch(e) {
> +      throw "Host unreachable, possibly offline.";

Is there a reason this is not wrapped in a ProxmoxApiException? If the 
reason is that it then displays "Host unreachable, possibly offline. -> 
null", I think it would generally be a good idea to adapt 
ProxmoxApiException's toString method to account for the details 
possibly being null.

nit: Since this is not a sentence, I think it should either be "Host is 
unreachable, possibly offline" or not end with a dot.

> +    }
>       final decoded = json.decode(response.body);
> -
>       var data = (decoded['data'] as List).map((f) {
>         return serializers.deserializeWith(PveClusterTasksModel.serializer, f);
>       });
> @@ -563,7 +567,7 @@ class ProxmoxApiClient extends http.BaseClient {
>       var data = (json.decode(response.body)['data'] as List).map((f) {
>         return serializers.deserializeWith(PveNodeRRDDataModel.serializer, f);
>       });
> -    return data.whereType<PveNodeRRDDataModel>().toList();
> +   return data.whereType<PveNodeRRDDataModel>().toList();
>     }
>   
>     Future<List<PveNodeServicesModel>> getNodeServices(



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH flutter_frontend 1/1] fix #4976: ui: Nodes offline

[Michael Köppl]

The commit message is not very descriptive. Also, please run dart format 
on these changes. There are some wrong indentations, missing spaces, 
trailing spaces, etc. I also added 3 comments inline.

Apart from that, I tested this by manually stopping a node I was 
currently looking at in the node overview. The app navigates me back to 
the previous screen once it detects that the node is offline and 
afterwards stops me from opening that node's overview again while it is 
offline. Opening the overview once the node is online again works as 
well. Everything seems to work as expected. It's a good change, well done!

On 4/29/25 15:16, Alexander Abraham wrote:
> diff --git a/lib/bloc/pve_node_overview_bloc.dart b/lib/bloc/pve_node_overview_bloc.dart
> index 19d6563..98005fd 100644
> --- a/lib/bloc/pve_node_overview_bloc.dart
> +++ b/lib/bloc/pve_node_overview_bloc.dart
> @@ -1,5 +1,5 @@
>   import 'dart:async';
> -
> +import 'package:flutter/material.dart';
>   import 'package:proxmox_dart_api_client/proxmox_dart_api_client.dart';
>   import 'package:pve_flutter_frontend/bloc/proxmox_base_bloc.dart';
>   import 'package:pve_flutter_frontend/states/pve_node_overview_state.dart';
> @@ -8,12 +8,13 @@ class PveNodeOverviewBloc
>       extends ProxmoxBaseBloc<PveNodeOverviewEvent, PveNodeOverviewState> {
>     final ProxmoxApiClient apiClient;
>     final String nodeID;
> +  final BuildContext context;
>     final PveNodeOverviewState init;
>     @override
>     PveNodeOverviewState get initialState => init;
>   
>     PveNodeOverviewBloc(
> -      {required this.apiClient, required this.nodeID, required this.init});
> +      {required this.apiClient, required this.nodeID, required this.init, required this.context});
>   
>     Timer? sTimer;
>     @override
> @@ -32,36 +33,56 @@ class PveNodeOverviewBloc
>     @override
>     Stream<PveNodeOverviewState> processEvents(
>         PveNodeOverviewEvent event) async* {
> -    if (event is UpdateNodeStatus) {
> -      final status = await apiClient.getNodeStatus(nodeID);
> -      yield latestState.rebuild((b) => b..status.replace(status));
> -      final rrdData =
> -          await apiClient.getNodeRRDdata(nodeID, PveRRDTimeframeType.hour);
> -      yield latestState.rebuild((b) => b..rrdData.replace(rrdData));
> -      final services = await apiClient.getNodeServices(nodeID);
> -      yield latestState.rebuild((b) => b..services.replace(services));
> -      try {
> -        final updates = await apiClient.getNodeAptUpdate(nodeID);
> -        yield latestState.rebuild((b) => b..updates.replace(updates));
> -        yield latestState
> -            .rebuild((b) => b..updatesQueryPermissionFailure = false);
> -      } on ProxmoxApiException catch (e) {
> -        // only throw on non permission related errors
> -        if (e.statusCode != 403) {
> -          rethrow;
> -        } else {
> +    PveNodeStatusModel? status;
> +    try {
> +      status = await apiClient.getNodeStatus(nodeID);
> +      if (event is UpdateNodeStatus) {
> +        yield latestState.rebuild((b) => b..status.replace(status!));
> +        final rrdData =
> +        await apiClient.getNodeRRDdata(nodeID, PveRRDTimeframeType.hour);
> +        yield latestState.rebuild((b) => b..rrdData.replace(rrdData));
> +        final services = await apiClient.getNodeServices(nodeID);
> +        yield latestState.rebuild((b) => b..services.replace(services));
> +        try {
> +          final updates = await apiClient.getNodeAptUpdate(nodeID);
> +          yield latestState.rebuild((b) => b..updates.replace(updates));
>             yield latestState
> -              .rebuild((b) => b..updatesQueryPermissionFailure = true);
> +              .rebuild((b) => b..updatesQueryPermissionFailure = false);
> +        }
> +        on ProxmoxApiException catch (e) {
> +            // only throw on non permission related errors
> +            if (e.statusCode != 403) {
> +              rethrow;
> +            }
> +            else {
> +              yield latestState
> +                .rebuild((b) => b..updatesQueryPermissionFailure = true);
> +            }
> +          }
> +          final disks = await apiClient.getNodeDisksList(nodeID);
> +          yield latestState.rebuild((b) => b..disks.replace(disks));
> +        }
> +
> +        if (event is PerformNodeAction) {
> +          await apiClient.doResourceAction(
> +            nodeID,
> +            '',
> +            'node',
> +            event.action,
> +            parameters: <String, String>{});
> +          yield latestState;
>           }
>         }
> -      final disks = await apiClient.getNodeDisksList(nodeID);
> -      yield latestState.rebuild((b) => b..disks.replace(disks));
> -    }
> -    if (event is PerformNodeAction) {
> -      await apiClient.doResourceAction(nodeID, '', 'node', event.action,
> -          parameters: <String, String>{});
> -      yield latestState;
> -    }
> +      catch(e){
> +        yield latestState.rebuild(
> +          (b) => b
> +            ..updatesQueryPermissionFailure = true
> +        );
> +        if (context.mounted){
> +          Navigator.of(context).pop();

Instead of navigating back immediately, I think it would make sense to 
instead display an AlertDialog, showing the error message to the user 
and letting them leave the screen through a "Close" button in the 
dialog. Otherwise, it feels a bit like the app is taking control away 
from the user. We use this approach in other places in the app (e.g. . 
What do you think?

> +        }
> +        else {}
> +      }
>     }
>   }

Wrapping the whole block in a try-catch without a rethrow also means 
that users will be navigated back to the home screen without an error 
message if e.g. the getNodeDisksList call returns an error. I think it'd 
make sense to rethrow here, similar to above.

>   
> diff --git a/lib/main.dart b/lib/main.dart
> index a985e3a..bbd3c97 100644
> --- a/lib/main.dart
> +++ b/lib/main.dart
> @@ -330,15 +330,18 @@ class MyApp extends StatelessWidget {
>                     final rbloc = Provider.of<PveResourceBloc>(context);
>                     return MultiProvider(
>                       providers: [
> +
>                         Provider<PveNodeOverviewBloc>(
>                           create: (context) => PveNodeOverviewBloc(
>                             apiClient: state.apiClient,
>                             nodeID: nodeID,
> +                          context: context,
>                             init: PveNodeOverviewState.init(
>                                 rbloc.latestState.isStandalone),
>                           )..events.add(UpdateNodeStatus()),
>                           dispose: (context, bloc) => bloc.dispose(),
>                         ),
> +
>                         Provider<PveTaskLogBloc>(
>                           create: (context) => PveTaskLogBloc(
>                               apiClient: state.apiClient,
> diff --git a/lib/pages/main_layout_slim.dart b/lib/pages/main_layout_slim.dart
> index 322bcac..5fa6199 100644
> --- a/lib/pages/main_layout_slim.dart
> +++ b/lib/pages/main_layout_slim.dart
> @@ -254,7 +254,7 @@ class MobileDashboard extends StatelessWidget {
>                                   fontWeight: FontWeight.bold,
>                                   color: Colors.white),
>                             ),
> -                          onPressed: () => showDialog(
> +                          onPressed: () => (

This does not seem to be related to the issue? The subscription dialog 
cannot be opened anymore with this change.

>                               context: context,
>                               builder: (c) => const PveSubscriptionAlertDialog(),
>                             ),
> @@ -556,7 +556,7 @@ class MobileDashboard extends StatelessWidget {
>     }
>   }
>   
> -class PveNodeListTile extends StatelessWidget {
> +class PveNodeListTile extends StatelessWidget{
>     final String name;
>     final bool online;
>     final String type;
> @@ -564,21 +564,58 @@ class PveNodeListTile extends StatelessWidget {
>     final String? ip;
>     const PveNodeListTile(
>         {super.key,
> -      required this.name,
> -      required this.online,
> -      required this.type,
> -      this.level,
> -      this.ip = ''});
> +        required this.name,
> +        required this.online,
> +        required this.type,
> +        this.level,
> +        this.ip = ''});
> +
>     @override
>     Widget build(BuildContext context) {
> +    late Color splashColor;
> +    if (online){
> +      splashColor = ProxmoxColors.greyShade40;
> +    }
> +    else {
> +      splashColor = Colors.transparent;
> +    }
>       return ListTile(
>         leading: Icon(
>           Renderers.getDefaultResourceIcon(type),
>         ),
>         title: Text(name),
> +      splashColor: splashColor,
>         subtitle: Text(getNodeTileSubtitle(online, level, ip)),
>         trailing: Icon(Icons.power, color: online ? Colors.green : Colors.grey),
> -      onTap: () => Navigator.pushNamed(context, '/nodes/$name'),
> +      onTap: () => {
> +        if (online){
> +          Navigator.pushNamed(context, '/nodes/$name')
> +        }
> +        else{
> +          showDialog(
> +            context: context,
> +            builder: (context) {
> +              return AlertDialog(
> +                contentPadding: const EdgeInsets.fromLTRB(24.0, 12.0, 24.0, 16.0),
> +                title: Row(
> +                  mainAxisAlignment: MainAxisAlignment.spaceBetween,
> +                  children: <Widget>[
> +                    Text('Error'),
> +                    Icon(Icons.warning),
> +                  ],
> +                ),
> +                content: Text("Node offline."),

nit: I think "Node is offline" would feel more polished. Or it should 
not have a period, since it's not a sentence.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH dart_api_client 1/1] fix #4976: Request errors are

[Michael Köppl]

On 5/2/25 12:19, Michael Köppl wrote:
> Hi, thanks for tackling this. Noticed a few small things. Find the
> comments inline. Generally, please run dart format on these changes.
> Every changed block contains wrong indentation. Also, the commit message
> needs to be fixed.
> 
> On 4/29/25 15:16, Alexander Abraham wrote:
>> This commit adds more explicit error-handling for when a host
>> is possibly offline.
>>
>> Signed-off-by: Alexander Abraham <a.abraham@proxmox.com>
>> ---
>>   lib/src/client.dart | 14 +++++++++-----
>>   1 file changed, 9 insertions(+), 5 deletions(-)
>>
>> diff --git a/lib/src/client.dart b/lib/src/client.dart
>> index f597c28..7c118d8 100644
>> --- a/lib/src/client.dart
>> +++ b/lib/src/client.dart
>> @@ -1,6 +1,5 @@
>>   import 'dart:convert';
>>   import 'dart:io';
>> -
>>   import 'package:http/http.dart' as http;
>>   //import 'package:proxmox_dart_api_client/src/models/
>> pve_access_user_model.dart';
>>   import 'package:proxmox_dart_api_client/src/models/pve_models.dart';
>> @@ -127,7 +126,7 @@ class ProxmoxApiClient extends http.BaseClient {
>>         () => get(url),
>>         retryIf: (e) => e is http.ClientException || e is
>> SocketException,
>>       ))
>> -        .validate(extensiveResponseValidation);
>> +         .validate(extensiveResponseValidation);
>>     }
>>       Future<http.Response> _postWithValidation(
>> @@ -398,9 +397,14 @@ class ProxmoxApiClient extends http.BaseClient {
>>         'typefilter': typefilter,
>>         'userfilter': userfilter
>>       };
>> -    final response = await _getWithValidation(path, queryParameters);
>> +    var response;
>> +    try {
>> +     response = await _getWithValidation(path, queryParameters);
>> +    }
>> +    catch(e) {
>> +      throw "Host unreachable, possibly offline.";
> 
> Is there a reason this is not wrapped in a ProxmoxApiException? If the
> reason is that it then displays "Host unreachable, possibly offline. ->
> null", I think it would generally be a good idea to adapt
> ProxmoxApiException's toString method to account for the details
> possibly being null.

Just wanted to add that I sent a patch for adapting
ProxmoxApiException's toString method since I encountered this problem
elsewhere as well:

https://lore.proxmox.com/pve-devel/20250505072845.28299-1-m.koeppl@proxmox.com/T/#u



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel