Merge pull request #61 from GoodOlClint/fix/http-client-timeout

fix: add -TimeoutSeconds for long-running HTTP calls (#59)
This commit is contained in:
GoodOlClint
2026-05-20 18:26:08 -05:00
committed by GitHub
12 changed files with 301 additions and 9 deletions
+35
View File
@@ -2846,6 +2846,41 @@
"evidence": "Added SizeParser.NormalizeToGibibytes() which strips G/GB/T/TB suffixes, rejects sub-GB units with a clear error, and converts TB overflows to ArgumentException. New-PveVm and New-PveContainer normalize -DiskSize and -RootFsSize before ShouldProcess so typos are caught with -WhatIf, regardless of whether the matching -DiskStorage/-RootFsStorage was supplied.",
"verified_by": "dotnet build + dotnet test (577 passed, 32 SizeParserTests) + Pester (39 passed, new DiskSize/RootFsSize validation contexts)"
}
},
{
"id": "F087",
"title": "HttpClient uses 100s default timeout; Send-PveFile and other long-running calls fail on large payloads",
"category": "reliability",
"severity": "high",
"status": "resolved",
"first_detected": "2026-05-20",
"github_issue": 59,
"files": [
"src/PSProxmoxVE.Core/Client/PveHttpClient.cs",
"src/PSProxmoxVE.Core/Authentication/PveSession.cs",
"src/PSProxmoxVE.Core/Authentication/PveAuthenticator.cs",
"src/PSProxmoxVE/Cmdlets/Storage/SendPveFileCmdlet.cs",
"src/PSProxmoxVE/Cmdlets/Storage/InvokePveStorageDownloadCmdlet.cs",
"src/PSProxmoxVE/Cmdlets/Connection/ConnectPveServerCmdlet.cs"
],
"description": "PveHttpClient constructs HttpClient without setting Timeout, so .NET's 100s default applies to every request. Send-PveFile, Invoke-PveStorageDownload, and Connect-PveServer expose no way to override it, so multi-GB ISO uploads on a real LAN reliably trip the 100s timeout with TaskCanceledException. PveHttpClient.SendAsync also failed to surface the timeout as a PveApiException, leaking the raw TaskCanceledException to callers.",
"scan_history": [
{
"scan_date": "2026-05-20",
"local_id": null,
"status": "new"
},
{
"scan_date": "2026-05-20",
"local_id": null,
"status": "fixed"
}
],
"resolution": {
"scan_date": "2026-05-20",
"evidence": "Added PveSession.Timeout (default 100s) and a TimeSpan? override on PveHttpClient. Connect-PveServer exposes -TimeoutSeconds to set the session-default; Send-PveFile and Invoke-PveStorageDownload expose -TimeoutSeconds with a 30-minute implicit default so large uploads/downloads do not trip the 100s HttpClient default. -TimeoutSeconds 0 means Timeout.InfiniteTimeSpan. PveHttpClient.SendAsync now catches the TimeoutException-wrapped TaskCanceledException and rethrows it as PveApiException(RequestTimeout) with the resource path.",
"verified_by": "dotnet build + dotnet test (passed including new SendAsync_TimeoutFires xUnit test) + Pester (-TimeoutSeconds coverage on all three cmdlets)"
}
}
]
}
@@ -18,12 +18,22 @@ namespace PSProxmoxVE.Core.Authentication
/// Authenticates using username and password, obtaining a ticket and CSRF token.
/// The username must be in the form user@realm (e.g. root@pam).
/// </summary>
/// <param name="hostname">Hostname or IP address of the Proxmox VE server.</param>
/// <param name="port">TCP port of the Proxmox VE API.</param>
/// <param name="skipCertificateCheck">When true, skips TLS certificate validation.</param>
/// <param name="username">Username including realm (e.g. root@pam).</param>
/// <param name="password">Plain-text password for the user.</param>
/// <param name="timeout">
/// Optional HTTP timeout to apply both to the authentication call and to subsequent
/// requests made with this session. When null, the default 100s applies.
/// </param>
public static PveSession AuthenticateWithCredentials(
string hostname,
int port,
bool skipCertificateCheck,
string username,
string password)
string password,
TimeSpan? timeout = null)
{
if (string.IsNullOrWhiteSpace(hostname))
throw new ArgumentException("Hostname cannot be null or empty.", nameof(hostname));
@@ -41,7 +51,7 @@ namespace PSProxmoxVE.Core.Authentication
};
string responseBody;
using (var httpClient = new PveHttpClient(hostname, port, skipCertificateCheck))
using (var httpClient = new PveHttpClient(hostname, port, skipCertificateCheck, timeout))
{
responseBody = httpClient.Post("/api2/json/access/ticket", formData);
}
@@ -57,6 +67,8 @@ namespace PSProxmoxVE.Core.Authentication
var ticketExpiry = DateTime.UtcNow.AddHours(2);
var session = new PveSession(hostname, port, skipCertificateCheck, ticket, csrfToken, ticketExpiry);
if (timeout.HasValue)
session.Timeout = timeout.Value;
session.ServerVersion = GetVersion(session);
@@ -67,11 +79,20 @@ namespace PSProxmoxVE.Core.Authentication
/// Authenticates using a Proxmox VE API token.
/// The token must be in the format USER@REALM!TOKENID=UUID (e.g. root@pam!mytoken=xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx).
/// </summary>
/// <param name="hostname">Hostname or IP address of the Proxmox VE server.</param>
/// <param name="port">TCP port of the Proxmox VE API.</param>
/// <param name="skipCertificateCheck">When true, skips TLS certificate validation.</param>
/// <param name="apiToken">API token in USER@REALM!TOKENID=UUID format.</param>
/// <param name="timeout">
/// Optional HTTP timeout to apply to requests made with this session.
/// When null, the default 100s applies.
/// </param>
public static PveSession AuthenticateWithApiToken(
string hostname,
int port,
bool skipCertificateCheck,
string apiToken)
string apiToken,
TimeSpan? timeout = null)
{
if (string.IsNullOrWhiteSpace(hostname))
throw new ArgumentException("Hostname cannot be null or empty.", nameof(hostname));
@@ -83,6 +104,8 @@ namespace PSProxmoxVE.Core.Authentication
nameof(apiToken));
var session = new PveSession(hostname, port, skipCertificateCheck, apiToken);
if (timeout.HasValue)
session.Timeout = timeout.Value;
session.ServerVersion = GetVersion(session);
@@ -33,6 +33,13 @@ namespace PSProxmoxVE.Core.Authentication
/// <summary>The Proxmox VE version detected on the server at connection time.</summary>
public PveVersion? ServerVersion { get; internal set; }
/// <summary>
/// The default HTTP request timeout applied to clients created with this session.
/// Defaults to 100 seconds (HttpClient's built-in default). Cmdlets that perform
/// long-running operations (e.g. Send-PveFile) may override this per-call.
/// </summary>
public TimeSpan Timeout { get; internal set; } = TimeSpan.FromSeconds(100);
/// <summary>Returns true if the ticket has expired (only relevant for Ticket auth mode)</summary>
public bool IsExpired
{
+22 -2
View File
@@ -37,7 +37,12 @@ namespace PSProxmoxVE.Core.Client
/// Creates an HTTP client authenticated with the specified PVE session.
/// </summary>
/// <param name="session">The authenticated PVE session providing credentials and base URL.</param>
public PveHttpClient(PveSession session)
/// <param name="timeoutOverride">
/// Optional per-instance timeout override. When supplied, takes precedence over
/// <see cref="PveSession.Timeout"/>. Pass <see cref="System.Threading.Timeout.InfiniteTimeSpan"/>
/// to disable the timeout entirely (useful for multi-GB uploads/downloads).
/// </param>
public PveHttpClient(PveSession session, TimeSpan? timeoutOverride = null)
{
_session = session ?? throw new ArgumentNullException(nameof(session));
_baseUrl = session.BaseUrl;
@@ -49,6 +54,7 @@ namespace PSProxmoxVE.Core.Client
(HttpRequestMessage _, X509Certificate2 _, X509Chain _, SslPolicyErrors _) => true;
}
_httpClient = new HttpClient(handler);
_httpClient.Timeout = timeoutOverride ?? session.Timeout;
_httpClient.DefaultRequestHeaders.Accept.Add(
new MediaTypeWithQualityHeaderValue("application/json"));
@@ -58,7 +64,7 @@ namespace PSProxmoxVE.Core.Client
/// Creates a bare HTTP client for pre-session use (e.g. initial authentication).
/// No auth headers are added to requests made with this constructor.
/// </summary>
internal PveHttpClient(string hostname, int port, bool skipCertificateCheck)
internal PveHttpClient(string hostname, int port, bool skipCertificateCheck, TimeSpan? timeout = null)
{
if (string.IsNullOrWhiteSpace(hostname))
throw new ArgumentException("Hostname cannot be null or empty.", nameof(hostname));
@@ -73,6 +79,8 @@ namespace PSProxmoxVE.Core.Client
(HttpRequestMessage _, X509Certificate2 _, X509Chain _, SslPolicyErrors _) => true;
}
_httpClient = new HttpClient(handler);
if (timeout.HasValue)
_httpClient.Timeout = timeout.Value;
_httpClient.DefaultRequestHeaders.Accept.Add(
new MediaTypeWithQualityHeaderValue("application/json"));
@@ -345,6 +353,18 @@ namespace PSProxmoxVE.Core.Client
{
response = await _httpClient.SendAsync(request).ConfigureAwait(false);
}
catch (TaskCanceledException ex)
{
// PveHttpClient.SendAsync passes no CancellationToken to HttpClient.SendAsync,
// so a TaskCanceledException reaching here can only be HttpClient.Timeout
// firing — on .NET Framework, on .NET Core, and on .NET 5+ (where it also
// carries a TimeoutException inner). Wrap it uniformly across frameworks.
var seconds = _httpClient.Timeout == System.Threading.Timeout.InfiniteTimeSpan
? "infinite"
: _httpClient.Timeout.TotalSeconds.ToString("0", System.Globalization.CultureInfo.InvariantCulture) + "s";
throw new PveApiException(HttpStatusCode.RequestTimeout,
$"Request timed out after {seconds}.", resource, httpMethod, ex);
}
catch (HttpRequestException ex)
{
throw new PveApiException(HttpStatusCode.ServiceUnavailable,
@@ -48,6 +48,17 @@ namespace PSProxmoxVE.Cmdlets.Connection
[Parameter(Mandatory = false, HelpMessage = "Skip TLS certificate validation.")]
public SwitchParameter SkipCertificateCheck { get; set; }
/// <summary>
/// HTTP request timeout in seconds for all calls made with this session.
/// Defaults to 100 seconds (HttpClient's built-in default). Pass 0 to disable
/// the timeout entirely. Cmdlets that perform long-running operations
/// (Send-PveFile, Invoke-PveStorageDownload) accept their own -TimeoutSeconds
/// that overrides this value per-call.
/// </summary>
[Parameter(Mandatory = false, HelpMessage = "HTTP timeout in seconds (0 = infinite). Default 100s.")]
[ValidateRange(0, int.MaxValue)]
public int? TimeoutSeconds { get; set; }
/// <summary>Deprecated — session is now always output. Kept for backwards compatibility.</summary>
[Parameter(Mandatory = false, DontShow = true)]
public SwitchParameter PassThru { get; set; }
@@ -59,6 +70,13 @@ namespace PSProxmoxVE.Cmdlets.Connection
protected override void ProcessRecord()
{
PveSession session;
TimeSpan? timeout = null;
if (TimeoutSeconds.HasValue)
{
timeout = TimeoutSeconds.Value == 0
? System.Threading.Timeout.InfiniteTimeSpan
: TimeSpan.FromSeconds(TimeoutSeconds.Value);
}
switch (ParameterSetName)
{
@@ -77,7 +95,7 @@ namespace PSProxmoxVE.Cmdlets.Connection
try
{
session = PveAuthenticator.AuthenticateWithCredentials(
Server, Port, SkipCertificateCheck.IsPresent, username, password);
Server, Port, SkipCertificateCheck.IsPresent, username, password, timeout);
}
catch (Exception ex)
{
@@ -96,7 +114,7 @@ namespace PSProxmoxVE.Cmdlets.Connection
try
{
session = PveAuthenticator.AuthenticateWithApiToken(
Server, Port, SkipCertificateCheck.IsPresent, ApiToken!);
Server, Port, SkipCertificateCheck.IsPresent, ApiToken!, timeout);
}
catch (Exception ex)
{
@@ -44,6 +44,16 @@ namespace PSProxmoxVE.Cmdlets.Storage
[Parameter(Mandatory = false, HelpMessage = "Wait for the task to complete before returning.")]
public SwitchParameter Wait { get; set; }
/// <summary>
/// HTTP timeout for issuing the download request, in seconds. Pass 0 for infinite.
/// When omitted, defaults to 30 minutes. Note: this only bounds the API call that
/// schedules the download; the actual file transfer runs server-side and is tracked
/// by the returned task.
/// </summary>
[Parameter(Mandatory = false, HelpMessage = "HTTP timeout in seconds (0 = infinite). Defaults to 1800 (30 min).")]
[ValidateRange(0, int.MaxValue)]
public int? TimeoutSeconds { get; set; }
protected override void ProcessRecord()
{
if (!ShouldProcess($"{Node}/{Storage}/{Filename}", $"Download from {Url}"))
@@ -51,7 +61,19 @@ namespace PSProxmoxVE.Cmdlets.Storage
var session = GetSession();
RequireVersion(session, "Storage URL download", 7, 0);
using var client = new PveHttpClient(session);
TimeSpan timeout;
if (TimeoutSeconds.HasValue)
{
timeout = TimeoutSeconds.Value == 0
? System.Threading.Timeout.InfiniteTimeSpan
: TimeSpan.FromSeconds(TimeoutSeconds.Value);
}
else
{
timeout = TimeSpan.FromMinutes(30);
}
using var client = new PveHttpClient(session, timeout);
WriteVerbose($"Downloading '{Url}' to {Node}/{Storage}...");
var resource = $"nodes/{Uri.EscapeDataString(Node)}/storage/{Uri.EscapeDataString(Storage)}/download-url";
@@ -60,6 +60,15 @@ namespace PSProxmoxVE.Cmdlets.Storage
[Parameter(Mandatory = false, HelpMessage = "Wait for the task to complete before returning.")]
public SwitchParameter Wait { get; set; }
/// <summary>
/// HTTP timeout for this upload, in seconds. Pass 0 for infinite (no timeout).
/// When omitted, defaults to 30 minutes — overriding the session timeout so that
/// large file uploads do not trip the default 100-second HttpClient timeout.
/// </summary>
[Parameter(Mandatory = false, HelpMessage = "HTTP timeout in seconds (0 = infinite). Defaults to 1800 (30 min).")]
[ValidateRange(0, int.MaxValue)]
public int? TimeoutSeconds { get; set; }
protected override void ProcessRecord()
{
var fileName = System.IO.Path.GetFileName(Path);
@@ -75,7 +84,18 @@ namespace PSProxmoxVE.Cmdlets.Storage
+ $"Connected server is PVE {session.ServerVersion}. The upload will proceed without checksum verification.");
}
using var client = new PveHttpClient(session);
TimeSpan timeout;
if (TimeoutSeconds.HasValue)
{
timeout = TimeoutSeconds.Value == 0
? System.Threading.Timeout.InfiniteTimeSpan
: TimeSpan.FromSeconds(TimeoutSeconds.Value);
}
else
{
timeout = TimeSpan.FromMinutes(30);
}
using var client = new PveHttpClient(session, timeout);
WriteVerbose($"Uploading {fileName} to {Node}/{Storage} (content={ContentType})...");
var resource = $"nodes/{Uri.EscapeDataString(Node)}/storage/{Uri.EscapeDataString(Storage)}/upload";
@@ -100,5 +100,14 @@ namespace PSProxmoxVE.Core.Tests.Authentication
Assert.True(session.SkipCertificateCheck);
}
[Fact]
public void Timeout_DefaultIs100Seconds()
{
var session = new PveSession(TestHostname, TestPort, false,
"root@pam!mytoken=aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee");
Assert.Equal(TimeSpan.FromSeconds(100), session.Timeout);
}
}
}
@@ -0,0 +1,101 @@
using System;
using System.Net;
using System.Net.Http;
using System.Reflection;
using System.Threading;
using System.Threading.Tasks;
using PSProxmoxVE.Core.Authentication;
using PSProxmoxVE.Core.Client;
using PSProxmoxVE.Core.Exceptions;
using Xunit;
namespace PSProxmoxVE.Core.Tests.Client
{
public class PveHttpClientTimeoutTests
{
private static HttpClient GetInnerHttpClient(PveHttpClient client)
{
var field = typeof(PveHttpClient).GetField("_httpClient",
BindingFlags.Instance | BindingFlags.NonPublic)!;
return (HttpClient)field.GetValue(client)!;
}
private static void SetInnerHttpClient(PveHttpClient client, HttpClient newInner)
{
var field = typeof(PveHttpClient).GetField("_httpClient",
BindingFlags.Instance | BindingFlags.NonPublic)!;
((HttpClient)field.GetValue(client)!).Dispose();
field.SetValue(client, newInner);
}
private static PveSession NewSession()
{
return new PveSession("pve.example.com", 8006, false,
"root@pam!token=aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee");
}
[Fact]
public void Constructor_UsesSessionTimeoutByDefault()
{
var session = NewSession();
session.Timeout = TimeSpan.FromSeconds(42);
using var client = new PveHttpClient(session);
Assert.Equal(TimeSpan.FromSeconds(42), GetInnerHttpClient(client).Timeout);
}
[Fact]
public void Constructor_OverrideTakesPrecedenceOverSessionTimeout()
{
var session = NewSession();
session.Timeout = TimeSpan.FromSeconds(42);
using var client = new PveHttpClient(session, TimeSpan.FromMinutes(30));
Assert.Equal(TimeSpan.FromMinutes(30), GetInnerHttpClient(client).Timeout);
}
[Fact]
public void Constructor_InfiniteTimeSpanIsAccepted()
{
var session = NewSession();
using var client = new PveHttpClient(session, Timeout.InfiniteTimeSpan);
Assert.Equal(Timeout.InfiniteTimeSpan, GetInnerHttpClient(client).Timeout);
}
[Fact]
public async Task SendAsync_TimeoutFires_ThrowsPveApiExceptionWithRequestTimeout()
{
var session = NewSession();
using var client = new PveHttpClient(session);
// Swap in an HttpClient with a delaying handler and a 50ms timeout so
// HttpClient.Timeout fires reliably without any real network.
var delayingClient = new HttpClient(new DelayingHandler(TimeSpan.FromSeconds(30)))
{
Timeout = TimeSpan.FromMilliseconds(50)
};
SetInnerHttpClient(client, delayingClient);
var ex = await Assert.ThrowsAsync<PveApiException>(() => client.GetAsync("version"));
Assert.Equal(HttpStatusCode.RequestTimeout, ex.StatusCode);
Assert.Contains("timed out", ex.Message, StringComparison.OrdinalIgnoreCase);
}
private sealed class DelayingHandler : HttpMessageHandler
{
private readonly TimeSpan _delay;
public DelayingHandler(TimeSpan delay) { _delay = delay; }
protected override async Task<HttpResponseMessage> SendAsync(
HttpRequestMessage request, CancellationToken cancellationToken)
{
await Task.Delay(_delay, cancellationToken).ConfigureAwait(false);
return new HttpResponseMessage(HttpStatusCode.OK);
}
}
}
}
@@ -112,5 +112,17 @@ Describe 'Connect-PveServer' {
$overlap = $credSets | Where-Object { $tokenSets -contains $_ }
$overlap | Should -BeNullOrEmpty
}
It 'Should have a TimeoutSeconds parameter' {
$script:Cmd.Parameters.ContainsKey('TimeoutSeconds') | Should -BeTrue
}
It 'TimeoutSeconds should reject negative values' {
$securePass = ConvertTo-SecureString 'hunter2' -AsPlainText -Force
$cred = [System.Management.Automation.PSCredential]::new('root@pam', $securePass)
{
Connect-PveServer -Server 'pve.example.com' -Credential $cred -TimeoutSeconds -1 -ErrorAction Stop
} | Should -Throw
}
}
}
@@ -154,6 +154,20 @@ Describe 'Send-PveFile' {
if (-not $script:CmdExists) { Set-ItResult -Skipped -Because 'Not yet compiled'; return }
$script:Cmd.Parameters.ContainsKey('Session') | Should -BeTrue
}
It 'Should have TimeoutSeconds parameter' {
if (-not $script:CmdExists) { Set-ItResult -Skipped -Because 'Not yet compiled'; return }
$script:Cmd.Parameters.ContainsKey('TimeoutSeconds') | Should -BeTrue
}
It 'TimeoutSeconds should reject negative values' {
if (-not $script:CmdExists) { Set-ItResult -Skipped -Because 'Not yet compiled'; return }
$tmpIso = [System.IO.Path]::GetTempFileName()
try {
{ Send-PveFile -Node 'n' -Storage 's' -Path $tmpIso -TimeoutSeconds -1 -Confirm:$false -ErrorAction Stop } |
Should -Throw
} finally { Remove-Item $tmpIso -ErrorAction SilentlyContinue }
}
}
Context 'Without active session' {
@@ -150,6 +150,17 @@ Describe 'Invoke-PveStorageDownload' {
$script:Cmd.Parameters.ContainsKey('Wait') | Should -BeTrue
$script:Cmd.Parameters['Wait'].SwitchParameter | Should -BeTrue
}
It 'Should have TimeoutSeconds parameter' {
Skip-IfMissing 'Invoke-PveStorageDownload'
$script:Cmd.Parameters.ContainsKey('TimeoutSeconds') | Should -BeTrue
}
It 'TimeoutSeconds should reject negative values' {
Skip-IfMissing 'Invoke-PveStorageDownload'
{ Invoke-PveStorageDownload -Node 'pve1' -Storage 'local' -Url 'https://example.com/test.iso' -Filename 'test.iso' -TimeoutSeconds -1 -Confirm:$false -ErrorAction Stop } |
Should -Throw
}
}
Context 'Session parameter' {