From dd7127147339f7da33c3dfa07b2bdcf19c61d524 Mon Sep 17 00:00:00 2001 From: 2GT-Rich <107219997+2GT-Rich@users.noreply.github.com> Date: Tue, 1 Sep 2026 13:14:26 -0700 Subject: [PATCH] fix(poller): the health panel must not blank the page, and pass the tunables through (v0.23.20-beta) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two faults reported against v0.23.19, one of which makes the other much worse. **POLLER_CONCURRENCY was never passed into the container.** It is read from the environment by the backend and documented in .env.example, but docker-compose did not forward it, so setting it did nothing at all. The same was true of the job timeout, interval and both retention limits. Every poller tunable shipped in v0.23.17 was inert for anyone running the compose stack — including the one knob the #114 reply told the reporter to reach for. All five are now forwarded. **The health panel crashed on an unexpected payload**, taking the whole page with it: `cap is undefined` from reading data.capacity without a guard. It also rendered "Loading…" indefinitely on a request error, so a failing endpoint and a slow one looked identical. Both are the wrong failure mode for this screen in particular. It is the page an operator opens precisely when monitoring already looks broken, and a blank white panel at that moment is worse than no panel: it suggests the whole tool is dead rather than that one query failed. Every field is now defaulted, so a backend a version behind or one that answered partially degrades the panel instead of destroying the page, and a failed request renders what actually went wrong with a retry. Verified against the four shapes that could produce the reported crash: capacity missing, an error body arriving with a 2xx, an older backend without the status field, and an empty object. The endpoint is hardened to match. Device count, timing and the stale-device listing are each guarded independently, so a partial failure returns partial numbers rather than a 500 — the same reasoning applied at the other end. I could not reproduce the original crash: at concurrency 48 the endpoint returns a complete payload here, there is no route shadowing, and the axios interceptor rejects errors correctly rather than passing them through as data. The defensive work stands on its own regardless of which of those shapes he actually hit. Co-Authored-By: Claude Opus 5 --- README.md | 2 +- backend/package.json | 2 +- backend/src/routes/system.ts | 10 ++- backend/src/services/PollerService.ts | 31 ++++++--- docker-compose.yml | 7 ++ frontend/package.json | 2 +- .../components/system/PollerHealthCard.tsx | 64 +++++++++++++++---- frontend/src/version.ts | 2 +- 8 files changed, 95 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index bd2c849..79cdd05 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@

