Merge pull request #1084 from LibreQoE/fix_1082

Prevent Urgent Issues modal lock-ups on clear actions

src/rust/lqosd/src/node_manager/js_build/src/executive_utils.js

@@ -1,17 +1,10 @@
+import {listenOnceMatchingWithTimeout} from "./pubsub/listeners";
 import {get_ws_client, subscribeWS} from "./pubsub/ws";
 
 const wsClient = get_ws_client();
 const DEFAULT_EXECUTIVE_REFRESH_MS = 5000;
 let executiveRequestCounter = 0;
 
-const listenOnce = (eventName, handler) => {
-    const wrapped = (msg) => {
-        wsClient.off(eventName, wrapped);
-        handler(msg);
-    };
-    wsClient.on(eventName, wrapped);
-};
-
 function normalizeEntityKinds(entityKinds) {
     return [...new Set((entityKinds || []).map((kind) => String(kind)))].sort();
 }
@@ -178,7 +171,7 @@ export function getNodeIdMap() {
             siteIdMap = map;
             resolve(map);
         };
-        listenOnce("NetworkTreeLite", (msg) => {
+        listenOnceMatchingWithTimeout(wsClient, "NetworkTreeLite", 5000, () => true, (msg) => {
             const data = msg && msg.data ? msg.data : [];
             const map = new Map();
             (data || []).forEach((entry) => {
@@ -190,11 +183,10 @@ export function getNodeIdMap() {
                 }
             });
             resolveOnce(map);
+        }, () => {
+            resolveOnce(new Map());
         });
         wsClient.send({ NetworkTreeLite: {} });
-        setTimeout(() => {
-            resolveOnce(new Map());
-        }, 5000);
     });
     return siteIdMapPromise;
 }

src/rust/lqosd/src/node_manager/js_build/src/pubsub/listeners.js

@@ -0,0 +1,23 @@
+export function listenOnceMatchingWithTimeout(wsClient, eventName, timeoutMs, matches, handler, onTimeout) {
+    let done = false;
+    const cleanup = wsClient.on(eventName, (msg) => {
+        if (done) return;
+        if (!matches(msg)) return;
+        done = true;
+        clearTimeout(timer);
+        cleanup();
+        handler(msg);
+    });
+    const timer = setTimeout(() => {
+        if (done) return;
+        done = true;
+        cleanup();
+        onTimeout();
+    }, timeoutMs);
+    return () => {
+        if (done) return;
+        done = true;
+        clearTimeout(timer);
+        cleanup();
+    };
+}

src/rust/lqosd/src/node_manager/js_build/src/pubsub/ws.test.mjs

@@ -0,0 +1,103 @@
+import assert from "node:assert/strict";
+import test from "node:test";
+
+globalThis.window = {
+    LQOS_UI_VERSION: "test",
+    location: { search: "", protocol: "http:", host: "localhost" },
+    localStorage: { getItem: () => null },
+    sessionStorage: {
+        getItem: () => null,
+        setItem: () => {},
+        removeItem: () => {},
+    },
+};
+globalThis.document = { cookie: "" };
+
+const { listenOnceMatchingWithTimeout } = await import("./listeners.js");
+
+function makeWsClient() {
+    const handlers = new Map();
+    return {
+        on(eventName, handler) {
+            if (!handlers.has(eventName)) {
+                handlers.set(eventName, new Set());
+            }
+            handlers.get(eventName).add(handler);
+            return () => this.off(eventName, handler);
+        },
+        off(eventName, handler) {
+            handlers.get(eventName)?.delete(handler);
+        },
+        emit(eventName, message) {
+            for (const handler of Array.from(handlers.get(eventName) || [])) {
+                handler(message);
+            }
+        },
+        handlerCount(eventName) {
+            return handlers.get(eventName)?.size || 0;
+        },
+    };
+}
+
+test("timeout listener ignores stale responses and resolves matching response", () => {
+    const wsClient = makeWsClient();
+    const seen = [];
+
+    listenOnceMatchingWithTimeout(
+        wsClient,
+        "UrgentClearResult",
+        1000,
+        (message) => message.request_id === 2,
+        (message) => seen.push(message.ok),
+        () => seen.push("timeout"),
+    );
+
+    wsClient.emit("UrgentClearResult", { request_id: 1, ok: false });
+    assert.deepEqual(seen, []);
+    assert.equal(wsClient.handlerCount("UrgentClearResult"), 1);
+
+    wsClient.emit("UrgentClearResult", { request_id: 2, ok: true });
+    assert.deepEqual(seen, [true]);
+    assert.equal(wsClient.handlerCount("UrgentClearResult"), 0);
+});
+
+test("timeout listener disposer removes pending handler", () => {
+    const wsClient = makeWsClient();
+    const seen = [];
+
+    const dispose = listenOnceMatchingWithTimeout(
+        wsClient,
+        "UrgentList",
+        1000,
+        () => true,
+        () => seen.push("response"),
+        () => seen.push("timeout"),
+    );
+
+    assert.equal(wsClient.handlerCount("UrgentList"), 1);
+    dispose();
+    wsClient.emit("UrgentList", { request_id: 1 });
+
+    assert.deepEqual(seen, []);
+    assert.equal(wsClient.handlerCount("UrgentList"), 0);
+});
+
+test("timeout listener fires timeout and removes pending handler", async () => {
+    const wsClient = makeWsClient();
+    const seen = [];
+
+    listenOnceMatchingWithTimeout(
+        wsClient,
+        "UrgentList",
+        5,
+        () => false,
+        () => seen.push("response"),
+        () => seen.push("timeout"),
+    );
+
+    assert.equal(wsClient.handlerCount("UrgentList"), 1);
+    await new Promise((resolve) => setTimeout(resolve, 20));
+
+    assert.deepEqual(seen, ["timeout"]);
+    assert.equal(wsClient.handlerCount("UrgentList"), 0);
+});

