From 00a822701e61c829c4c979f2b8043216a7d774c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Helge=20H=C3=B8ibraaten?= Date: Thu, 27 Aug 2026 02:38:53 +0200 Subject: [PATCH] VI-5068 Stop reporting failures as successes Five places told the user something untrue when a request failed. Both flow delete modals raised their success alerts before awaiting the deletes, and awaited them with no catch. A rejection therefore arrived after the user had already been told the delete was under way, and skipped the rest of the handler, so the button kept spinning and the modal never closed -- with nothing on screen to say why. The webhook delete modal swallowed its error under the comment "Alert emitted by useApi", which useApi does not do: it has no reference to the alerts store and simply throws, so a failed delete was entirely silent and the modal closed as though it had worked. All three now settle first and report per item, through one useOutcomeAlerts helper rather than three copies of the alert-building. resolveVideoSourceId answered a failed collection request with source.id. That is a real answer for a source with no collection, so the caller could not tell it apart from a resolved one, and went on to show another flow's fps, duration and status as if they were this source's. It returns null now and the source is left for the next pass. The two remaining catches in the embed loops logged nothing, which is how a query parameter rejected on every poll went unnoticed for four months. They warn now. The delete-segments timerange was interpolated into the query unencoded, though a timerange is typed by the user and carries `[`, `:` and `)`. It goes through buildQuery like every other query in the app. Verified against a stub TAMS 8.2 store in headless Chrome: the embed still resolves a multi-source recording and reaches Open and LIVE 5s in, and a failing collection request now names itself in the console instead of resolving to the wrong flow. Co-Authored-By: Claude Opus 5 (1M context) --- src/components/FlowDeleteModal.jsx | 24 ++++++-------- src/components/FlowDeleteTimeRangeModal.jsx | 30 +++++++---------- src/components/WebhookDeleteModal.jsx | 35 +++++++------------- src/hooks/useFlows.jsx | 12 +++++-- src/stores/useAlertsStore.jsx | 36 +++++++++++++++++++++ src/views/Embed/index.jsx | 21 +++++++++--- 6 files changed, 95 insertions(+), 63 deletions(-) diff --git a/src/components/FlowDeleteModal.jsx b/src/components/FlowDeleteModal.jsx index d80d390e..7af9fbe2 100644 --- a/src/components/FlowDeleteModal.jsx +++ b/src/components/FlowDeleteModal.jsx @@ -6,7 +6,7 @@ import { SpaceBetween, TextContent, } from "@cloudscape-design/components"; -import useAlertsStore from "@/stores/useAlertsStore"; +import { useOutcomeAlerts } from "@/stores/useAlertsStore"; import { useDelete } from "@/hooks/useFlows"; const FlowDeleteModal = ({ @@ -16,24 +16,18 @@ const FlowDeleteModal = ({ }) => { const [isDeleting, setIsDeleting] = useState(false); const { del } = useDelete(); - const addAlertItems = useAlertsStore((state) => state.addAlertItems); - const delAlertItem = useAlertsStore((state) => state.delAlertItem); + const reportOutcomes = useOutcomeAlerts(); const deleteFlow = async () => { setIsDeleting(true); - const promises = selectedItems.map((item) => del({ flowId: item.id })); - const id = crypto.randomUUID(); - addAlertItems( - selectedItems.map((flow, n) => ({ - type: "success", - dismissible: true, - dismissLabel: "Dismiss message", - content: `Flow ${flow.id} is being deleted. This will happen asynchronously`, - id: `${id}-${n}`, - onDismiss: () => delAlertItem(`${id}-${n}`), - })) + const results = await Promise.allSettled( + selectedItems.map((item) => del({ flowId: item.id })) ); - await Promise.all(promises); + reportOutcomes(results, selectedItems, { + success: (flow) => + `Flow ${flow.id} is being deleted. This will happen asynchronously`, + failure: (flow, reason) => `Failed to delete flow ${flow.id}: ${reason}`, + }); setIsDeleting(false); setModalVisible(false); }; diff --git a/src/components/FlowDeleteTimeRangeModal.jsx b/src/components/FlowDeleteTimeRangeModal.jsx index ff44ce3f..dfe89843 100644 --- a/src/components/FlowDeleteTimeRangeModal.jsx +++ b/src/components/FlowDeleteTimeRangeModal.jsx @@ -7,7 +7,7 @@ import { Modal, SpaceBetween, } from "@cloudscape-design/components"; -import useAlertsStore from "@/stores/useAlertsStore"; +import { useOutcomeAlerts } from "@/stores/useAlertsStore"; import { useDeleteTimerange } from "@/hooks/useFlows"; const FlowDeleteTimeRangeModal = ({ @@ -16,28 +16,22 @@ const FlowDeleteTimeRangeModal = ({ selectedItems, }) => { const { delTimerange, isDeletingTimerange } = useDeleteTimerange(); - const addAlertItems = useAlertsStore((state) => state.addAlertItems); - const delAlertItem = useAlertsStore((state) => state.delAlertItem); + const reportOutcomes = useOutcomeAlerts(); const [timerange, setTimerange] = useState(""); const deleteTimerange = async () => { - const promises = selectedItems.map((item) => - delTimerange({ flowId: item.id, timerange }) + const results = await Promise.allSettled( + selectedItems.map((item) => delTimerange({ flowId: item.id, timerange })) ); - const id = crypto.randomUUID(); - addAlertItems( - selectedItems.map((flow, n) => ({ - type: "success", - dismissible: true, - dismissLabel: "Dismiss message", - content: `Flow segments on flow ${flow.id} within the timerange ${timerange} are being deleted. This will happen asynchronously.`, - id: `${id}-${n}`, - onDismiss: () => delAlertItem(`${id}-${n}`), - })) - ); - await Promise.all(promises); + const allSucceeded = reportOutcomes(results, selectedItems, { + success: (flow) => + `Flow segments on flow ${flow.id} within the timerange ${timerange} are being deleted. This will happen asynchronously.`, + failure: (flow, reason) => + `Failed to delete segments on flow ${flow.id} within the timerange ${timerange}: ${reason}`, + }); setModalVisible(false); - setTimerange(""); + // Kept on failure so the timerange is still there to retry with. + if (allSucceeded) setTimerange(""); }; const handleDismiss = () => { diff --git a/src/components/WebhookDeleteModal.jsx b/src/components/WebhookDeleteModal.jsx index 4c3e89d6..c74601ff 100644 --- a/src/components/WebhookDeleteModal.jsx +++ b/src/components/WebhookDeleteModal.jsx @@ -2,7 +2,7 @@ import { useState } from "react"; import { useNavigate } from "react-router-dom"; import { Modal, TextContent } from "@cloudscape-design/components"; import CancelModalFooter from "@/components/CancelModalFooter"; -import useAlertsStore from "@/stores/useAlertsStore"; +import { useOutcomeAlerts } from "@/stores/useAlertsStore"; import { useDelete } from "@/hooks/useWebhooks"; const WebhookDeleteModal = ({ @@ -13,8 +13,7 @@ const WebhookDeleteModal = ({ const navigate = useNavigate(); const [isDeleting, setIsDeleting] = useState(false); const { del } = useDelete(); - const addAlertItems = useAlertsStore((state) => state.addAlertItems); - const delAlertItem = useAlertsStore((state) => state.delAlertItem); + const reportOutcomes = useOutcomeAlerts(); const handleDismiss = () => { setIsDeleting(false); @@ -24,26 +23,16 @@ const WebhookDeleteModal = ({ const deleteWebhooks = async () => { setIsDeleting(true); - try { - await Promise.all( - selectedItems.map((item) => del({ webhookId: item.id })) - ); - const id = crypto.randomUUID(); - addAlertItems( - selectedItems.map((webhook, n) => ({ - type: "success", - dismissible: true, - dismissLabel: "Dismiss message", - content: `Webhook ${webhook.id} is being deleted. This will happen asynchronously`, - id: `${id}-${n}`, - onDismiss: () => delAlertItem(`${id}-${n}`), - })) - ); - } catch { - // Alert emitted by useApi - } finally { - handleDismiss(); - } + const results = await Promise.allSettled( + selectedItems.map((item) => del({ webhookId: item.id })) + ); + reportOutcomes(results, selectedItems, { + success: (webhook) => + `Webhook ${webhook.id} is being deleted. This will happen asynchronously`, + failure: (webhook, reason) => + `Failed to delete webhook ${webhook.id}: ${reason}`, + }); + handleDismiss(); }; return ( diff --git a/src/hooks/useFlows.jsx b/src/hooks/useFlows.jsx index fdb5291b..4000049e 100644 --- a/src/hooks/useFlows.jsx +++ b/src/hooks/useFlows.jsx @@ -115,9 +115,15 @@ export const useDeleteTimerange = () => { const { trigger, isMutating } = useSWRMutation( api.endpoint ? [api.endpoint, "/flows"] : null, ([, path], { arg }) => - api.del(`${path}/${arg.flowId}/segments?timerange=${arg.timerange}`).then( - (response) => response.data - ) + // buildQuery rather than interpolation: a timerange is typed by the user + // and carries `[`, `:` and `)`, which have to be percent-encoded. + api + .del( + buildQuery(`${path}/${arg.flowId}/segments`, { + timerange: arg.timerange, + }) + ) + .then((response) => response.data) ); return { diff --git a/src/stores/useAlertsStore.jsx b/src/stores/useAlertsStore.jsx index 504a5ea0..d0803e64 100644 --- a/src/stores/useAlertsStore.jsx +++ b/src/stores/useAlertsStore.jsx @@ -16,4 +16,40 @@ const useAlertsStore = create((set) => ({ })), })); +/** + * Report the outcome of a batch of requests, one alert per item. + * + * Alerts are raised only once the requests have settled: announcing success + * up front and then awaiting the promises told the user a delete had happened + * when it might still fail, and a rejection skipped the rest of the handler, + * leaving the button spinning with nothing on screen to explain it. + * + * Returns whether every request succeeded. + */ +export const useOutcomeAlerts = () => { + const addAlertItems = useAlertsStore((state) => state.addAlertItems); + const delAlertItem = useAlertsStore((state) => state.delAlertItem); + + return (results, items, { success, failure }) => { + const batch = crypto.randomUUID(); + addAlertItems( + results.map((result, n) => { + const id = `${batch}-${n}`; + const failed = result.status === "rejected"; + const reason = + result.reason instanceof Error ? result.reason.message : "Unknown error"; + return { + type: failed ? "error" : "success", + dismissible: true, + dismissLabel: "Dismiss message", + content: failed ? failure(items[n], reason) : success(items[n]), + id, + onDismiss: () => delAlertItem(id), + }; + }) + ); + return results.every((result) => result.status === "fulfilled"); + }; +}; + export default useAlertsStore; diff --git a/src/views/Embed/index.jsx b/src/views/Embed/index.jsx index a41b207d..05bf5913 100644 --- a/src/views/Embed/index.jsx +++ b/src/views/Embed/index.jsx @@ -112,8 +112,13 @@ const resolveVideoSourceId = async (source, api, canQueryCollections) => { const children = Array.isArray(response.data) ? response.data : []; const byId = new Map(children.map((child) => [child.id, child])); return findVideoMember(collection, byId) ?? source.id; - } catch { - return source.id; + } catch (error) { + // Not `source.id`: that is a real answer for a source with no collection, + // and returning it here would pin the wrong flow's fps, duration and + // status onto the row and present them as the source's own. Null instead, + // so the caller leaves the source unresolved and looks again. + console.warn("[embed] could not resolve the video source for", source.id, error); + return null; } }; @@ -192,6 +197,7 @@ const useSourceFlowDetails = (displayedSources, liveSourceIds) => { api, canQueryCollections ); + if (!videoSourceId) return; const listRes = await api.get(`/flows?source_id=${videoSourceId}&limit=1`); const listed = listRes.data?.[0]; if (!listed) { @@ -244,7 +250,12 @@ const useSourceFlowDetails = (displayedSources, liveSourceIds) => { return next; }); } - } catch { /* skip */ } + } catch (error) { + // Reported rather than swallowed: a request that fails on every + // pass is otherwise indistinguishable from one that succeeds, and + // an unknown query parameter went unnoticed for months that way. + console.warn("[embed] could not read the flow for source", source.id, error); + } }) ); if (!cancelled && showLoading) setIsLoading(false); @@ -303,7 +314,9 @@ const useSourceFlowDetails = (displayedSources, liveSourceIds) => { next.set(sourceId, { ...existing, isGrowing, durationMs, hasSegments }); return next; }); - } catch { /* skip */ } + } catch (error) { + console.warn("[embed] could not poll flow", flowId, error); + } }) ); };