diff --git a/src/PSProxmoxVE.Core/Services/ContainerService.cs b/src/PSProxmoxVE.Core/Services/ContainerService.cs
index 341f1f0..67a5be3 100644
--- a/src/PSProxmoxVE.Core/Services/ContainerService.cs
+++ b/src/PSProxmoxVE.Core/Services/ContainerService.cs
@@ -34,7 +34,13 @@ namespace PSProxmoxVE.Core.Services
///
/// Returns containers. If is null, queries every cluster node.
///
- public PveContainer[] GetContainers(PveSession session, string? node = null)
+ ///
+ /// Optional callback invoked with the node name and the exception when a node is
+ /// skipped because it is unreachable (connectivity failure or a 5xx from that node).
+ /// A 401/403/404 or any other non-5xx
+ /// propagates instead of being swallowed.
+ ///
+ public PveContainer[] GetContainers(PveSession session, string? node = null, Action? onNodeSkipped = null)
{
if (session == null) throw new ArgumentNullException(nameof(session));
@@ -52,14 +58,31 @@ namespace PSProxmoxVE.Core.Services
ct.Node ??= n.Name;
all.AddRange(containers);
}
- catch (Exception ex) when (ex is PSProxmoxVE.Core.Exceptions.PveApiException or System.Net.Http.HttpRequestException)
+ catch (Exception ex) when (IsNodeUnreachable(ex))
{
- // Skip offline/inaccessible nodes
+ onNodeSkipped?.Invoke(n.Name, ex);
}
}
return all.ToArray();
}
+ ///
+ /// True for a connectivity failure or a 5xx PVE API response — the cases where the
+ /// node itself is unreachable rather than the request being rejected. A 401/403/404
+ /// (or any other non-5xx status) means the request was understood and refused, which
+ /// is not something a per-node listing loop should hide.
+ ///
+ private static bool IsNodeUnreachable(Exception ex) => ex switch
+ {
+ System.Net.Http.HttpRequestException => true,
+ // PveHttpClient wraps a connection failure as 503 and a client-side timeout as
+ // 408 (Client/PveHttpClient.cs SendOnceAsync) — both mean the node did not answer,
+ // not that it rejected the request.
+ PSProxmoxVE.Core.Exceptions.PveApiException apiEx =>
+ apiEx.StatusCode == System.Net.HttpStatusCode.RequestTimeout || (int)apiEx.StatusCode >= 500,
+ _ => false
+ };
+
private PveContainer[] GetContainersOnNode(PveSession session, string node)
{
IPveHttpClient client = _injectedClient ?? new PveHttpClient(session);
diff --git a/src/PSProxmoxVE.Core/Services/VmService.cs b/src/PSProxmoxVE.Core/Services/VmService.cs
index 8df89d0..013140c 100644
--- a/src/PSProxmoxVE.Core/Services/VmService.cs
+++ b/src/PSProxmoxVE.Core/Services/VmService.cs
@@ -37,7 +37,13 @@ namespace PSProxmoxVE.Core.Services
///
/// The authenticated PVE session.
/// Optional cluster node name to filter VMs by node.
- public PveVm[] GetVms(PveSession session, string? node = null)
+ ///
+ /// Optional callback invoked with the node name and the exception when a node is
+ /// skipped because it is unreachable (connectivity failure or a 5xx from that node).
+ /// A 401/403/404 or any other non-5xx
+ /// propagates instead of being swallowed.
+ ///
+ public PveVm[] GetVms(PveSession session, string? node = null, Action? onNodeSkipped = null)
{
if (session == null) throw new ArgumentNullException(nameof(session));
@@ -57,14 +63,31 @@ namespace PSProxmoxVE.Core.Services
vm.Node ??= n.Name;
all.AddRange(vms);
}
- catch (Exception ex) when (ex is PSProxmoxVE.Core.Exceptions.PveApiException or System.Net.Http.HttpRequestException)
+ catch (Exception ex) when (IsNodeUnreachable(ex))
{
- // Skip nodes that are offline or inaccessible
+ onNodeSkipped?.Invoke(n.Name, ex);
}
}
return all.ToArray();
}
+ ///
+ /// True for a connectivity failure or a 5xx PVE API response — the cases where the
+ /// node itself is unreachable rather than the request being rejected. A 401/403/404
+ /// (or any other non-5xx status) means the request was understood and refused, which
+ /// is not something a per-node listing loop should hide.
+ ///
+ private static bool IsNodeUnreachable(Exception ex) => ex switch
+ {
+ System.Net.Http.HttpRequestException => true,
+ // PveHttpClient wraps a connection failure as 503 and a client-side timeout as
+ // 408 (Client/PveHttpClient.cs SendOnceAsync) — both mean the node did not answer,
+ // not that it rejected the request.
+ PSProxmoxVE.Core.Exceptions.PveApiException apiEx =>
+ apiEx.StatusCode == System.Net.HttpStatusCode.RequestTimeout || (int)apiEx.StatusCode >= 500,
+ _ => false
+ };
+
private PveVm[] GetVmsOnNode(PveSession session, string node)
{
IPveHttpClient client = _injectedClient ?? new PveHttpClient(session);
diff --git a/src/PSProxmoxVE/Cmdlets/Containers/GetPveContainerCmdlet.cs b/src/PSProxmoxVE/Cmdlets/Containers/GetPveContainerCmdlet.cs
index 30d00d2..6c7e5b4 100644
--- a/src/PSProxmoxVE/Cmdlets/Containers/GetPveContainerCmdlet.cs
+++ b/src/PSProxmoxVE/Cmdlets/Containers/GetPveContainerCmdlet.cs
@@ -59,7 +59,8 @@ namespace PSProxmoxVE.Cmdlets.Containers
WriteVerbose("Getting containers...");
var service = new ContainerService();
- IEnumerable containers = service.GetContainers(session, Node);
+ IEnumerable containers = service.GetContainers(session, Node,
+ onNodeSkipped: (nodeName, ex) => WriteWarning($"Skipping node '{nodeName}': {ex.Message}"));
if (VmId.HasValue)
containers = containers.Where(c => c.VmId == VmId.Value);
diff --git a/src/PSProxmoxVE/Cmdlets/PveCmdletBase.cs b/src/PSProxmoxVE/Cmdlets/PveCmdletBase.cs
index 3a3cc31..c3bf6f8 100644
--- a/src/PSProxmoxVE/Cmdlets/PveCmdletBase.cs
+++ b/src/PSProxmoxVE/Cmdlets/PveCmdletBase.cs
@@ -144,9 +144,16 @@ namespace PSProxmoxVE.Cmdlets
if (snapshot.StatusMatched && !snapshot.Locked)
return task;
}
- catch (Exception ex) when (ex is not OutOfMemoryException and not StackOverflowException)
+ catch (PSProxmoxVE.Core.Exceptions.PveApiException ex) when (
+ ex.StatusCode != System.Net.HttpStatusCode.Unauthorized
+ && ex.StatusCode != System.Net.HttpStatusCode.Forbidden
+ && ex.StatusCode != System.Net.HttpStatusCode.NotFound)
{
- // Ignore transient errors during polling
+ WriteVerbose($"Status poll failed, retrying: {ex.Message}");
+ }
+ catch (System.Net.Http.HttpRequestException ex)
+ {
+ WriteVerbose($"Status poll failed, retrying: {ex.Message}");
}
System.Threading.Thread.Sleep(2000);
diff --git a/src/PSProxmoxVE/Cmdlets/Vms/GetPveVmCmdlet.cs b/src/PSProxmoxVE/Cmdlets/Vms/GetPveVmCmdlet.cs
index 4b79eaf..7e1a325 100644
--- a/src/PSProxmoxVE/Cmdlets/Vms/GetPveVmCmdlet.cs
+++ b/src/PSProxmoxVE/Cmdlets/Vms/GetPveVmCmdlet.cs
@@ -75,7 +75,8 @@ namespace PSProxmoxVE.Cmdlets.Vms
WriteVerbose("Getting VMs...");
var service = new VmService();
- IEnumerable vms = service.GetVms(session, Node);
+ IEnumerable vms = service.GetVms(session, Node,
+ onNodeSkipped: (nodeName, ex) => WriteWarning($"Skipping node '{nodeName}': {ex.Message}"));
if (VmId.HasValue)
vms = vms.Where(v => v.VmId == VmId.Value);
diff --git a/tests/PSProxmoxVE.Core.Tests/Services/ContainerServiceTests.cs b/tests/PSProxmoxVE.Core.Tests/Services/ContainerServiceTests.cs
index 1e99458..533a547 100644
--- a/tests/PSProxmoxVE.Core.Tests/Services/ContainerServiceTests.cs
+++ b/tests/PSProxmoxVE.Core.Tests/Services/ContainerServiceTests.cs
@@ -1,7 +1,12 @@
+using System;
using System.Collections.Generic;
+using System.Net;
+using System.Net.Http;
+using System.Reflection;
using Moq;
using PSProxmoxVE.Core.Authentication;
using PSProxmoxVE.Core.Client;
+using PSProxmoxVE.Core.Exceptions;
using PSProxmoxVE.Core.Services;
using Xunit;
@@ -113,5 +118,126 @@ namespace PSProxmoxVE.Core.Tests.Services
Assert.NotNull(captured);
Assert.Equal("305", captured!["newid"]);
}
+
+ // ---------------------------------------------------------------------
+ // GetContainers multi-node aggregation: issue #142
+ // ---------------------------------------------------------------------
+
+ ///
+ /// Points the private NodeService field a ContainerService constructs at a mock
+ /// client, so the "list all nodes" call the multi-node overload issues is reachable
+ /// without a real HTTP connection. ContainerService(client) only injects the client
+ /// used for the per-node lxc calls; NodeService is never constructor-injectable from
+ /// ContainerService, so this is the only offline path to the aggregation loop.
+ ///
+ private static void InjectNodeServiceClient(ContainerService service, IPveHttpClient client)
+ {
+ var field = typeof(ContainerService).GetField("_nodeService", BindingFlags.NonPublic | BindingFlags.Instance)
+ ?? throw new InvalidOperationException("ContainerService._nodeService field not found.");
+ field.SetValue(service, new NodeService(client));
+ }
+
+ private static Mock SetupTwoNodeCluster()
+ {
+ var mockClient = new Mock();
+ mockClient
+ .Setup(c => c.GetAsync("nodes"))
+ .ReturnsAsync("{\"data\":[{\"node\":\"pve1\"},{\"node\":\"pve2\"}]}");
+ return mockClient;
+ }
+
+ [Fact]
+ public void GetContainers_AllNodes_A500OnOneNodeIsSkippedAndReportedButOtherNodeResultsReturn()
+ {
+ var mockClient = SetupTwoNodeCluster();
+ mockClient
+ .Setup(c => c.GetAsync("nodes/pve1/lxc"))
+ .ReturnsAsync("{\"data\":[{\"vmid\":100}]}");
+ mockClient
+ .Setup(c => c.GetAsync("nodes/pve2/lxc"))
+ .ThrowsAsync(new PveApiException(HttpStatusCode.InternalServerError, "internal error", "nodes/pve2/lxc", "GET"));
+
+ var service = new ContainerService(mockClient.Object);
+ InjectNodeServiceClient(service, mockClient.Object);
+
+ var skipped = new List();
+ var containers = service.GetContainers(CreateSession(), onNodeSkipped: (node, ex) => skipped.Add(node));
+
+ var ct = Assert.Single(containers);
+ Assert.Equal(100, ct.VmId);
+ Assert.Equal(new[] { "pve2" }, skipped);
+ }
+
+ [Fact]
+ public void GetContainers_AllNodes_A403OnOneNodePropagatesInsteadOfBeingSwallowed()
+ {
+ var mockClient = SetupTwoNodeCluster();
+ mockClient
+ .Setup(c => c.GetAsync("nodes/pve1/lxc"))
+ .ReturnsAsync("{\"data\":[{\"vmid\":100}]}");
+ mockClient
+ .Setup(c => c.GetAsync("nodes/pve2/lxc"))
+ .ThrowsAsync(new PveApiException(HttpStatusCode.Forbidden, "permission denied", "nodes/pve2/lxc", "GET"));
+
+ var service = new ContainerService(mockClient.Object);
+ InjectNodeServiceClient(service, mockClient.Object);
+
+ var ex = Assert.Throws(() => service.GetContainers(CreateSession()));
+ Assert.Equal(HttpStatusCode.Forbidden, ex.StatusCode);
+ }
+
+ [Fact]
+ public void GetContainers_AllNodes_ConnectivityFailureOnOneNodeIsSkippedAndReported()
+ {
+ // PveHttpClient.SendOnceAsync never lets a raw HttpRequestException escape — it
+ // wraps one as PveApiException(ServiceUnavailable, ..., inner: HttpRequestException).
+ // That is the shape a real connectivity failure takes by the time it reaches
+ // ContainerService, so that is what this test throws.
+ var mockClient = SetupTwoNodeCluster();
+ mockClient
+ .Setup(c => c.GetAsync("nodes/pve1/lxc"))
+ .ReturnsAsync("{\"data\":[{\"vmid\":100}]}");
+ mockClient
+ .Setup(c => c.GetAsync("nodes/pve2/lxc"))
+ .ThrowsAsync(new PveApiException(HttpStatusCode.ServiceUnavailable, "connection refused",
+ "nodes/pve2/lxc", "GET", new HttpRequestException("connection refused")));
+
+ var service = new ContainerService(mockClient.Object);
+ InjectNodeServiceClient(service, mockClient.Object);
+
+ var skipped = new List();
+ var containers = service.GetContainers(CreateSession(), onNodeSkipped: (node, ex) => skipped.Add(node));
+
+ var ct = Assert.Single(containers);
+ Assert.Equal(100, ct.VmId);
+ Assert.Equal(new[] { "pve2" }, skipped);
+ }
+
+ [Fact]
+ public void GetContainers_AllNodes_ClientTimeoutOnOneNodeIsSkippedAndReported()
+ {
+ // PveHttpClient.SendOnceAsync wraps an HttpClient timeout as
+ // PveApiException(RequestTimeout) — the case of a powered-off or
+ // firewall-blackholed node, which must be skipped like any other
+ // unreachable node rather than aborting the whole listing.
+ var mockClient = SetupTwoNodeCluster();
+ mockClient
+ .Setup(c => c.GetAsync("nodes/pve1/lxc"))
+ .ReturnsAsync("{\"data\":[{\"vmid\":100}]}");
+ mockClient
+ .Setup(c => c.GetAsync("nodes/pve2/lxc"))
+ .ThrowsAsync(new PveApiException(HttpStatusCode.RequestTimeout, "Request timed out after 100s.",
+ "nodes/pve2/lxc", "GET"));
+
+ var service = new ContainerService(mockClient.Object);
+ InjectNodeServiceClient(service, mockClient.Object);
+
+ var skipped = new List();
+ var containers = service.GetContainers(CreateSession(), onNodeSkipped: (node, ex) => skipped.Add(node));
+
+ var ct = Assert.Single(containers);
+ Assert.Equal(100, ct.VmId);
+ Assert.Equal(new[] { "pve2" }, skipped);
+ }
}
}
diff --git a/tests/PSProxmoxVE.Core.Tests/Services/VmServiceTests.cs b/tests/PSProxmoxVE.Core.Tests/Services/VmServiceTests.cs
index be05cbb..d7f9e74 100644
--- a/tests/PSProxmoxVE.Core.Tests/Services/VmServiceTests.cs
+++ b/tests/PSProxmoxVE.Core.Tests/Services/VmServiceTests.cs
@@ -1,9 +1,13 @@
using System;
using System.Collections.Generic;
using System.Linq;
+using System.Net;
+using System.Net.Http;
+using System.Reflection;
using Moq;
using PSProxmoxVE.Core.Authentication;
using PSProxmoxVE.Core.Client;
+using PSProxmoxVE.Core.Exceptions;
using PSProxmoxVE.Core.Services;
using Xunit;
@@ -268,5 +272,126 @@ namespace PSProxmoxVE.Core.Tests.Services
Assert.Equal("305", captured!["newid"]);
}
+
+ // ---------------------------------------------------------------------
+ // GetVms multi-node aggregation: issue #142
+ // ---------------------------------------------------------------------
+
+ ///
+ /// Points the private NodeService field a VmService constructs at a mock client,
+ /// so the "list all nodes" call the multi-node overload issues is reachable
+ /// without a real HTTP connection. VmService(client) only injects the client used
+ /// for the per-node qemu calls; NodeService is never constructor-injectable from
+ /// VmService, so this is the only offline path to the aggregation loop.
+ ///
+ private static void InjectNodeServiceClient(VmService service, IPveHttpClient client)
+ {
+ var field = typeof(VmService).GetField("_nodeService", BindingFlags.NonPublic | BindingFlags.Instance)
+ ?? throw new InvalidOperationException("VmService._nodeService field not found.");
+ field.SetValue(service, new NodeService(client));
+ }
+
+ private static Mock SetupTwoNodeCluster()
+ {
+ var mockClient = new Mock();
+ mockClient
+ .Setup(c => c.GetAsync("nodes"))
+ .ReturnsAsync("{\"data\":[{\"node\":\"pve1\"},{\"node\":\"pve2\"}]}");
+ return mockClient;
+ }
+
+ [Fact]
+ public void GetVms_AllNodes_A500OnOneNodeIsSkippedAndReportedButOtherNodeResultsReturn()
+ {
+ var mockClient = SetupTwoNodeCluster();
+ mockClient
+ .Setup(c => c.GetAsync("nodes/pve1/qemu"))
+ .ReturnsAsync("{\"data\":[{\"vmid\":100}]}");
+ mockClient
+ .Setup(c => c.GetAsync("nodes/pve2/qemu"))
+ .ThrowsAsync(new PveApiException(HttpStatusCode.InternalServerError, "internal error", "nodes/pve2/qemu", "GET"));
+
+ var service = new VmService(mockClient.Object);
+ InjectNodeServiceClient(service, mockClient.Object);
+
+ var skipped = new List();
+ var vms = service.GetVms(CreateSession(), onNodeSkipped: (node, ex) => skipped.Add(node));
+
+ var vm = Assert.Single(vms);
+ Assert.Equal(100, vm.VmId);
+ Assert.Equal(new[] { "pve2" }, skipped);
+ }
+
+ [Fact]
+ public void GetVms_AllNodes_A403OnOneNodePropagatesInsteadOfBeingSwallowed()
+ {
+ var mockClient = SetupTwoNodeCluster();
+ mockClient
+ .Setup(c => c.GetAsync("nodes/pve1/qemu"))
+ .ReturnsAsync("{\"data\":[{\"vmid\":100}]}");
+ mockClient
+ .Setup(c => c.GetAsync("nodes/pve2/qemu"))
+ .ThrowsAsync(new PveApiException(HttpStatusCode.Forbidden, "permission denied", "nodes/pve2/qemu", "GET"));
+
+ var service = new VmService(mockClient.Object);
+ InjectNodeServiceClient(service, mockClient.Object);
+
+ var ex = Assert.Throws(() => service.GetVms(CreateSession()));
+ Assert.Equal(HttpStatusCode.Forbidden, ex.StatusCode);
+ }
+
+ [Fact]
+ public void GetVms_AllNodes_ConnectivityFailureOnOneNodeIsSkippedAndReported()
+ {
+ // PveHttpClient.SendOnceAsync never lets a raw HttpRequestException escape — it
+ // wraps one as PveApiException(ServiceUnavailable, ..., inner: HttpRequestException).
+ // That is the shape a real connectivity failure takes by the time it reaches
+ // VmService, so that is what this test throws.
+ var mockClient = SetupTwoNodeCluster();
+ mockClient
+ .Setup(c => c.GetAsync("nodes/pve1/qemu"))
+ .ReturnsAsync("{\"data\":[{\"vmid\":100}]}");
+ mockClient
+ .Setup(c => c.GetAsync("nodes/pve2/qemu"))
+ .ThrowsAsync(new PveApiException(HttpStatusCode.ServiceUnavailable, "connection refused",
+ "nodes/pve2/qemu", "GET", new HttpRequestException("connection refused")));
+
+ var service = new VmService(mockClient.Object);
+ InjectNodeServiceClient(service, mockClient.Object);
+
+ var skipped = new List();
+ var vms = service.GetVms(CreateSession(), onNodeSkipped: (node, ex) => skipped.Add(node));
+
+ var vm = Assert.Single(vms);
+ Assert.Equal(100, vm.VmId);
+ Assert.Equal(new[] { "pve2" }, skipped);
+ }
+
+ [Fact]
+ public void GetVms_AllNodes_ClientTimeoutOnOneNodeIsSkippedAndReported()
+ {
+ // PveHttpClient.SendOnceAsync wraps an HttpClient timeout as
+ // PveApiException(RequestTimeout) — the case of a powered-off or
+ // firewall-blackholed node, which must be skipped like any other
+ // unreachable node rather than aborting the whole listing.
+ var mockClient = SetupTwoNodeCluster();
+ mockClient
+ .Setup(c => c.GetAsync("nodes/pve1/qemu"))
+ .ReturnsAsync("{\"data\":[{\"vmid\":100}]}");
+ mockClient
+ .Setup(c => c.GetAsync("nodes/pve2/qemu"))
+ .ThrowsAsync(new PveApiException(HttpStatusCode.RequestTimeout, "Request timed out after 100s.",
+ "nodes/pve2/qemu", "GET"));
+
+ var service = new VmService(mockClient.Object);
+ InjectNodeServiceClient(service, mockClient.Object);
+
+ var skipped = new List();
+ var vms = service.GetVms(CreateSession(), onNodeSkipped: (node, ex) => skipped.Add(node));
+
+ var vm = Assert.Single(vms);
+ Assert.Equal(100, vm.VmId);
+ Assert.Equal(new[] { "pve2" }, skipped);
+ }
}
}