src/rust/lqosd/src/node_manager/js_build/src/template.js

@@ -2,6 +2,7 @@ import {clearDiv} from "./helpers/builders";
 import {initRedact} from "./helpers/redact";
 import {initDayNightMode} from "./helpers/dark_mode";
 import {initColorBlind} from "./helpers/colorblind";
+import {listenOnceMatchingWithTimeout as listenWsOnceMatchingWithTimeout} from "./pubsub/listeners";
 import {get_ws_client} from "./pubsub/ws";
 
 const wsClient = get_ws_client();
@@ -29,6 +30,7 @@ function escapeHtml(text) {
 
 const SCHEDULER_STATUS_POLL_MS = 2000;
 const SCHEDULER_STATUS_TIMEOUT_MS = 2500;
+const URGENT_ISSUES_TIMEOUT_MS = 2500;
 const SCHEDULER_STATUS_STARTING_GRACE_MS = 30000;
 const SCHEDULER_MODAL_IDLE_POLL_MS = 3000;
 const SCHEDULER_MODAL_ACTIVE_POLL_MS = 1000;
@@ -42,24 +44,6 @@ let schedulerModalInstance = null;
 let cachedSchedulerDetailsData = null;
 let cachedQueueModeConfig = null;
 
-function listenOnceWithTimeout(eventName, timeoutMs, handler, onTimeout) {
-    let done = false;
-    const wrapped = (msg) => {
-        if (done) return;
-        done = true;
-        clearTimeout(timer);
-        wsClient.off(eventName, wrapped);
-        handler(msg);
-    };
-    const timer = setTimeout(() => {
-        if (done) return;
-        done = true;
-        wsClient.off(eventName, wrapped);
-        onTimeout();
-    }, timeoutMs);
-    wsClient.on(eventName, wrapped);
-}
-
 function clampSchedulerPercent(value) {
     const parsed = Number(value);
     if (!Number.isFinite(parsed)) return 0;
@@ -526,7 +510,7 @@ function loadSchedulerStatus(force = false) {
     }
 
     schedulerStatusRequestInFlight = true;
-    listenOnceWithTimeout("SchedulerStatus", SCHEDULER_STATUS_TIMEOUT_MS, (msg) => {
+    listenWsOnceMatchingWithTimeout(wsClient, "SchedulerStatus", SCHEDULER_STATUS_TIMEOUT_MS, () => true, (msg) => {
         schedulerStatusRequestInFlight = false;
         if (!msg || !msg.data) {
             if (cachedSchedulerStatusData) {
@@ -685,7 +669,7 @@ function refreshSchedulerModalDetails(showLoading = false) {
         renderSchedulerModalLoading();
     }
     schedulerModalRequestInFlight = true;
-    listenOnceWithTimeout("SchedulerDetails", SCHEDULER_STATUS_TIMEOUT_MS, (msg) => {
+    listenWsOnceMatchingWithTimeout(wsClient, "SchedulerDetails", SCHEDULER_STATUS_TIMEOUT_MS, () => true, (msg) => {
         schedulerModalRequestInFlight = false;
         if (!isSchedulerModalOpen()) {
             stopSchedulerModalPolling();
@@ -1108,6 +1092,15 @@ function initUrgentIssues() {
     const containerId = 'urgentStatus';
     const linkId = 'urgentStatusLink';
     const badgeId = 'urgentBadge';
+    let urgentClearState = null;
+    let urgentStatusPending = false;
+    let urgentStatusRefreshQueued = false;
+    let urgentListPending = false;
+    let urgentListActiveRequestId = null;
+    let urgentListCancel = null;
+    let urgentRequestId = 0;
+    let pendingUrgentActionError = null;
+    let pendingUrgentRetryMessage = null;
 
     function ensurePlaceholder() {
         return document.getElementById(containerId) !== null;
@@ -1131,38 +1124,251 @@ function initUrgentIssues() {
 
     function poll() {
         if (!ensurePlaceholder()) return;
-        listenOnce("UrgentStatus", (msg) => {
-            const count = msg && msg.data ? msg.data.count : 0;
-            renderStatus(count || 0);
+        if (urgentStatusPending) {
+            urgentStatusRefreshQueued = true;
+            return;
+        }
+        urgentStatusPending = true;
+        const requestId = ++urgentRequestId;
+        listenWsOnceMatchingWithTimeout(
+            wsClient,
+            "UrgentStatus",
+            URGENT_ISSUES_TIMEOUT_MS,
+            (msg) => msg.request_id === requestId,
+            (msg) => {
+                urgentStatusPending = false;
+                const count = msg && msg.data ? msg.data.count : 0;
+                renderStatus(count || 0);
+                runQueuedUrgentStatusRefresh();
+            },
+            () => {
+                urgentStatusPending = false;
+                runQueuedUrgentStatusRefresh();
+            }
+        );
+        wsClient.send({ UrgentStatus: { request_id: requestId } });
+    }
+
+    function runQueuedUrgentStatusRefresh() {
+        if (!urgentStatusRefreshQueued) return false;
+        urgentStatusRefreshQueued = false;
+        poll();
+        return true;
+    }
+
+    function clearActionInFlight() {
+        return urgentClearState !== null;
+    }
+
+    function clearActionBlocked() {
+        return clearActionInFlight() || urgentListPending;
+    }
+
+    function setClearControlsDisabled(disabled) {
+        document.querySelectorAll('.urgent-clear').forEach((button) => {
+            button.disabled = disabled;
         });
-        wsClient.send({ UrgentStatus: {} });
+        const clearAllButton = document.getElementById('urgentClearAll');
+        if (clearAllButton) {
+            clearAllButton.disabled = disabled;
+        }
+    }
+
+    function showUrgentBusyState(message) {
+        const holder = document.getElementById('urgentListContainer');
+        if (!holder || !urgentModalIsOpen()) return;
+        holder.setAttribute('aria-busy', 'true');
+        holder.querySelector('.urgent-busy-status')?.remove();
+        holder.insertAdjacentHTML('afterbegin', `
+            <div class="alert alert-info mb-2 urgent-busy-status" role="status" aria-live="polite" tabindex="-1">
+                <i class="fa fa-spinner fa-spin" aria-hidden="true"></i>
+                ${escapeHtml(message)}
+            </div>`);
+        holder.querySelector('.urgent-busy-status')?.focus();
+    }
+
+    function clearUrgentBusyState(holder) {
+        const target = holder || document.getElementById('urgentListContainer');
+        if (!target) return;
+        target.removeAttribute('aria-busy');
+        target.querySelector('.urgent-busy-status')?.remove();
+    }
+
+    function renderUrgentWarning(holder, message, { focus = false, retry = false } = {}) {
+        if (!holder) return;
+        holder.querySelector('.urgent-action-error')?.remove();
+        const retryButton = retry
+            ? '<button type="button" class="btn btn-link btn-sm urgent-refresh">Retry</button>'
+            : '';
+        holder.insertAdjacentHTML('afterbegin', `
+            <div class="alert alert-warning mb-2 urgent-action-error" role="alert" tabindex="-1">
+                ${escapeHtml(message)}
+                ${retryButton}
+            </div>`);
+        if (focus) {
+            holder.querySelector(retry ? '.urgent-refresh' : '.urgent-action-error')?.focus();
+        }
+    }
+
+    function showUrgentActionError(message) {
+        pendingUrgentActionError = message;
+        if (!urgentModalIsOpen()) return;
+        const holder = document.getElementById('urgentListContainer');
+        if (!holder) return;
+        clearUrgentBusyState(holder);
+        renderUrgentWarning(holder, message, { focus: true });
+        pendingUrgentActionError = null;
+    }
+
+    function attachPendingUrgentActionError(holder) {
+        if (!pendingUrgentActionError || !urgentModalIsOpen() || !holder) return;
+        renderUrgentWarning(holder, pendingUrgentActionError);
+        pendingUrgentActionError = null;
+    }
+
+    function showUrgentRetryState(message) {
+        pendingUrgentRetryMessage = message;
+        const holder = document.getElementById('urgentListContainer');
+        if (!holder || !urgentModalIsOpen()) return;
+        pendingUrgentRetryMessage = null;
+        setClearControlsDisabled(true);
+        clearUrgentBusyState(holder);
+        holder.innerHTML = '';
+        renderUrgentWarning(holder, message, { focus: true, retry: true });
+        $(holder).off('click').on('click', '.urgent-refresh', function (e) {
+            e.preventDefault();
+            showModal();
+        });
+    }
+
+    function showPendingUrgentRetryState() {
+        if (!pendingUrgentRetryMessage || !urgentModalIsOpen()) return false;
+        showUrgentRetryState(pendingUrgentRetryMessage);
+        return true;
+    }
+
+    function urgentModalIsOpen() {
+        return document.getElementById('urgentModal')?.classList.contains('show') === true;
+    }
+
+    function focusUrgentModalFallback() {
+        const modalEl = document.getElementById('urgentModal');
+        if (!modalEl || !urgentModalIsOpen()) return;
+        modalEl
+            .querySelector('.urgent-clear:not(:disabled), #urgentClearAll:not(:disabled), .btn-close:not(:disabled), [data-bs-dismiss="modal"]:not(:disabled)')
+            ?.focus();
+    }
+
+    function refreshUrgentIssues() {
+        showModal();
+        poll();
+    }
+
+    function completeClearAction() {
+        urgentClearState = null;
+        setClearControlsDisabled(clearActionInFlight());
+        refreshUrgentIssues();
+    }
+
+    function startUrgentClear({
+        responseEvent,
+        requestPayload,
+        busyMessage,
+        failureMessage,
+        timeoutMessage,
+    }) {
+        if (clearActionBlocked()) {
+            return;
+        }
+        const requestId = ++urgentRequestId;
+        setClearControlsDisabled(true);
+        showUrgentBusyState(busyMessage);
+        urgentClearState = {
+            busyMessage,
+            cancel: null,
+        };
+        urgentClearState.cancel = listenWsOnceMatchingWithTimeout(
+            wsClient,
+            responseEvent,
+            URGENT_ISSUES_TIMEOUT_MS,
+            (msg) => msg.request_id === requestId,
+            (msg) => {
+                urgentClearState = null;
+                if (msg && msg.ok === false) {
+                    setClearControlsDisabled(false);
+                    showUrgentActionError(failureMessage);
+                    return;
+                }
+                completeClearAction();
+            },
+            () => {
+                urgentClearState = null;
+                setClearControlsDisabled(false);
+                showUrgentActionError(timeoutMessage);
+            }
+        );
+        wsClient.send(requestPayload(requestId));
     }
 
     function showModal() {
         const modalEl = document.getElementById('urgentModal');
-        if (!modalEl) return;
-        new bootstrap.Modal(modalEl, { focus: true }).show();
+        if (!modalEl) {
+            return;
+        }
+        bootstrap.Modal.getOrCreateInstance(modalEl, { focus: true }).show();
         const holder = document.getElementById('urgentListContainer');
-        if (!holder) return;
-        holder.innerHTML = `<div class="text-center text-muted"><i class='fa fa-spinner fa-spin'></i> Loading...</div>`;
-        listenOnce("UrgentList", (msg) => {
-            const items = msg && msg.data ? msg.data.items || [] : [];
-            if (items.length === 0) {
-                holder.innerHTML = '<div class="text-center text-success">No urgent issues.</div>';
-                return;
-            }
-            const table = document.createElement('table');
-            table.className = 'lqos-table lqos-table-compact mb-0';
-            const tbody = document.createElement('tbody');
-            items.forEach((it) => {
-                const tr = document.createElement('tr');
-                const td = document.createElement('td');
-                const when = new Date(it.ts * 1000).toLocaleString();
-                const sev = it.severity === 'Error' ? 'danger' : 'warning';
-                const clearControl = it.clearable === false
-                    ? '<span class="badge bg-secondary-subtle text-secondary border float-end ms-3">Active</span>'
-                    : `<button type="button" class="btn btn-link btn-sm text-secondary float-end ms-3 p-0 urgent-clear" data-id="${it.id}" title="Acknowledge issue" aria-label="Acknowledge issue ${escapeAttr(it.code)}"><i class="fa fa-times" aria-hidden="true"></i></button>`;
-                td.innerHTML = `
+        if (!holder) {
+            return;
+        }
+        if (showPendingUrgentRetryState()) {
+            return;
+        }
+        if (clearActionInFlight()) {
+            setClearControlsDisabled(true);
+            showUrgentBusyState(urgentClearState.busyMessage);
+            return;
+        }
+        if (urgentListPending) {
+            setClearControlsDisabled(true);
+            return;
+        }
+        urgentListPending = true;
+        const requestId = ++urgentRequestId;
+        urgentListActiveRequestId = requestId;
+        setClearControlsDisabled(true);
+        clearUrgentBusyState(holder);
+        holder.innerHTML = `<div class="text-center text-muted urgent-loading" role="status" aria-live="polite" tabindex="-1"><i class='fa fa-spinner fa-spin'></i> Loading...</div>`;
+        holder.querySelector('.urgent-loading')?.focus();
+        urgentListCancel = listenWsOnceMatchingWithTimeout(
+            wsClient,
+            "UrgentList",
+            URGENT_ISSUES_TIMEOUT_MS,
+            (msg) => msg.request_id === requestId && urgentListActiveRequestId === requestId,
+            (msg) => {
+                urgentListPending = false;
+                urgentListActiveRequestId = null;
+                urgentListCancel = null;
+                const items = msg && msg.data ? msg.data.items || [] : [];
+                clearUrgentBusyState(holder);
+                if (items.length === 0) {
+                    holder.innerHTML = '<div class="text-center text-success" role="status" aria-live="polite" tabindex="-1">No urgent issues.</div>';
+                    attachPendingUrgentActionError(holder);
+                    setClearControlsDisabled(true);
+                    holder.querySelector('[role="status"]')?.focus();
+                    return;
+                }
+                const table = document.createElement('table');
+                table.className = 'lqos-table lqos-table-compact mb-0';
+                const tbody = document.createElement('tbody');
+                items.forEach((it) => {
+                    const tr = document.createElement('tr');
+                    const td = document.createElement('td');
+                    const when = new Date(it.ts * 1000).toLocaleString();
+                    const sev = it.severity === 'Error' ? 'danger' : 'warning';
+                    const clearControl = it.clearable === false
+                        ? '<span class="badge bg-secondary-subtle text-secondary border float-end ms-3">Active</span>'
+                        : `<button type="button" class="btn btn-link btn-sm text-secondary float-end ms-3 p-0 urgent-clear" data-id="${it.id}" title="Acknowledge issue" aria-label="Acknowledge issue ${escapeAttr(it.code)}"><i class="fa fa-times" aria-hidden="true"></i></button>`;
+                    td.innerHTML = `
                     <div>
                         <span class="badge bg-${sev}">${it.severity}</span>
                         <strong class="ms-2">${it.code}</strong>
@@ -1173,26 +1379,44 @@ function initUrgentIssues() {
                     <div class="mt-1" style="white-space: pre-wrap;">${it.message}</div>
                     ${it.context ? `<pre class="mt-2">${it.context}</pre>` : ''}
                     `;
-                tr.appendChild(td);
-                tbody.appendChild(tr);
-            });
-            table.appendChild(tbody);
-            holder.innerHTML = '';
-            const tableWrap = document.createElement('div');
-            tableWrap.className = 'table-responsive lqos-table-wrap';
-            tableWrap.appendChild(table);
-            holder.appendChild(tableWrap);
-            $(holder).off('click').on('click', '.urgent-clear', function (e) {
-                e.preventDefault();
-                const id = $(this).data('id');
-                listenOnce("UrgentClearResult", () => {
-                    showModal();
-                    poll();
+                    tr.appendChild(td);
+                    tbody.appendChild(tr);
                 });
-                wsClient.send({ UrgentClear: { id } });
-            });
-        });
-        wsClient.send({ UrgentList: {} });
+                table.appendChild(tbody);
+                holder.innerHTML = '';
+                const tableWrap = document.createElement('div');
+                tableWrap.className = 'table-responsive lqos-table-wrap';
+                tableWrap.appendChild(table);
+                holder.appendChild(tableWrap);
+                attachPendingUrgentActionError(holder);
+                $(holder).off('click').on('click', '.urgent-clear', function (e) {
+                    e.preventDefault();
+                    const id = $(this).data('id');
+                    startUrgentClear({
+                        responseEvent: 'UrgentClearResult',
+                        requestPayload: (requestId) => ({ UrgentClear: { id, request_id: requestId } }),
+                        busyMessage: 'Acknowledging urgent issue...',
+                        failureMessage: 'Unable to acknowledge urgent issue.',
+                        timeoutMessage: 'Timed out while acknowledging urgent issue.',
+                    });
+                });
+                setClearControlsDisabled(clearActionInFlight());
+                focusUrgentModalFallback();
+            },
+            () => {
+                if (urgentListActiveRequestId !== requestId) return;
+                urgentListPending = false;
+                urgentListActiveRequestId = null;
+                urgentListCancel = null;
+                if (!urgentModalIsOpen()) {
+                    setClearControlsDisabled(clearActionInFlight());
+                    return;
+                }
+                showUrgentRetryState('Unable to refresh urgent issues.');
+                setClearControlsDisabled(true);
+            }
+        );
+        wsClient.send({ UrgentList: { request_id: requestId } });
     }
 
     if (!document.getElementById(containerId)) {
@@ -1206,11 +1430,31 @@ function initUrgentIssues() {
     }
 
     $(document).off('click', '#urgentClearAll').on('click', '#urgentClearAll', () => {
-        listenOnce("UrgentClearAllResult", () => {
-            showModal();
-            poll();
+        startUrgentClear({
+            responseEvent: 'UrgentClearAllResult',
+            requestPayload: (requestId) => ({ UrgentClearAll: { request_id: requestId } }),
+            busyMessage: 'Acknowledging urgent issues...',
+            failureMessage: 'Unable to acknowledge urgent issues.',
+            timeoutMessage: 'Timed out while acknowledging urgent issues.',
         });
-        wsClient.send({ UrgentClearAll: {} });
+    });
+
+    $(document).off('hidden.bs.modal', '#urgentModal').on('hidden.bs.modal', '#urgentModal', () => {
+        pendingUrgentRetryMessage = null;
+        if (urgentClearState?.cancel) {
+            urgentClearState.cancel();
+            urgentClearState = null;
+        }
+        if (urgentListPending) {
+            if (urgentListCancel) {
+                urgentListCancel();
+                urgentListCancel = null;
+            }
+            urgentListPending = false;
+            urgentListActiveRequestId = null;
+        }
+        clearUrgentBusyState();
+        setClearControlsDisabled(clearActionInFlight());
     });
 
     poll();

src/rust/lqosd/src/node_manager/js_build/test-node-manager.sh

@@ -9,7 +9,9 @@ if ! command -v node >/dev/null 2>&1; then
 fi
 
 echo "[test-node-manager] Running frontend contract tests..."
-node --test "${SCRIPT_DIR}/src/config/shaped_device_wire.test.mjs"
+node --test \
+  "${SCRIPT_DIR}/src/config/shaped_device_wire.test.mjs" \
+  "${SCRIPT_DIR}/src/pubsub/ws.test.mjs"
 
 echo "[test-node-manager] Building bundles..."
 "${SCRIPT_DIR}/esbuild.sh"

src/rust/lqosd/src/node_manager/ws.rs

@@ -676,32 +676,37 @@ async fn receive_channel_message(
                 return true;
             }
         }
-        WsRequest::UrgentStatus => {
+        WsRequest::UrgentStatus { request_id } => {
             let response = WsResponse::UrgentStatus {
                 data: urgent::urgent_status_data(),
+                request_id,
             };
             if send_ws_response(&tx, response).await {
                 return true;
             }
         }
-        WsRequest::UrgentList => {
+        WsRequest::UrgentList { request_id } => {
             let response = WsResponse::UrgentList {
                 data: urgent::urgent_list_data(),
+                request_id,
             };
             if send_ws_response(&tx, response).await {
                 return true;
             }
         }
-        WsRequest::UrgentClear { id } => {
+        WsRequest::UrgentClear { id, request_id } => {
             let ok = urgent::urgent_clear_id(id);
-            let response = WsResponse::UrgentClearResult { ok };
+            let response = WsResponse::UrgentClearResult { ok, request_id };
             if send_ws_response(&tx, response).await {
                 return true;
             }
         }
-        WsRequest::UrgentClearAll => {
+        WsRequest::UrgentClearAll { request_id } => {
             urgent::urgent_clear_all_data();
-            let response = WsResponse::UrgentClearAllResult { ok: true };
+            let response = WsResponse::UrgentClearAllResult {
+                ok: true,
+                request_id,
+            };
             if send_ws_response(&tx, response).await {
                 return true;
             }
@@ -2502,7 +2507,9 @@ fn payload_hint(payload: &[u8]) -> &'static str {
 #[cfg(test)]
 mod tests {
     use super::decode_ws_request;
+    use crate::node_manager::local_api::urgent::{UrgentList, UrgentStatus};
     use crate::node_manager::ws::messages::WsRequest;
+    use crate::node_manager::ws::messages::{WsResponse, encode_ws_message};
     use serde_cbor::Value as CborValue;
     use std::collections::BTreeMap;
 
@@ -2567,6 +2574,163 @@ mod tests {
         .expect("valid update payload")
     }
 
+    fn encode_response_value(response: &WsResponse) -> CborValue {
+        let payload = encode_ws_message(response).expect("response should encode");
+        serde_cbor::from_slice(payload.as_ref()).expect("response should decode as CBOR value")
+    }
+
+    fn response_request_id(response: &WsResponse, event_name: &str) -> Option<u64> {
+        let CborValue::Map(entries) = encode_response_value(response) else {
+            panic!("response should encode as a CBOR map");
+        };
+        let event_key = text("event");
+        let request_id_key = text("request_id");
+        assert_eq!(entries.get(&event_key), Some(&text(event_name)));
+        match entries.get(&request_id_key) {
+            Some(CborValue::Integer(value)) => {
+                Some(u64::try_from(*value).expect("request_id is u64"))
+            }
+            Some(other) => panic!("unexpected request_id value: {other:?}"),
+            None => None,
+        }
+    }
+
+    fn response_has_request_id(response: &WsResponse) -> bool {
+        let CborValue::Map(entries) = encode_response_value(response) else {
+            panic!("response should encode as a CBOR map");
+        };
+        entries.contains_key(&text("request_id"))
+    }
+
+    #[test]
+    fn decodes_urgent_status_with_request_id() {
+        let payload = serde_cbor::to_vec(&map(vec![(
+            "UrgentStatus",
+            map(vec![("request_id", integer(42))]),
+        )]))
+        .expect("valid urgent status payload");
+
+        let request = decode_ws_request(&payload).expect("urgent status request should decode");
+        match request {
+            WsRequest::UrgentStatus { request_id } => assert_eq!(request_id, Some(42)),
+            other => panic!("unexpected request: {other:?}"),
+        }
+    }
+
+    #[test]
+    fn decodes_urgent_list_without_request_id() {
+        let payload = serde_cbor::to_vec(&map(vec![("UrgentList", map(vec![]))]))
+            .expect("valid urgent list payload");
+
+        let request = decode_ws_request(&payload).expect("urgent list request should decode");
+        match request {
+            WsRequest::UrgentList { request_id } => assert_eq!(request_id, None),
+            other => panic!("unexpected request: {other:?}"),
+        }
+    }
+
+    #[test]
+    fn decodes_urgent_clear_with_request_id() {
+        let payload = serde_cbor::to_vec(&map(vec![(
+            "UrgentClear",
+            map(vec![("id", integer(7)), ("request_id", integer(42))]),
+        )]))
+        .expect("valid urgent clear payload");
+
+        let request = decode_ws_request(&payload).expect("urgent clear request should decode");
+        match request {
+            WsRequest::UrgentClear { id, request_id } => {
+                assert_eq!(id, 7);
+                assert_eq!(request_id, Some(42));
+            }
+            other => panic!("unexpected request: {other:?}"),
+        }
+    }
+
+    #[test]
+    fn decodes_urgent_clear_all_without_request_id() {
+        let payload = serde_cbor::to_vec(&map(vec![("UrgentClearAll", map(vec![]))]))
+            .expect("valid urgent clear all payload");
+
+        let request = decode_ws_request(&payload).expect("urgent clear all request should decode");
+        match request {
+            WsRequest::UrgentClearAll { request_id } => assert_eq!(request_id, None),
+            other => panic!("unexpected request: {other:?}"),
+        }
+    }
+
+    #[test]
+    fn urgent_responses_echo_request_id() {
+        assert_eq!(
+            response_request_id(
+                &WsResponse::UrgentStatus {
+                    data: UrgentStatus {
+                        has_urgent: true,
+                        count: 1,
+                    },
+                    request_id: Some(42),
+                },
+                "UrgentStatus",
+            ),
+            Some(42),
+        );
+        assert_eq!(
+            response_request_id(
+                &WsResponse::UrgentList {
+                    data: UrgentList { items: Vec::new() },
+                    request_id: Some(43),
+                },
+                "UrgentList",
+            ),
+            Some(43),
+        );
+        assert_eq!(
+            response_request_id(
+                &WsResponse::UrgentClearResult {
+                    ok: true,
+                    request_id: Some(44),
+                },
+                "UrgentClearResult",
+            ),
+            Some(44),
+        );
+        assert_eq!(
+            response_request_id(
+                &WsResponse::UrgentClearAllResult {
+                    ok: true,
+                    request_id: Some(45),
+                },
+                "UrgentClearAllResult",
+            ),
+            Some(45),
+        );
+    }
+
+    #[test]
+    fn urgent_responses_omit_missing_request_id() {
+        assert!(!response_has_request_id(&WsResponse::UrgentStatus {
+            data: UrgentStatus {
+                has_urgent: false,
+                count: 0,
+            },
+            request_id: None,
+        }));
+        assert!(!response_has_request_id(&WsResponse::UrgentList {
+            data: UrgentList { items: Vec::new() },
+            request_id: None,
+        }));
+        assert!(!response_has_request_id(&WsResponse::UrgentClearResult {
+            ok: true,
+            request_id: None,
+        }));
+        assert!(!response_has_request_id(
+            &WsResponse::UrgentClearAllResult {
+                ok: true,
+                request_id: None,
+            }
+        ));
+    }
+
     #[test]
     fn decodes_create_shaped_device_with_browser_style_ipv4_bytes() {
         let payload = create_shaped_device_payload(device_value(

src/rust/lqosd/src/node_manager/ws/messages.rs

@@ -324,12 +324,19 @@ pub enum WsRequest {
     ProtocolFlowTimeline {
         protocol: String,
     },
-    UrgentStatus,
-    UrgentList,
+    UrgentStatus {
+        request_id: Option<u64>,
+    },
+    UrgentList {
+        request_id: Option<u64>,
+    },
     UrgentClear {
         id: u64,
+        request_id: Option<u64>,
+    },
+    UrgentClearAll {
+        request_id: Option<u64>,
     },
-    UrgentClearAll,
     UnknownIps,
     UnknownIpsClear,
     UnknownIpsCsv,
@@ -626,15 +633,23 @@ pub enum WsResponse {
     },
     UrgentStatus {
         data: UrgentStatus,
+        #[serde(skip_serializing_if = "Option::is_none")]
+        request_id: Option<u64>,
     },
     UrgentList {
         data: UrgentList,
+        #[serde(skip_serializing_if = "Option::is_none")]
+        request_id: Option<u64>,
     },
     UrgentClearResult {
         ok: bool,
+        #[serde(skip_serializing_if = "Option::is_none")]
+        request_id: Option<u64>,
     },
     UrgentClearAllResult {
         ok: bool,
+        #[serde(skip_serializing_if = "Option::is_none")]
+        request_id: Option<u64>,
     },
     UnknownIps {
         data: Vec<UnknownIp>,