Fix critical bug: prevent reloads from stale updateLogsData.isComplete

- Add isUpdating guard before processing updateLogsData.isComplete
- Reset shouldSubscribe when update completes or fails
- Prevent stale isComplete data from triggering reloads during normal usage
This commit is contained in:
Michel Roegl-Brunner
2025-11-26 10:17:57 +01:00
parent 65bed15722
commit 53df7201d6

View File

@@ -4,6 +4,7 @@ import { api } from "~/trpc/react";
import { Badge } from "./ui/badge"; import { Badge } from "./ui/badge";
import { Button } from "./ui/button"; import { Button } from "./ui/button";
import { ContextualHelpIcon } from "./ContextualHelpIcon"; import { ContextualHelpIcon } from "./ContextualHelpIcon";
import { UpdateConfirmationModal } from "./UpdateConfirmationModal";
import { ExternalLink, Download, RefreshCw, Loader2 } from "lucide-react"; import { ExternalLink, Download, RefreshCw, Loader2 } from "lucide-react";
import { useState, useEffect, useRef, useCallback } from "react"; import { useState, useEffect, useRef, useCallback } from "react";
@@ -85,6 +86,7 @@ export function VersionDisplay({ onOpenReleaseNotes }: VersionDisplayProps = {})
const [updateLogs, setUpdateLogs] = useState<string[]>([]); const [updateLogs, setUpdateLogs] = useState<string[]>([]);
const [shouldSubscribe, setShouldSubscribe] = useState(false); const [shouldSubscribe, setShouldSubscribe] = useState(false);
const [updateStartTime, setUpdateStartTime] = useState<number | null>(null); const [updateStartTime, setUpdateStartTime] = useState<number | null>(null);
const [showUpdateConfirmation, setShowUpdateConfirmation] = useState(false);
const lastLogTimeRef = useRef<number>(Date.now()); const lastLogTimeRef = useRef<number>(Date.now());
const reconnectIntervalRef = useRef<NodeJS.Timeout | null>(null); const reconnectIntervalRef = useRef<NodeJS.Timeout | null>(null);
const hasReloadedRef = useRef<boolean>(false); const hasReloadedRef = useRef<boolean>(false);
@@ -101,11 +103,13 @@ export function VersionDisplay({ onOpenReleaseNotes }: VersionDisplayProps = {})
setUpdateLogs(['Update started...']); setUpdateLogs(['Update started...']);
} else { } else {
setIsUpdating(false); setIsUpdating(false);
setShouldSubscribe(false); // Reset subscription on failure
} }
}, },
onError: (error) => { onError: (error) => {
setUpdateResult({ success: false, message: error.message }); setUpdateResult({ success: false, message: error.message });
setIsUpdating(false); setIsUpdating(false);
setShouldSubscribe(false); // Reset subscription on error
} }
}); });
@@ -120,8 +124,9 @@ export function VersionDisplay({ onOpenReleaseNotes }: VersionDisplayProps = {})
// Memoized with useCallback to prevent recreation on every render // Memoized with useCallback to prevent recreation on every render
// Only depends on refs to avoid stale closures // Only depends on refs to avoid stale closures
const startReconnectAttempts = useCallback(() => { const startReconnectAttempts = useCallback(() => {
// Stricter guard: check refs BEFORE starting reconnect attempts // CRITICAL: Stricter guard - check refs BEFORE starting reconnect attempts
// Only start if we're actually updating and haven't already started // Only start if we're actually updating and haven't already started
// Double-check isUpdating state to prevent false triggers from stale data
if (reconnectIntervalRef.current || !isUpdatingRef.current || hasReloadedRef.current) { if (reconnectIntervalRef.current || !isUpdatingRef.current || hasReloadedRef.current) {
return; return;
} }
@@ -173,18 +178,26 @@ export function VersionDisplay({ onOpenReleaseNotes }: VersionDisplayProps = {})
// Update logs when data changes // Update logs when data changes
useEffect(() => { useEffect(() => {
// CRITICAL: Only process update logs if we're actually updating
// This prevents stale isComplete data from triggering reloads when not updating
if (!isUpdating) {
return;
}
if (updateLogsData?.success && updateLogsData.logs) { if (updateLogsData?.success && updateLogsData.logs) {
lastLogTimeRef.current = Date.now(); lastLogTimeRef.current = Date.now();
setUpdateLogs(updateLogsData.logs); setUpdateLogs(updateLogsData.logs);
if (updateLogsData.isComplete) { // CRITICAL: Only process isComplete if we're actually updating
// Double-check isUpdating state to prevent false triggers
if (updateLogsData.isComplete && isUpdating) {
setUpdateLogs(prev => [...prev, 'Update complete! Server restarting...']); setUpdateLogs(prev => [...prev, 'Update complete! Server restarting...']);
setIsNetworkError(true); setIsNetworkError(true);
// Start reconnection attempts when we know update is complete // Start reconnection attempts when we know update is complete
startReconnectAttempts(); startReconnectAttempts();
} }
} }
}, [updateLogsData, startReconnectAttempts]); }, [updateLogsData, startReconnectAttempts, isUpdating]);
// Monitor for server connection loss and auto-reload (fallback only) // Monitor for server connection loss and auto-reload (fallback only)
useEffect(() => { useEffect(() => {
@@ -229,11 +242,15 @@ export function VersionDisplay({ onOpenReleaseNotes }: VersionDisplayProps = {})
// Clear reconnect interval when update completes or component unmounts // Clear reconnect interval when update completes or component unmounts
useEffect(() => { useEffect(() => {
// If we're no longer updating, clear the reconnect interval // If we're no longer updating, clear the reconnect interval and reset subscription
if (!isUpdating && reconnectIntervalRef.current) { if (!isUpdating) {
if (reconnectIntervalRef.current) {
clearInterval(reconnectIntervalRef.current); clearInterval(reconnectIntervalRef.current);
reconnectIntervalRef.current = null; reconnectIntervalRef.current = null;
} }
// Reset subscription to prevent stale polling
setShouldSubscribe(false);
}
return () => { return () => {
if (reconnectIntervalRef.current) { if (reconnectIntervalRef.current) {
@@ -244,6 +261,14 @@ export function VersionDisplay({ onOpenReleaseNotes }: VersionDisplayProps = {})
}, [isUpdating]); }, [isUpdating]);
const handleUpdate = () => { const handleUpdate = () => {
// Show confirmation modal instead of starting update directly
setShowUpdateConfirmation(true);
};
const handleConfirmUpdate = () => {
// Close the confirmation modal
setShowUpdateConfirmation(false);
// Start the actual update process
setIsUpdating(true); setIsUpdating(true);
setUpdateResult(null); setUpdateResult(null);
setIsNetworkError(false); setIsNetworkError(false);
@@ -290,6 +315,18 @@ export function VersionDisplay({ onOpenReleaseNotes }: VersionDisplayProps = {})
{/* Loading overlay */} {/* Loading overlay */}
{isUpdating && <LoadingOverlay isNetworkError={isNetworkError} logs={updateLogs} />} {isUpdating && <LoadingOverlay isNetworkError={isNetworkError} logs={updateLogs} />}
{/* Update Confirmation Modal */}
{versionStatus?.releaseInfo && (
<UpdateConfirmationModal
isOpen={showUpdateConfirmation}
onClose={() => setShowUpdateConfirmation(false)}
onConfirm={handleConfirmUpdate}
releaseInfo={versionStatus.releaseInfo}
currentVersion={versionStatus.currentVersion}
latestVersion={versionStatus.latestVersion}
/>
)}
<div className="flex flex-col sm:flex-row items-center gap-2 sm:gap-2"> <div className="flex flex-col sm:flex-row items-center gap-2 sm:gap-2">
<Badge <Badge
variant={isUpToDate ? "default" : "secondary"} variant={isUpToDate ? "default" : "secondary"}