Skip to content

Commit f681388

Browse files
committed
Improve agent/config validation and UI robustness
Validate and tighten runtime and agent handling, fix event listener stability, and improve UX/error handling. Key changes: - server/config: validate DASHBOARD_DENSITY against allowed values (compact/comfortable) and default to comfortable. - runtime-config: remove configuredAgentUrl from runtime shape to avoid exposing internal URL. - types/agent & agent-config-manager: make agent.token optional (undefined by default) and remove required-token validation to support env-managed agents. - AgentFormModal: ensure token field is initialized to an empty string when missing. - DashboardSidebar: render disabled sidebar items as <button> with aria-disabled and preventDefault for better accessibility. - map component: use refs and stable callback handlers for locationfound/locationerror to ensure proper add/remove of listeners and avoid stale closures. - place-autocomplete: keep onResultsChange in a ref and call it when results clear or on errors; remove it from effect deps to avoid unnecessary fetches. - useLogFetcher: memoize returned state and reset function, useCallback for resetAndLoadRecent, and rely on clearLogsFromIDB to handle its own errors. - AgentContext: surface a toast on agent refresh failure and handle AbortError specially when checking agent status. - LogContext: remove redundant useMemo and return logFetcher state directly. These changes improve stability, avoid exposing internal configuration, and make UI interactions and async flows more resilient.
1 parent dd67676 commit f681388

11 files changed

Lines changed: 53 additions & 39 deletions

File tree

dashboard/server/config.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,9 @@ function buildRuntimeConfig(): Record<string, unknown> {
3939
const trafficTopItemsRaw = pick('DASHBOARD_TRAFFIC_TOP_ITEMS_LIMIT');
4040
const parserTrendWindowRaw = pick('DASHBOARD_PARSER_TREND_WINDOW_MINUTES');
4141
const agentsEnvOnlyRaw = pick('DASHBOARD_AGENTS_ENV_ONLY');
42-
const density = pick('DASHBOARD_DENSITY', 'UI_DENSITY') || 'comfortable';
42+
const VALID_DENSITIES = new Set(['compact', 'comfortable']);
43+
const rawDensity = pick('DASHBOARD_DENSITY', 'UI_DENSITY');
44+
const density = VALID_DENSITIES.has(rawDensity) ? rawDensity : 'comfortable';
4345
const agentUrl = pick('AGENT_API_URL', 'AGENT_URL');
4446
const agentToken = pick('AGENT_API_TOKEN', 'AGENT_TOKEN');
4547
const frontendAgentUrl = pick('DASHBOARD_DEFAULT_AGENT_URL');
@@ -80,7 +82,6 @@ function buildRuntimeConfig(): Record<string, unknown> {
8082
themeTokens: {},
8183
defaultAgentUrl: frontendAgentUrl,
8284
defaultAgentConfigured: !!agentToken,
83-
configuredAgentUrl: agentUrl,
8485
};
8586
}
8687