- Version + Version License Docker TypeScript diff --git a/backend/package.json b/backend/package.json index 81a29d0..b2b6100 100644 --- a/backend/package.json +++ b/backend/package.json @@ -1,6 +1,6 @@ { "name": "mikrotik-manager-backend", - "version": "0.23.19-beta", + "version": "0.23.20-beta", "description": "Mikrotik Network Management Platform - Backend API", "main": "dist/index.js", "scripts": { diff --git a/backend/src/routes/system.ts b/backend/src/routes/system.ts index fd2c84e..e1440f5 100644 --- a/backend/src/routes/system.ts +++ b/backend/src/routes/system.ts @@ -68,7 +68,10 @@ router.get('/poller', async (_req: Request, res: Response) => { // Devices the poller has not managed to reach lately, worst first. This is // the list that answers "which ones is it actually missing?". - const stale = await query( + // Stale-device listing is a nicety; never let it take the whole payload down. + let stale: unknown[] = []; + try { + stale = await query( `SELECT d.id, d.name, d.status, s.kind, s.last_attempt_at, s.last_success_at, s.last_duration_ms, s.last_error, s.attempts, s.failures, @@ -79,7 +82,10 @@ router.get('/poller', async (_req: Request, res: Response) => { AND (s.last_success_at IS NULL OR s.last_success_at < NOW() - INTERVAL '3 minutes') ORDER BY s.last_success_at ASC NULLS FIRST LIMIT 50` - ); + ); + } catch (e) { + console.error('Poller health: stale-device query failed:', (e as Error).message); + } res.json({ ...health, stale_devices: stale }); } catch (error) { diff --git a/backend/src/services/PollerService.ts b/backend/src/services/PollerService.ts index 9fb54dc..d01d3e8 100644 --- a/backend/src/services/PollerService.ts +++ b/backend/src/services/PollerService.ts @@ -721,14 +721,29 @@ export class PollerService { } } - const [{ count: deviceCount } = { count: 0 }] = await query<{ count: number }>( - `SELECT COUNT(*)::int AS count FROM devices WHERE status != 'disabled'` - ); - const [timing] = await query<{ avg_ms: number | null; p90_ms: number | null }>( - `SELECT AVG(last_duration_ms)::int AS avg_ms, - (PERCENTILE_CONT(0.9) WITHIN GROUP (ORDER BY last_duration_ms))::int AS p90_ms - FROM device_poll_stats WHERE kind = 'fast' AND last_duration_ms IS NOT NULL` - ); + // Each read is independently guarded. This endpoint is what an operator opens + // when monitoring already looks broken, so it must degrade to partial numbers + // rather than fail whole — a 500 here reads as "the tool is dead too". + let deviceCount = 0; + try { + const [row] = await query<{ count: number }>( + `SELECT COUNT(*)::int AS count FROM devices WHERE status != 'disabled'` + ); + deviceCount = row?.count ?? 0; + } catch (e) { + console.error('[Poller] Health: device count failed:', (e as Error).message); + } + + let timing: { avg_ms: number | null; p90_ms: number | null } | undefined; + try { + [timing] = await query<{ avg_ms: number | null; p90_ms: number | null }>( + `SELECT AVG(last_duration_ms)::int AS avg_ms, + (PERCENTILE_CONT(0.9) WITHIN GROUP (ORDER BY last_duration_ms))::int AS p90_ms + FROM device_poll_stats WHERE kind = 'fast' AND last_duration_ms IS NOT NULL` + ); + } catch (e) { + console.error('[Poller] Health: timing query failed:', (e as Error).message); + } // Can the workers keep up? Arrival is one fast poll per device per cycle. const avgSec = (timing?.avg_ms || 0) / 1000; diff --git a/docker-compose.yml b/docker-compose.yml index a99bea3..eaaf4bb 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -37,6 +37,13 @@ services: CORS_ORIGIN: ${CORS_ORIGIN:-https://localhost,http://localhost,https://127.0.0.1,http://127.0.0.1} PORT: 3001 NETFLOW_BIND_PORT: 2055 + # Poller tuning. These were documented in .env.example but never passed + # through, so setting them had no effect on the container at all. + POLLER_CONCURRENCY: ${POLLER_CONCURRENCY:-12} + POLLER_JOB_TIMEOUT_MS: ${POLLER_JOB_TIMEOUT_MS:-45000} + POLLER_INTERVAL_MS: ${POLLER_INTERVAL_MS:-30000} + POLLER_JOB_RETENTION_SEC: ${POLLER_JOB_RETENTION_SEC:-3600} + POLLER_JOB_RETENTION_COUNT: ${POLLER_JOB_RETENTION_COUNT:-5000} ports: # NetFlow/IPFIX collector — UDP goes straight to the backend (nginx not involved). # Host port must match the netflow_collector_port app setting (default 2055). diff --git a/frontend/package.json b/frontend/package.json index c79c270..8cb0c1f 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,6 +1,6 @@ { "name": "mikrotik-manager-frontend", - "version": "0.23.19-beta", + "version": "0.23.20-beta", "private": true, "type": "module", "scripts": { diff --git a/frontend/src/components/system/PollerHealthCard.tsx b/frontend/src/components/system/PollerHealthCard.tsx index 81fe532..e83dad8 100644 --- a/frontend/src/components/system/PollerHealthCard.tsx +++ b/frontend/src/components/system/PollerHealthCard.tsx @@ -50,12 +50,19 @@ function Stat({ label, value, hint, alarm }: { ); } +/** Fallback capacity, so a partial or unexpected payload cannot blank the page. */ +const EMPTY_CAPACITY: PollerHealth['capacity'] = { + devices: 0, avg_fast_poll_ms: null, p90_fast_poll_ms: null, + arrival_per_sec: 0, service_per_sec: null, headroom: null, + backlog: 0, drain_eta_sec: 0, +}; + export default function PollerHealthCard() { const queryClient = useQueryClient(); const [drainResult, setDrainResult] = useState(''); const [confirmDrain, setConfirmDrain] = useState(false); - const { data, isLoading } = useQuery({ + const { data, isLoading, isError, error } = useQuery({ queryKey: ['poller-health'], queryFn: () => pollerApi.health().then(r => r.data), refetchInterval: 15_000, @@ -70,14 +77,49 @@ export default function PollerHealthCard() { }, }); - if (isLoading || !data) { + if (isLoading) { return

Loading poller health…
; } + /** + * Failure has to be visible here of all places. + * + * This panel previously rendered "Loading…" forever on an error and threw + * outright on an unexpected payload, blanking the whole page — the worst + * possible behaviour for the one screen you open when monitoring looks broken. + * Whatever comes back, say something useful about it. + */ + if (isError || !data || typeof data !== 'object') { + const msg = (error as { response?: { data?: { error?: string } }; message?: string } | null); + return ( +
+
+ Could not read poller health +
+

+ {msg?.response?.data?.error || msg?.message || 'The server returned an unexpected response.'} +

+

+ Polling itself may still be running — this panel only reports on it. Check the backend logs + if this persists. +

+ +
+ ); + } + + // Every field below is defaulted. A backend a version behind, or one that + // answered partially, degrades the panel rather than destroying the page. const s = STATUS[data.status] ?? STATUS.ok; const Icon = s.icon; - const cap = data.capacity; - const backlog = cap.backlog; + const cap = data.capacity ?? EMPTY_CAPACITY; + const backlog = cap.backlog ?? 0; + const queues = Array.isArray(data.queues) ? data.queues : []; + const staleDevices = Array.isArray(data.stale_devices) ? data.stale_devices : []; + const workers = data.workers ?? { concurrency: 0, job_timeout_ms: 0, poll_interval_ms: 0 }; return (
@@ -103,7 +145,7 @@ export default function PollerHealthCard() { {/* Capacity. headroom and backlog are shown together deliberately: headroom alone once reported "healthy" to an operator with 752k jobs queued. */}
- +
@@ -148,7 +190,7 @@ export default function PollerHealthCard() { - {data.queues.map(q => ( + {queues.map(q => ( {q.name}

- Devices not polled recently {data.stale_devices.length > 0 && `(${data.stale_devices.length})`} + Devices not polled recently {staleDevices.length > 0 && `(${staleDevices.length})`}

- {data.stale_devices.length === 0 ? ( + {staleDevices.length === 0 ? (

Every device has been polled successfully in the last few minutes.

) : (