From: Willy Tarreau Date: Wed, 29 Jul 2026 16:52:29 +0000 (+0200) Subject: DEV: patchbot: retrieve the shared state on page load, with a timeout X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=f36994ca1b7404417b6c0a618a6c4ea3f4dbcc07;p=thirdparty%2Fhaproxy.git DEV: patchbot: retrieve the shared state on page load, with a timeout I got trapped a few times reviewing patches without clicking the "Get Updates" button (not yet being used to it). After checking with AI, it's not complicate to implement a fetch timeout, so let's do that directly on page load. It also means that a reload will automatically retrieve updates. In case of failure (offline, server dead), after 2s the fetch is abandonned and the page can be used normally. For browsers that wouldn't implement the timeout, it would just act as if the button had been clicked, so likely that pressing Esc would stop it. Tested in firefox 140. --- diff --git a/dev/patchbot/README b/dev/patchbot/README index 68d67b345..4b8fd6d78 100644 --- a/dev/patchbot/README +++ b/dev/patchbot/README @@ -410,9 +410,9 @@ Using the HTML output is a bit rustic but efficient. The interface is split in buttons, the user is free to change this selection. A simple reload done by Ctrl-R will usually preserve user changes because only the document defines them (radio buttons, input boxes). But a full reload (Ctrl-Shift-R) - will lose the user's choices that were not saved to the server. In any case - the shared states are not retrieved by a full reload, the user has to click - on "Get Updates" to retrieve them. When changing a selection, the line's + will lose the user's choices that were not saved to the server. In this + case the shared states are retrieved by the automatic update performed on + load, or by clicking on "Get Updates". When changing a selection, the line's background changes to match a similar color tone, allowing to visually spot preselected patches. @@ -432,9 +432,13 @@ Using the HTML output is a bit rustic but efficient. The interface is split in When the shared review persistence is deployed, two buttons also appear at the top right of the page: - - "Get updates" retrieves the current shared state from the server. Nothing - is ever fetched automatically, not even when the page loads: it's up to - the user to click when they want to synchronize. Lines the user has not + - "Get updates" retrieves the current shared state from the server. This is + also done once automatically when the page loads, since forgetting to + click before starting a review is too easy; that automatic attempt is + given 2 seconds and is then silently abandoned, so that a page opened + from a place which cannot reach the server (or while it is down) is never + delayed nor broken, the button remaining the way to retry. Nothing else + is ever fetched behind the user's back. Lines the user has not touched are updated to reflect the shared verdicts; lines the user has already changed locally keep their local value (which will overwrite the shared one on the next save, this is intentional: last opinion wins, diff --git a/dev/patchbot/scripts/post-ai.sh b/dev/patchbot/scripts/post-ai.sh index c3b184a66..6bcda4f76 100755 --- a/dev/patchbot/scripts/post-ai.sh +++ b/dev/patchbot/scripts/post-ai.sh @@ -167,8 +167,9 @@ var branch = '$VERSION'; // server last told us, on top of which the user's edits sit; it starts // equal to the original and is only advanced by "Get updates" and by a // successful save. The local state is the DOM itself (the checked radios), -// there is no separate copy. Nothing is fetched automatically: the user -// explicitly clicks "Get updates" to retrieve the shared state. +// there is no separate copy. The reference is fetched once at page load and +// on each "Get updates" click; it is never pushed to nor fetched from the +// server behind the user's back while reviewing. var orig = []; var ref_state = []; var ref_notes = []; @@ -184,6 +185,14 @@ var cidmap = {}; var note_mode = []; var note_base = []; +// Timeout in milliseconds of the automatic update performed at page load. +// It exists only so that an unreachable server (the page is also read from +// places which cannot reach it) delays nothing: the load-time fetch is a +// convenience, the user did not ask for it, hence it must never make the +// page look stuck. The manual "Get updates" has no timeout: there the user +// explicitly asked and is willing to wait. +var autofetch_ms = 2000; + // SDBM hash (h = c + h * 65599) of a string's UTF-8 bytes, as 8 hex chars; // the concurrency token sent with a note replacement. Must match the // server's C version; the small multiplier keeps the 32-bit state exact @@ -368,8 +377,12 @@ function apply_ref(list) { updt_save_btn(); } -// "Get updates" button: fetches the current shared state from the server -function fetch_ref() { +// "Get updates" button: fetches the current shared state from the server. +// Also called once at page load with set, in which case the request +// is aborted after autofetch_ms and its failure is reported as a hint to +// use the button rather than as an error. +function fetch_ref(auto) { + var ctl = null, tmo = 0; var i, el; if (!branch) @@ -389,11 +402,26 @@ function fetch_ref() { cancel_note(i); } - sync_msg("fetching..."); - fetch(cgi_url + "?branch=" + branch) + if (auto && typeof AbortController != "undefined") { + ctl = new AbortController(); + tmo = setTimeout(function() { ctl.abort(); }, autofetch_ms); + } + + sync_msg(auto ? "getting updates..." : "fetching..."); + fetch(cgi_url + "?branch=" + branch, ctl ? { signal: ctl.signal } : {}) .then(function(r) { if (!r.ok) throw 0; return r.json(); }) - .then(function(list) { apply_ref(list); sync_msg("reference updated"); }) - .catch(function() { sync_msg("fetch failed (server unreachable?)"); }); + .then(function(list) { + if (tmo) + clearTimeout(tmo); + apply_ref(list); + sync_msg("reference updated"); + }) + .catch(function() { + if (tmo) + clearTimeout(tmo); + sync_msg(auto ? "no updates retrieved (server unreachable?), use Get updates" : + "fetch failed (server unreachable?)"); + }); } // shows/hides the per-line note links according to the edition mode and to @@ -972,4 +1000,11 @@ echo "

Output:

" echo "" echo "

" echo "" +# the shared state is fetched right away so that a page left open or reloaded +# always shows the current review; forgetting to click "Get updates" is far +# too easy. It is only a time-boxed attempt (see autofetch_ms), the button +# remains the way to retry when the server is not reachable. +if [ -n "$VERSION" ]; then + echo "" +fi echo ""