dashboard/src/components/AgentFormModal.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ export default function AgentFormModal({ isOpen, onClose, agent }: AgentFormModa
3131
setFormData({
3232
name: agent.name,
3333
url: agent.configuredUrl || agent.url,
34-
token: agent.token,
34+
token: agent.token ?? '',
3535
location: agent.location,
3636
description: agent.description || '',
3737
tags: agent.tags?.join(', ') || '',

dashboard/src/components/layout/DashboardSidebar.tsx

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -195,12 +195,15 @@ export function DashboardSidebar() {
195195
<item.icon className="size-4" />
196196
</a>
197197
) : (
198-
<span
198+
<button
199+
type="button"
200+
aria-disabled="true"
199201
aria-label={`${item.label} coming soon`}
200202
className="inline-flex cursor-not-allowed items-center justify-center rounded-md p-1.5 text-sidebar-foreground/30"
203+
onClick={(e) => e.preventDefault()}
201204
>
202205
<item.icon className="size-4" />
203-
</span>
206+
</button>
204207
)}
205208
</TooltipTrigger>
206209
<TooltipContent side="top">{tooltipLabel}</TooltipContent>

dashboard/src/components/ui/map.tsx

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -754,28 +754,37 @@ function MapLocateControl({
754754
const [isLocating, setIsLocating] = useDebounceLoadingState(200)
755755
const [position, setPosition] = useState<LatLngExpression | null>(null)
756756

757+
const onLocationFoundRef = useRef(onLocationFound)
758+
onLocationFoundRef.current = onLocationFound
759+
const onLocationErrorRef = useRef(onLocationError)
760+
onLocationErrorRef.current = onLocationError
761+
762+
const onLocationFoundHandler = useCallback((location: LocationEvent) => {
763+
setPosition(location.latlng)
764+
setIsLocating(false)
765+
onLocationFoundRef.current?.(location)
766+
}, []) // eslint-disable-line react-hooks/exhaustive-deps -- reads from ref
767+
768+
const onLocationErrorHandler = useCallback((error: ErrorEvent) => {
769+
setPosition(null)
770+
setIsLocating(false)
771+
onLocationErrorRef.current?.(error)
772+
}, []) // eslint-disable-line react-hooks/exhaustive-deps -- reads from ref
773+
757774
function startLocating() {
758775
setIsLocating(true)
759776
map.locate({ setView: true, maxZoom: map.getMaxZoom(), watch })
760-
map.on("locationfound", (location: LocationEvent) => {
761-
setPosition(location.latlng)
762-
setIsLocating(false)
763-
onLocationFound?.(location)
764-
})
765-
map.on("locationerror", (error: ErrorEvent) => {
766-
setPosition(null)
767-
setIsLocating(false)
768-
onLocationError?.(error)
769-
})
777+
map.on("locationfound", onLocationFoundHandler)
778+
map.on("locationerror", onLocationErrorHandler)
770779
}
771780

772781
const stopLocating = useCallback(() => {
773782
map.stopLocate()
774-
map.off("locationfound")
775-
map.off("locationerror")
783+
map.off("locationfound", onLocationFoundHandler)
784+
map.off("locationerror", onLocationErrorHandler)
776785
setPosition(null)
777786
setIsLocating(false)
778-
}, [map]) // eslint-disable-line react-hooks/exhaustive-deps -- setIsLocating is stable setState
787+
}, [map, onLocationFoundHandler, onLocationErrorHandler])
779788

780789
// eslint-disable-next-line no-restricted-syntax -- cleanup on unmount
781790
useEffect(() => () => stopLocating(), [stopLocating])

dashboard/src/components/ui/place-autocomplete.tsx

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -188,10 +188,15 @@ function usePlaceSearch({
188188

189189
const debouncedQuery = useDebounce(query, debounceMs)
190190

191+
const onResultsChangeRef = React.useRef(onResultsChange)
192+
// eslint-disable-next-line no-restricted-syntax -- keep ref current without re-triggering fetch effect
193+
React.useEffect(() => { onResultsChangeRef.current = onResultsChange }, [onResultsChange])
194+
191195
// eslint-disable-next-line no-restricted-syntax -- search on debounced query change
192196
React.useEffect(() => {
193197
if (!debouncedQuery.trim()) {
194198
setResults([])
199+
onResultsChangeRef.current?.([])
195200
setIsLoading(false)
196201
setHasSearched(false)
197202
return
@@ -225,11 +230,12 @@ function usePlaceSearch({
225230
return true
226231
})
227232
setResults(dedupedFeatures)
228-
onResultsChange?.(dedupedFeatures)
233+
onResultsChangeRef.current?.(dedupedFeatures)
229234
} catch (err) {
230235
if (err instanceof Error && err.name !== "AbortError") {
231236
setError(err)
232237
setResults([])
238+
onResultsChangeRef.current?.([])
233239
}
234240
} finally {
235241
setIsLoading(false)
@@ -248,7 +254,6 @@ function usePlaceSearch({
248254
lon,
249255
zoom,
250256
locationBiasScale,
251-
onResultsChange,
252257
])
253258

254259
return { results, isLoading, error, hasSearched }

dashboard/src/hooks/useLogFetcher.ts

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useState, useEffect, useRef, useSyncExternalStore } from 'react'; // eslint-disable-line no-restricted-syntax
1+
import { useState, useEffect, useRef, useSyncExternalStore, useCallback, useMemo } from 'react'; // eslint-disable-line no-restricted-syntax
22
import { TraefikLog } from '@/utils/types';
33
import { enrichLogsWithGeoLocation } from '@/utils/location';
44
import { apiClient } from '@/utils/api-client';
@@ -363,18 +363,17 @@ export function useLogFetcher() {
363363
logStore.trimLogs(maxLogsDisplay);
364364
}, [maxLogsDisplay]);
365365

366-
const resetAndLoadRecent = () => {
366+
const resetAndLoadRecent = useCallback(() => {
367367
if (selectedAgent?.id) {
368-
clearLogsFromIDB(selectedAgent.id).catch((err) => {
369-
console.warn('[useLogFetcher] Failed to clear IndexedDB logs:', err);
370-
});
368+
// clearLogsFromIDB handles its own errors internally
369+
clearLogsFromIDB(selectedAgent.id);
371370
logStore.clearPosition(selectedAgent.id);
372371
logStore.clearLogs();
373372
logStore.requestReset();
374373
}
375-
};
374+
}, [selectedAgent?.id]);
376375

377-
return {
376+
return useMemo(() => ({
378377
logs: storeState.logs,
379378
loading: storeState.loading,
380379
error: storeState.error,
@@ -388,5 +387,5 @@ export function useLogFetcher() {
388387
isCatchingUp: storeState.isCatchingUp,
389388
isCached: storeState.isCached,
390389
resetAndLoadRecent,
391-
};
390+
}), [storeState.logs, storeState.loading, storeState.error, storeState.connected, storeState.lastUpdate, isPaused, setIsPaused, storeState.agentId, storeState.agentName, dedupeDebug, storeState.isCatchingUp, storeState.isCached, resetAndLoadRecent]);
392391
}

dashboard/src/utils/agent-config-manager.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ export class AgentConfigManager {
4646
id: 'agent-001',
4747
name: 'Default Agent',
4848
url: runtime.defaultAgentUrl || sameOrigin,
49-
token: '',
49+
token: undefined,
5050
location: 'on-site',
5151
number: 1,
5252
status: 'checking',
@@ -199,10 +199,6 @@ export class AgentConfigManager {
199199
}
200200
}
201201

202-
if (!agent.token?.trim()) {
203-
errors.push('Authentication token is required');
204-
}
205-
206202
if (!agent.location || !['on-site', 'off-site'].includes(agent.location)) {
207203
errors.push('Location must be either "on-site" or "off-site"');
208204
}

dashboard/src/utils/config/runtime-config.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ export interface RuntimeConfig {
1515
themeTokens?: Record<string, string>;
1616
defaultAgentUrl?: string;
1717
defaultAgentConfigured?: boolean;
18-
configuredAgentUrl?: string;
1918
}
2019

2120
const buildTimeFallback: RuntimeConfig = {

dashboard/src/utils/contexts/AgentContext.tsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ export function AgentProvider({ children }: { children: React.ReactNode }) {
3939
setSelectedAgent(agentStore.getSelectedAgent());
4040
}).catch((err) => {
4141
console.error('[AgentContext] Failed to refresh agents:', err);
42+
toast.error('Failed to refresh agents');
4243
});
4344
}, []);
4445

@@ -124,6 +125,9 @@ export function AgentProvider({ children }: { children: React.ReactNode }) {
124125

125126
return isOnline;
126127
} catch (error) {
128+
if (error instanceof Error && error.name === 'AbortError') {
129+
return false;
130+
}
127131
console.error(`Agent ${id} status check failed:`, error);
128132

129133
agentStore.updateAgent(id, { status: 'offline' });

dashboard/src/utils/contexts/LogContext.tsx

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import React, { createContext, useContext, useMemo } from 'react';
1+
import React, { createContext, useContext } from 'react';
22
import { TraefikLog } from '@/utils/types';
33
import { useLogFetcher, DedupeDebugStats } from '@/hooks/useLogFetcher';
44

@@ -31,9 +31,7 @@ const LogContext = createContext<LogContextType | undefined>(undefined);
3131
export function LogProvider({ children }: { children: React.ReactNode }) {
3232
const logFetcherState = useLogFetcher();
3333

34-
const value = useMemo(() => logFetcherState, [logFetcherState]);
35-
36-
return <LogContext.Provider value={value}>{children}</LogContext.Provider>;
34+
return <LogContext.Provider value={logFetcherState}>{children}</LogContext.Provider>;
3735
}
3836

3937
export function useLogContext(): LogContextType {

0 commit comments

Comments
 (0)