From 2ced2e18bb3b5433c1be95b573e57de3e2537a80 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 30 Mar 2026 10:23:13 +0000 Subject: [PATCH] fix: improve reliability across agent, backend, and web layers - BackendApiClient: dispose HttpRequestMessage/HttpResponseMessage, cap Retry-After delay - EmergencyReporter: add lock for thread-safe anti-flood state access - GeoLocationService: use static HttpClient to prevent socket exhaustion - EnrollmentTracker: wrap fire-and-forget Task.Run in try-catch - NtpTimeCheckService: validate NTP response length and leap indicator - GraphTokenService: cap Retry-After delay at 120s - NdjsonParser: gracefully skip malformed JSON event lines - authenticatedFetch: add 30s default timeout via AbortSignal - SignalRContext: clear retry timeout on unmount to prevent memory leak - MCP client: add 30s request timeout https://claude.ai/code/session_01EWzmYXFmXUaPpeksi6WGKZ --- .../Monitoring/Network/BackendApiClient.cs | 232 ++++++++++-------- .../Monitoring/Network/EmergencyReporter.cs | 58 +++-- .../Monitoring/Network/GeoLocationService.cs | 19 +- .../Monitoring/Network/NtpTimeCheckService.cs | 20 +- .../Monitoring/Tracking/EnrollmentTracker.cs | 14 +- .../Functions/Ingest/NdjsonParser.cs | 16 +- .../Security/GraphTokenService.cs | 7 +- .../autopilot-monitor-mcp/src/client.ts | 13 +- .../contexts/SignalRContext.tsx | 8 +- .../lib/authenticatedFetch.ts | 15 +- 10 files changed, 254 insertions(+), 148 deletions(-) diff --git a/src/Agent/AutopilotMonitor.Agent.Core/Monitoring/Network/BackendApiClient.cs b/src/Agent/AutopilotMonitor.Agent.Core/Monitoring/Network/BackendApiClient.cs index 41f3e6f6..0444c4d4 100644 --- a/src/Agent/AutopilotMonitor.Agent.Core/Monitoring/Network/BackendApiClient.cs +++ b/src/Agent/AutopilotMonitor.Agent.Core/Monitoring/Network/BackendApiClient.cs @@ -137,43 +137,47 @@ namespace AutopilotMonitor.Agent.Core.Monitoring.Network _logger?.Debug($"IngestEventsAsync: POST {url} — {request.Events.Count} events, {compressedContent.Length} bytes compressed"); _logger?.Verbose($"IngestEventsAsync: NDJSON {ndjsonLength} bytes → gzip {compressedContent.Length} bytes ({(ndjsonLength > 0 ? (100 - compressedContent.Length * 100 / ndjsonLength) : 0)}% reduction)"); - var httpRequest = new HttpRequestMessage(HttpMethod.Post, url) + using (var httpRequest = new HttpRequestMessage(HttpMethod.Post, url) { Content = new ByteArrayContent(compressedContent) - }; - httpRequest.Content.Headers.ContentType = new System.Net.Http.Headers.MediaTypeHeaderValue("application/x-ndjson"); - httpRequest.Content.Headers.Add("Content-Encoding", "gzip"); - - // Add TenantId header so the backend can run security checks before parsing the body - httpRequest.Headers.Add("X-Tenant-Id", request.TenantId); - - // Add additional security headers for device authorization (client cert is at TLS layer, but we can add hardware info in headers for whitelist validation and Autopilot device verification) - AddSecurityHeaders(httpRequest); - - var sw = Stopwatch.StartNew(); - var failed = false; - long bytesDown = 0; - try + }) { - var httpResponse = await _httpClient.SendAsync(httpRequest); - ThrowOnAuthFailure(httpResponse); - httpResponse.EnsureSuccessStatusCode(); + httpRequest.Content.Headers.ContentType = new System.Net.Http.Headers.MediaTypeHeaderValue("application/x-ndjson"); + httpRequest.Content.Headers.Add("Content-Encoding", "gzip"); - var responseJson = await httpResponse.Content.ReadAsStringAsync(); - bytesDown = Encoding.UTF8.GetByteCount(responseJson); - var response = JsonConvert.DeserializeObject(responseJson); + // Add TenantId header so the backend can run security checks before parsing the body + httpRequest.Headers.Add("X-Tenant-Id", request.TenantId); - return response; - } - catch - { - failed = true; - throw; - } - finally - { - sw.Stop(); - _networkMetrics.RecordRequest(compressedContent.Length, bytesDown, sw.ElapsedMilliseconds, failed); + // Add additional security headers for device authorization (client cert is at TLS layer, but we can add hardware info in headers for whitelist validation and Autopilot device verification) + AddSecurityHeaders(httpRequest); + + var sw = Stopwatch.StartNew(); + var failed = false; + long bytesDown = 0; + try + { + using (var httpResponse = await _httpClient.SendAsync(httpRequest)) + { + ThrowOnAuthFailure(httpResponse); + httpResponse.EnsureSuccessStatusCode(); + + var responseJson = await httpResponse.Content.ReadAsStringAsync(); + bytesDown = Encoding.UTF8.GetByteCount(responseJson); + var response = JsonConvert.DeserializeObject(responseJson); + + return response; + } + } + catch + { + failed = true; + throw; + } + finally + { + sw.Stop(); + _networkMetrics.RecordRequest(compressedContent.Length, bytesDown, sw.ElapsedMilliseconds, failed); + } } } @@ -189,54 +193,58 @@ namespace AutopilotMonitor.Agent.Core.Monitoring.Network // Retry on 503 (backend signals transient device validation failure) const int maxAttempts = 3; + const int maxRetryAfterSeconds = 120; // Cap Retry-After to prevent indefinite blocking for (int attempt = 1; attempt <= maxAttempts; attempt++) { - var httpRequest = new HttpRequestMessage(HttpMethod.Get, url); - AddSecurityHeaders(httpRequest); - - _logger?.Debug($"GetAgentConfigAsync: GET {url} (attempt {attempt}/{maxAttempts})"); - - var sw = Stopwatch.StartNew(); - var failed = false; - long bytesDown = 0; - try + using (var httpRequest = new HttpRequestMessage(HttpMethod.Get, url)) { - var response = await _httpClient.SendAsync(httpRequest); + AddSecurityHeaders(httpRequest); - if (response.StatusCode == System.Net.HttpStatusCode.ServiceUnavailable && attempt < maxAttempts) + _logger?.Debug($"GetAgentConfigAsync: GET {url} (attempt {attempt}/{maxAttempts})"); + + var sw = Stopwatch.StartNew(); + var failed = false; + long bytesDown = 0; + try { - // Backend says "try again" — parse Retry-After or default to 30s - var retryAfterSeconds = 30; - if (response.Headers.TryGetValues("Retry-After", out var retryValues) && - int.TryParse(System.Linq.Enumerable.FirstOrDefault(retryValues), out var parsedRetry)) + using (var response = await _httpClient.SendAsync(httpRequest)) { - retryAfterSeconds = parsedRetry; + if (response.StatusCode == System.Net.HttpStatusCode.ServiceUnavailable && attempt < maxAttempts) + { + // Backend says "try again" — parse Retry-After or default to 30s + var retryAfterSeconds = 30; + if (response.Headers.TryGetValues("Retry-After", out var retryValues) && + int.TryParse(System.Linq.Enumerable.FirstOrDefault(retryValues), out var parsedRetry)) + { + retryAfterSeconds = Math.Min(parsedRetry, maxRetryAfterSeconds); + } + + _logger?.Warning($"GetAgentConfigAsync: 503 Service Unavailable (attempt {attempt}/{maxAttempts}). Retrying in {retryAfterSeconds}s..."); + sw.Stop(); + _networkMetrics.RecordRequest(0, 0, sw.ElapsedMilliseconds, true); + await System.Threading.Tasks.Task.Delay(retryAfterSeconds * 1000); + continue; + } + + ThrowOnAuthFailure(response); + response.EnsureSuccessStatusCode(); + + var responseJson = await response.Content.ReadAsStringAsync(); + bytesDown = Encoding.UTF8.GetByteCount(responseJson); + var result = JsonConvert.DeserializeObject(responseJson); + return result; } - - _logger?.Warning($"GetAgentConfigAsync: 503 Service Unavailable (attempt {attempt}/{maxAttempts}). Retrying in {retryAfterSeconds}s..."); - sw.Stop(); - _networkMetrics.RecordRequest(0, 0, sw.ElapsedMilliseconds, true); - await System.Threading.Tasks.Task.Delay(retryAfterSeconds * 1000); - continue; } - - ThrowOnAuthFailure(response); - response.EnsureSuccessStatusCode(); - - var responseJson = await response.Content.ReadAsStringAsync(); - bytesDown = Encoding.UTF8.GetByteCount(responseJson); - var result = JsonConvert.DeserializeObject(responseJson); - return result; - } - catch - { - failed = true; - throw; - } - finally - { - sw.Stop(); - _networkMetrics.RecordRequest(0, bytesDown, sw.ElapsedMilliseconds, failed); + catch + { + failed = true; + throw; + } + finally + { + sw.Stop(); + _networkMetrics.RecordRequest(0, bytesDown, sw.ElapsedMilliseconds, failed); + } } } @@ -278,20 +286,23 @@ namespace AutopilotMonitor.Agent.Core.Monitoring.Network var url = $"{_baseUrl}{errorEndpoint}"; var json = JsonConvert.SerializeObject(report); - var httpRequest = new HttpRequestMessage(HttpMethod.Post, url) + using (var httpRequest = new HttpRequestMessage(HttpMethod.Post, url) { Content = new StringContent(json, Encoding.UTF8, "application/json") - }; + }) + { + // Security validation on the backend requires tenant ID before parsing the body + httpRequest.Headers.Add("X-Tenant-Id", report.TenantId); - // Security validation on the backend requires tenant ID before parsing the body - httpRequest.Headers.Add("X-Tenant-Id", report.TenantId); + // Hardware headers are required for full security validation + AddSecurityHeaders(httpRequest); - // Hardware headers are required for full security validation - AddSecurityHeaders(httpRequest); - - using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(5)); - await _httpClient.SendAsync(httpRequest, cts.Token); - // Response status is deliberately ignored — endpoint always returns 200 + using (var cts = new CancellationTokenSource(TimeSpan.FromSeconds(5))) + using (var response = await _httpClient.SendAsync(httpRequest, cts.Token)) + { + // Response status is deliberately ignored — endpoint always returns 200 + } + } } catch (Exception ex) { @@ -305,38 +316,41 @@ namespace AutopilotMonitor.Agent.Core.Monitoring.Network var json = JsonConvert.SerializeObject(data); var bytesUp = Encoding.UTF8.GetByteCount(json); - var httpRequest = new HttpRequestMessage(HttpMethod.Post, url) + using (var httpRequest = new HttpRequestMessage(HttpMethod.Post, url) { Content = new StringContent(json, Encoding.UTF8, "application/json") - }; - - // Add additional security headers for device authorization (client cert is at TLS layer, but we can add hardware info in headers for whitelist validation and Autopilot device verification) - AddSecurityHeaders(httpRequest); - - var sw = Stopwatch.StartNew(); - var failed = false; - long bytesDown = 0; - try + }) { - var response = await _httpClient.SendAsync(httpRequest); - ThrowOnAuthFailure(response); - response.EnsureSuccessStatusCode(); + // Add additional security headers for device authorization (client cert is at TLS layer, but we can add hardware info in headers for whitelist validation and Autopilot device verification) + AddSecurityHeaders(httpRequest); - var responseJson = await response.Content.ReadAsStringAsync(); - bytesDown = Encoding.UTF8.GetByteCount(responseJson); - var result = JsonConvert.DeserializeObject(responseJson); + var sw = Stopwatch.StartNew(); + var failed = false; + long bytesDown = 0; + try + { + using (var response = await _httpClient.SendAsync(httpRequest)) + { + ThrowOnAuthFailure(response); + response.EnsureSuccessStatusCode(); - return result; - } - catch - { - failed = true; - throw; - } - finally - { - sw.Stop(); - _networkMetrics.RecordRequest(bytesUp, bytesDown, sw.ElapsedMilliseconds, failed); + var responseJson = await response.Content.ReadAsStringAsync(); + bytesDown = Encoding.UTF8.GetByteCount(responseJson); + var result = JsonConvert.DeserializeObject(responseJson); + + return result; + } + } + catch + { + failed = true; + throw; + } + finally + { + sw.Stop(); + _networkMetrics.RecordRequest(bytesUp, bytesDown, sw.ElapsedMilliseconds, failed); + } } } diff --git a/src/Agent/AutopilotMonitor.Agent.Core/Monitoring/Network/EmergencyReporter.cs b/src/Agent/AutopilotMonitor.Agent.Core/Monitoring/Network/EmergencyReporter.cs index 2a2fd4ee..3979343b 100644 --- a/src/Agent/AutopilotMonitor.Agent.Core/Monitoring/Network/EmergencyReporter.cs +++ b/src/Agent/AutopilotMonitor.Agent.Core/Monitoring/Network/EmergencyReporter.cs @@ -34,6 +34,7 @@ namespace AutopilotMonitor.Agent.Core.Monitoring.Network private readonly AgentLogger _logger; private readonly HashSet _reportedKeys = new HashSet(); + private readonly object _antiFloodLock = new object(); private DateTime? _lastReportTime; private int _reportCount; @@ -64,34 +65,41 @@ namespace AutopilotMonitor.Agent.Core.Monitoring.Network int? httpStatusCode = null, long? sequenceNumber = null) { - // Deduplicate by error type + status code: same failure category sent only once per session + // Deduplicate by error type + status code: same failure category sent only once per session. + // All anti-flood checks are protected by a lock to prevent race conditions when + // TrySendAsync is called concurrently (fire-and-forget from multiple threads). var key = $"{errorType}:{httpStatusCode}"; + int currentReportCount; - if (_reportCount >= MaxReportsPerSession) + lock (_antiFloodLock) { - _logger?.Debug($"[EmergencyChannel] Suppressed ({_reportCount}/{MaxReportsPerSession} reports used): {key}"); - return; - } + if (_reportCount >= MaxReportsPerSession) + { + _logger?.Debug($"[EmergencyChannel] Suppressed ({_reportCount}/{MaxReportsPerSession} reports used): {key}"); + return; + } - if (_reportedKeys.Contains(key)) - { - _logger?.Debug($"[EmergencyChannel] Suppressed (already reported): {key}"); - return; - } + if (_reportedKeys.Contains(key)) + { + _logger?.Debug($"[EmergencyChannel] Suppressed (already reported): {key}"); + return; + } - if (_lastReportTime != null && - (DateTime.UtcNow - _lastReportTime.Value).TotalMinutes < MinIntervalMinutes) - { - _logger?.Debug($"[EmergencyChannel] Suppressed (cooldown active, last report at {_lastReportTime.Value:HH:mm:ss}): {key}"); - return; - } + if (_lastReportTime != null && + (DateTime.UtcNow - _lastReportTime.Value).TotalMinutes < MinIntervalMinutes) + { + _logger?.Debug($"[EmergencyChannel] Suppressed (cooldown active, last report at {_lastReportTime.Value:HH:mm:ss}): {key}"); + return; + } - _reportedKeys.Add(key); - _reportCount++; - _lastReportTime = DateTime.UtcNow; + _reportedKeys.Add(key); + _reportCount++; + _lastReportTime = DateTime.UtcNow; + currentReportCount = _reportCount; + } var statusText = httpStatusCode.HasValue ? $" HTTP {httpStatusCode}" : string.Empty; - _logger?.Warning($"[EmergencyChannel] Sending report {_reportCount}/{MaxReportsPerSession}: {errorType}{statusText}"); + _logger?.Warning($"[EmergencyChannel] Sending report {currentReportCount}/{MaxReportsPerSession}: {errorType}{statusText}"); var report = new AgentErrorReport { @@ -105,7 +113,15 @@ namespace AutopilotMonitor.Agent.Core.Monitoring.Network Timestamp = DateTime.UtcNow }; - await _apiClient.ReportAgentErrorAsync(report); + try + { + await _apiClient.ReportAgentErrorAsync(report); + } + catch (Exception ex) + { + // Swallow — emergency channel must never cascade failures into the caller + _logger?.Debug($"[EmergencyChannel] Failed to send report: {ex.Message}"); + } } } } diff --git a/src/Agent/AutopilotMonitor.Agent.Core/Monitoring/Network/GeoLocationService.cs b/src/Agent/AutopilotMonitor.Agent.Core/Monitoring/Network/GeoLocationService.cs index b7fc691b..4a0bf824 100644 --- a/src/Agent/AutopilotMonitor.Agent.Core/Monitoring/Network/GeoLocationService.cs +++ b/src/Agent/AutopilotMonitor.Agent.Core/Monitoring/Network/GeoLocationService.cs @@ -44,6 +44,17 @@ namespace AutopilotMonitor.Agent.Core.Monitoring.Network private static readonly TimeSpan RequestTimeout = TimeSpan.FromSeconds(5); private static readonly TimeSpan RetryDelay = TimeSpan.FromSeconds(2); + // Static HttpClient to avoid socket exhaustion from repeated short-lived instances. + // HttpClient is designed to be reused across requests. + private static readonly HttpClient SharedHttpClient = CreateSharedHttpClient(); + + private static HttpClient CreateSharedHttpClient() + { + var client = new HttpClient { Timeout = RequestTimeout }; + client.DefaultRequestHeaders.Add("Accept", "application/json"); + return client; + } + public static async Task GetLocationAsync(AgentLogger logger) { var attempt = new GeoLocationAttemptResult(); @@ -88,10 +99,8 @@ namespace AutopilotMonitor.Agent.Core.Monitoring.Network { logger?.Info("GeoLocation: Querying ipinfo.io..."); - using (var client = new HttpClient { Timeout = RequestTimeout }) + using (var httpResponse = await SharedHttpClient.GetAsync("https://ipinfo.io/json")) { - client.DefaultRequestHeaders.Add("Accept", "application/json"); - var httpResponse = await client.GetAsync("https://ipinfo.io/json"); if (!httpResponse.IsSuccessStatusCode) { var error = $"HTTP {(int)httpResponse.StatusCode} ({httpResponse.ReasonPhrase})"; @@ -136,10 +145,8 @@ namespace AutopilotMonitor.Agent.Core.Monitoring.Network { logger?.Info("GeoLocation: Falling back to ifconfig.co..."); - using (var client = new HttpClient { Timeout = RequestTimeout }) + using (var httpResponse = await SharedHttpClient.GetAsync("https://ifconfig.co/json")) { - client.DefaultRequestHeaders.Add("Accept", "application/json"); - var httpResponse = await client.GetAsync("https://ifconfig.co/json"); if (!httpResponse.IsSuccessStatusCode) { var error = $"HTTP {(int)httpResponse.StatusCode} ({httpResponse.ReasonPhrase})"; diff --git a/src/Agent/AutopilotMonitor.Agent.Core/Monitoring/Network/NtpTimeCheckService.cs b/src/Agent/AutopilotMonitor.Agent.Core/Monitoring/Network/NtpTimeCheckService.cs index 1e1c6f9e..5f4da254 100644 --- a/src/Agent/AutopilotMonitor.Agent.Core/Monitoring/Network/NtpTimeCheckService.cs +++ b/src/Agent/AutopilotMonitor.Agent.Core/Monitoring/Network/NtpTimeCheckService.cs @@ -47,9 +47,27 @@ namespace AutopilotMonitor.Agent.Core.Monitoring.Network using (var socket = new Socket(addresses[0].AddressFamily, SocketType.Dgram, ProtocolType.Udp)) { socket.ReceiveTimeout = ReceiveTimeoutMs; + socket.SendTimeout = ReceiveTimeoutMs; socket.Connect(ipEndPoint); socket.Send(ntpData); - socket.Receive(ntpData); + var received = socket.Receive(ntpData); + + // Validate NTP response: must be at least 48 bytes + if (received < 48) + { + result.Error = $"Invalid NTP response: received {received} bytes (expected 48)"; + logger.Warning($"NTP time check failed: {result.Error}"); + return result; + } + + // Check Leap Indicator (bits 6-7 of first byte): value 3 means unsynchronized + var leapIndicator = (ntpData[0] >> 6) & 3; + if (leapIndicator == 3) + { + result.Error = "NTP server is unsynchronized (Leap Indicator = 3)"; + logger.Warning($"NTP time check failed: {result.Error}"); + return result; + } } // Parse transmit timestamp from bytes 40-47 (NTP epoch: 1900-01-01) diff --git a/src/Agent/AutopilotMonitor.Agent.Core/Monitoring/Tracking/EnrollmentTracker.cs b/src/Agent/AutopilotMonitor.Agent.Core/Monitoring/Tracking/EnrollmentTracker.cs index 060f7a5c..d75c6483 100644 --- a/src/Agent/AutopilotMonitor.Agent.Core/Monitoring/Tracking/EnrollmentTracker.cs +++ b/src/Agent/AutopilotMonitor.Agent.Core/Monitoring/Tracking/EnrollmentTracker.cs @@ -270,7 +270,17 @@ namespace AutopilotMonitor.Agent.Core.Monitoring.Tracking _imeLogTracker.Start(); // Collect and emit device info asynchronously (3 WMI queries + 5+ registry reads) - Task.Run(CollectDeviceInfo); + Task.Run(() => + { + try + { + CollectDeviceInfo(); + } + catch (Exception ex) + { + _logger.Warning($"EnrollmentTracker: background device info collection failed: {ex.Message}"); + } + }); // TODO: Überdenken ob ein 30s timer hier wirklich immer gut ist, hab einen Fall gesehen mit // laaanger Wartephase in WhiteGlove weil ein 24H2 feature update reinkam und installiert wurde @@ -288,9 +298,11 @@ namespace AutopilotMonitor.Agent.Core.Monitoring.Tracking public void Stop() { _logger.Info("EnrollmentTracker: stopping"); + _summaryTimer?.Change(Timeout.Infinite, Timeout.Infinite); _debugStateTimer?.Change(Timeout.Infinite, Timeout.Infinite); _imeLogTracker?.Stop(); + _logger.Info("EnrollmentTracker: stopped"); } diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Ingest/NdjsonParser.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Ingest/NdjsonParser.cs index 9d3bc1b3..8cd126c9 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Ingest/NdjsonParser.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Ingest/NdjsonParser.cs @@ -61,11 +61,19 @@ namespace AutopilotMonitor.Functions.Functions.Ingest var events = new List(); for (int i = 1; i < lines.Length; i++) { - var evt = JsonConvert.DeserializeObject(lines[i]); - if (evt != null) + try { - NormalizeEventData(evt); - events.Add(evt); + var evt = JsonConvert.DeserializeObject(lines[i]); + if (evt != null) + { + NormalizeEventData(evt); + events.Add(evt); + } + } + catch (JsonException) + { + // Skip malformed event lines rather than failing the entire batch. + // A single corrupted event should not cause data loss for valid events. } } diff --git a/src/Backend/AutopilotMonitor.Functions/Security/GraphTokenService.cs b/src/Backend/AutopilotMonitor.Functions/Security/GraphTokenService.cs index d484e03b..2afd3306 100644 --- a/src/Backend/AutopilotMonitor.Functions/Security/GraphTokenService.cs +++ b/src/Backend/AutopilotMonitor.Functions/Security/GraphTokenService.cs @@ -149,10 +149,13 @@ namespace AutopilotMonitor.Functions.Security var delay = retryDelays[attempt]; - // Respect Retry-After header from Azure AD / Graph throttling + // Respect Retry-After header from Azure AD / Graph throttling, but cap + // at 120s to prevent a misbehaving server from blocking indefinitely if (response.Headers.RetryAfter?.Delta is TimeSpan retryAfterDelta && retryAfterDelta > delay) { - delay = retryAfterDelta; + delay = retryAfterDelta > TimeSpan.FromSeconds(120) + ? TimeSpan.FromSeconds(120) + : retryAfterDelta; } _logger.LogInformation( diff --git a/src/McpServer/autopilot-monitor-mcp/src/client.ts b/src/McpServer/autopilot-monitor-mcp/src/client.ts index 4adc7523..6ffab257 100644 --- a/src/McpServer/autopilot-monitor-mcp/src/client.ts +++ b/src/McpServer/autopilot-monitor-mcp/src/client.ts @@ -1,9 +1,16 @@ const BASE_URL = process.env.AUTOPILOT_API_URL ?? 'https://autopilotmonitor-api.azurewebsites.net'; +/** Default timeout for backend API requests (30 seconds) */ +const API_TIMEOUT_MS = 30_000; + /** * Per-request token store. The MCP request handler sets the current user's * Bearer token before tool execution, and apiFetch reads it to pass through * to the backend API. + * + * NOTE: This is safe in the MCP server context because each MCP request is + * handled sequentially within a session — the token is set before tool + * execution and read during it, with no concurrent overlap. */ let _currentToken: string | undefined; @@ -28,7 +35,11 @@ async function apiFetch(path: string, options: RequestInit = {}): Promise) ?? {}), }; - const res = await fetch(url, { ...options, headers }); + + // Apply timeout to prevent hanging on unresponsive backend + const signal = options.signal ?? AbortSignal.timeout(API_TIMEOUT_MS); + + const res = await fetch(url, { ...options, headers, signal }); if (!res.ok) { const text = await res.text(); throw new Error(`API error ${res.status}: ${text}`); diff --git a/src/Web/autopilot-monitor-web/contexts/SignalRContext.tsx b/src/Web/autopilot-monitor-web/contexts/SignalRContext.tsx index ae01f96b..d438d62a 100644 --- a/src/Web/autopilot-monitor-web/contexts/SignalRContext.tsx +++ b/src/Web/autopilot-monitor-web/contexts/SignalRContext.tsx @@ -30,6 +30,7 @@ export function SignalRProvider({ children }: { children: React.ReactNode }) { const joinedGroupsRef = useRef>(new Set()); const [joinedGroups, setJoinedGroups] = useState([]); const retryCountRef = useRef(0); + const retryTimeoutRef = useRef | null>(null); const maxRetries = 3; const syncJoinedGroups = useCallback(() => { @@ -135,7 +136,7 @@ export function SignalRProvider({ children }: { children: React.ReactNode }) { if (retryCountRef.current < maxRetries) { retryCountRef.current++; const delay = Math.min(1000 * Math.pow(2, retryCountRef.current), 30000); - setTimeout(startConnection, delay); + retryTimeoutRef.current = setTimeout(startConnection, delay); } else { console.error('[SignalR] Max retries reached. Connection failed.'); trackEvent("signalr_connection_failed"); @@ -147,6 +148,11 @@ export function SignalRProvider({ children }: { children: React.ReactNode }) { // Cleanup only when provider unmounts (app closes) return () => { + // Clear any pending retry timeout to prevent reconnection after unmount + if (retryTimeoutRef.current) { + clearTimeout(retryTimeoutRef.current); + retryTimeoutRef.current = null; + } if (connectionRef.current) { connectionRef.current.stop(); connectionRef.current = null; diff --git a/src/Web/autopilot-monitor-web/lib/authenticatedFetch.ts b/src/Web/autopilot-monitor-web/lib/authenticatedFetch.ts index 944dfcac..449c226c 100644 --- a/src/Web/autopilot-monitor-web/lib/authenticatedFetch.ts +++ b/src/Web/autopilot-monitor-web/lib/authenticatedFetch.ts @@ -5,8 +5,15 @@ * request once. If the retry also fails with 401, a `TokenExpiredError` is * thrown so callers can show a clear "session expired" message instead of a * generic backend error. + * + * All requests are subject to a default timeout (30s) to prevent hanging + * indefinitely on unresponsive backends. Callers can override the timeout + * by passing their own AbortSignal via init.signal. */ +/** Default request timeout in milliseconds */ +const DEFAULT_TIMEOUT_MS = 30_000; + export class TokenExpiredError extends Error { constructor() { super('Your session has expired. Please reload the page to sign in again.'); @@ -29,7 +36,10 @@ export async function authenticatedFetch( const headers = new Headers(init?.headers); headers.set('Authorization', `Bearer ${token}`); - const response = await fetch(url, { ...init, headers }); + // Apply a default timeout unless the caller already provided an AbortSignal + const signal = init?.signal ?? AbortSignal.timeout(DEFAULT_TIMEOUT_MS); + + const response = await fetch(url, { ...init, headers, signal }); if (response.status === 401) { // Force MSAL to bypass its cache and obtain a fresh token. @@ -41,7 +51,8 @@ export async function authenticatedFetch( const retryHeaders = new Headers(init?.headers); retryHeaders.set('Authorization', `Bearer ${freshToken}`); - const retryResponse = await fetch(url, { ...init, headers: retryHeaders }); + const retrySignal = init?.signal ?? AbortSignal.timeout(DEFAULT_TIMEOUT_MS); + const retryResponse = await fetch(url, { ...init, headers: retryHeaders, signal: retrySignal }); if (retryResponse.status === 401) { throw new TokenExpiredError(); }