From 61d95b99a4893607d61593bc12227c827d1be6a0 Mon Sep 17 00:00:00 2001
From: Mike Teehan <mike.teehan@gmail.com>
Date: Fri, 14 Aug 2026 23:22:39 +0000
Subject: [PATCH] model: add onSuccess/onFailure lifecycle hooks to
 modelRegister

---
 docs/hoover.md               |  6 +++++-
 webui/static/hoover/model.js | 31 ++++++++++++++++++++++++++++---
 2 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/docs/hoover.md b/docs/hoover.md
index 5cd42be..744ae49 100644
--- a/docs/hoover.md
+++ b/docs/hoover.md
@@ -144,6 +144,8 @@ modelRegister('firewall', {
         return r.data;
     },
     defaultData: null,               // optional, initial data value
+    // onSuccess: (name, data, param?) => { },  // optional — after model.data is set (also for null)
+    // onFailure: (name, error) => { },         // optional — after model.error is set (real throws only)
 });
 
 // Parameterized example — tab-aware fetch:
@@ -164,6 +166,8 @@ modelRegister('logs', {
 | `definition.subsystem` | WS topic string. Use `'firewall'`, `'dnsmasq'`, etc. Use `'*'` to match all topics. |
 | `definition.fetch(signal?, param?)` | Async function that fetches and returns data. Throws on error. Receives optional `AbortSignal` and optional parameter (e.g., tab key). |
 | `definition.defaultData` | Optional initial data value (default: `null`) |
+| `definition.onSuccess(name, data, param?)` | Optional lifecycle hook called after `model.data` is assigned — including `data === null` (a resolved `null` is normal, not an error). `param` is the action object passed to `fetch` (or `undefined`), so hooks can tell which action produced the data. Fire-and-forget: hook errors are caught and logged via `console.warn`; they never clobber `model.error`, the returned promise, or the `finally` flag clearing. |
+| `definition.onFailure(name, error)` | Optional lifecycle hook called after `model.error` is assigned. Only reachable on a real throw from `fetch` (e.g., network error). Same fire-and-forget error isolation as `onSuccess`. |
 
 ### `getModel(name)`
 
@@ -212,7 +216,7 @@ modelFetch('logs', 'nginx-access');
 - On failure, stores error in `model.error`.
 - Flags cleared in `finally` block.
 - Does not abort in-progress fetches — other consumers may still need the data.
-- The `param` argument is passed to `fetch(signal, param)` for parameterized models. Dedup key is `name: param`.
+- The `param` argument is passed to `fetch(signal, param)` for parameterized models. Dedup key is `name` (no param) or `name: JSON.stringify(param)` (with param) — object params (e.g. `{ action: 'refresh' }` vs `{ action: 'check' }`) therefore get distinct keys, and param-less `modelFetch(name)` calls retain the bare `name` key.
 
 ### `refreshByTopic(topic)`
 
diff --git a/webui/static/hoover/model.js b/webui/static/hoover/model.js
index a057257..b49ad78 100644
--- a/webui/static/hoover/model.js
+++ b/webui/static/hoover/model.js
@@ -15,7 +15,7 @@
 
 import { reactive } from './reactivity.js';
 
-/** Registered models: name → { model, subsystem, fetch } */
+/** Registered models: name → { model, subsystem, fetch, onSuccess?, onFailure? } */
 const _models = new Map();
 
 /** In-flight fetch promises for dedup: name → Promise */
@@ -29,6 +29,13 @@ const _fetchPromises = new Map();
  * @param {string} definition.subsystem - WS topic to listen for ('*' = all)
  * @param {function} definition.fetch - async (signal?, param?) => Promise<data>
  * @param {any} [definition.defaultData] - Initial data value (default: null)
+ * @param {function} [definition.onSuccess] - (name, data, param?) => void.
+ *        Called after model.data is assigned. Also fires when data === null —
+ *        only a thrown fetch error skips it. Hook errors are caught and logged
+ *        (console.warn) and never affect the fetch promise or model state.
+ * @param {function} [definition.onFailure] - (name, error) => void.
+ *        Called after model.error is assigned, only on a real throw from fetch.
+ *        Hook errors are caught and logged and never affect the fetch promise.
  * @returns {object} reactive model
  */
 export function modelRegister(name, definition) {
@@ -43,6 +50,8 @@ export function modelRegister(name, definition) {
         model,
         subsystem: definition.subsystem,
         fetch: definition.fetch,
+        onSuccess: definition.onSuccess,
+        onFailure: definition.onFailure,
     });
 
     return model;
@@ -59,9 +68,13 @@ export function getModel(name) {
     return entry.model;
 }
 
-/** Build dedup key from model name and optional param. */
+/**
+ * Build dedup key from model name and optional param.
+ * Objects (e.g. `{ action: 'refresh' }`) are JSON-serialized so distinct
+ * param objects get distinct keys; strings/undefined keep bare behavior.
+ */
 function _dedupKey(name, param) {
-    return param !== undefined ? `${name}:${String(param)}` : name;
+    return param !== undefined ? name + ':' + JSON.stringify(param) : name;
 }
 
 /**
@@ -97,8 +110,20 @@ export function modelFetch(name, signalOrParam, signal) {
         try {
             const data = await entry.fetch(actualSignal, param);
             model.data = data;
+            // Fires for any resolved result, including data === null. Hook
+            // errors must not clobber model state or the returned promise.
+            try {
+                entry.onSuccess?.(name, data, param);
+            } catch (err) {
+                console.warn('Model onSuccess hook (' + name + ') threw:', err);
+            }
         } catch (e) {
             model.error = e.message || 'Fetch failed';
+            try {
+                entry.onFailure?.(name, e);
+            } catch (err) {
+                console.warn('Model onFailure hook (' + name + ') threw:', err);
+            }
         } finally {
             model.loading = false;
             model.refreshing = false;