refactor: Consolidate validation and improve error logging

This commit is contained in:
Shirone
2026-01-21 22:28:22 +01:00
parent a9616ff309
commit 28d50aa017
3 changed files with 62 additions and 72 deletions

View File

@@ -69,28 +69,13 @@ export function createAddRemoteHandler() {
remoteUrl: string; remoteUrl: string;
}; };
if (!worktreePath) { // Validate required fields
res.status(400).json({ const requiredFields = { worktreePath, remoteName, remoteUrl };
success: false, for (const [key, value] of Object.entries(requiredFields)) {
error: 'worktreePath required', if (!value) {
}); res.status(400).json({ success: false, error: `${key} required` });
return; return;
} }
if (!remoteName) {
res.status(400).json({
success: false,
error: 'remoteName required',
});
return;
}
if (!remoteUrl) {
res.status(400).json({
success: false,
error: 'remoteUrl required',
});
return;
} }
// Validate remote name // Validate remote name
@@ -129,8 +114,13 @@ export function createAddRemoteHandler() {
}); });
return; return;
} }
} catch { } catch (error) {
// If git remote fails, continue with adding the remote // If git remote fails, continue with adding the remote. Log for debugging.
logWorktreeError(
error,
'Checking for existing remotes failed, proceeding to add.',
worktreePath
);
} }
// Add the remote using execFile with array arguments to prevent command injection // Add the remote using execFile with array arguments to prevent command injection
@@ -146,8 +136,13 @@ export function createAddRemoteHandler() {
timeout: FETCH_TIMEOUT_MS, timeout: FETCH_TIMEOUT_MS,
}); });
fetchSucceeded = true; fetchSucceeded = true;
} catch { } catch (fetchError) {
// Fetch failed (maybe offline or invalid URL), but remote was added successfully // Fetch failed (maybe offline or invalid URL), but remote was added successfully
logWorktreeError(
fetchError,
`Fetch from new remote '${remoteName}' failed (remote added successfully)`,
worktreePath
);
fetchSucceeded = false; fetchSucceeded = false;
} }

View File

@@ -19,6 +19,7 @@ import {
SelectValue, SelectValue,
} from '@/components/ui/select'; } from '@/components/ui/select';
import { getHttpApiClient } from '@/lib/http-api-client'; import { getHttpApiClient } from '@/lib/http-api-client';
import { getErrorMessage } from '@/lib/utils';
import { toast } from 'sonner'; import { toast } from 'sonner';
import { Upload, RefreshCw, AlertTriangle, Sparkles, Plus, Link } from 'lucide-react'; import { Upload, RefreshCw, AlertTriangle, Sparkles, Plus, Link } from 'lucide-react';
import { Spinner } from '@/components/ui/spinner'; import { Spinner } from '@/components/ui/spinner';
@@ -31,16 +32,6 @@ interface RemoteInfo {
const logger = createLogger('PushToRemoteDialog'); const logger = createLogger('PushToRemoteDialog');
/**
* Extracts error message from unknown error type
*/
function getErrorMessage(error: unknown): string {
if (error instanceof Error) {
return error.message;
}
return String(error);
}
interface PushToRemoteDialogProps { interface PushToRemoteDialogProps {
open: boolean; open: boolean;
onOpenChange: (open: boolean) => void; onOpenChange: (open: boolean) => void;
@@ -67,41 +58,6 @@ export function PushToRemoteDialog({
const [isAddingRemote, setIsAddingRemote] = useState(false); const [isAddingRemote, setIsAddingRemote] = useState(false);
const [addRemoteError, setAddRemoteError] = useState<string | null>(null); const [addRemoteError, setAddRemoteError] = useState<string | null>(null);
// Fetch remotes when dialog opens
useEffect(() => {
if (open && worktree) {
fetchRemotes();
}
}, [open, worktree]);
// Reset state when dialog closes
useEffect(() => {
if (!open) {
setSelectedRemote('');
setError(null);
setShowAddRemoteForm(false);
setNewRemoteName('origin');
setNewRemoteUrl('');
setAddRemoteError(null);
}
}, [open]);
// Auto-select default remote when remotes are loaded
useEffect(() => {
if (remotes.length > 0 && !selectedRemote) {
// Default to 'origin' if available, otherwise first remote
const defaultRemote = remotes.find((r) => r.name === 'origin') || remotes[0];
setSelectedRemote(defaultRemote.name);
}
}, [remotes, selectedRemote]);
// Show add remote form when no remotes
useEffect(() => {
if (!isLoading && remotes.length === 0) {
setShowAddRemoteForm(true);
}
}, [isLoading, remotes.length]);
/** /**
* Transforms API remote data to RemoteInfo format * Transforms API remote data to RemoteInfo format
*/ */
@@ -125,7 +81,7 @@ export function PushToRemoteDialog({
} }
}, []); }, []);
const fetchRemotes = async () => { const fetchRemotes = useCallback(async () => {
if (!worktree) return; if (!worktree) return;
setIsLoading(true); setIsLoading(true);
@@ -147,7 +103,42 @@ export function PushToRemoteDialog({
} finally { } finally {
setIsLoading(false); setIsLoading(false);
} }
}; }, [worktree, transformRemoteData, updateRemotesState]);
// Fetch remotes when dialog opens
useEffect(() => {
if (open && worktree) {
fetchRemotes();
}
}, [open, worktree, fetchRemotes]);
// Reset state when dialog closes
useEffect(() => {
if (!open) {
setSelectedRemote('');
setError(null);
setShowAddRemoteForm(false);
setNewRemoteName('origin');
setNewRemoteUrl('');
setAddRemoteError(null);
}
}, [open]);
// Auto-select default remote when remotes are loaded
useEffect(() => {
if (remotes.length > 0 && !selectedRemote) {
// Default to 'origin' if available, otherwise first remote
const defaultRemote = remotes.find((r) => r.name === 'origin') || remotes[0];
setSelectedRemote(defaultRemote.name);
}
}, [remotes, selectedRemote]);
// Show add remote form when no remotes (but not when there's an error)
useEffect(() => {
if (!isLoading && remotes.length === 0 && !error) {
setShowAddRemoteForm(true);
}
}, [isLoading, remotes.length, error]);
const handleRefresh = async () => { const handleRefresh = async () => {
if (!worktree) return; if (!worktree) return;

View File

@@ -7,6 +7,10 @@ export function cn(...inputs: ClassValue[]) {
return twMerge(clsx(inputs)); return twMerge(clsx(inputs));
} }
// Re-export getErrorMessage from @automaker/utils to maintain backward compatibility
// for components that already import it from here
export { getErrorMessage } from '@automaker/utils';
/** /**
* Determine if the current model supports extended thinking controls * Determine if the current model supports extended thinking controls
* Note: This is for Claude's "thinking levels" only, not Codex's "reasoning effort" * Note: This is for Claude's "thinking levels" only, not Codex's "reasoning effort"