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 68d67b3454e5f65271b4695381af15bc8745b8d1..4b8fd6d78540cbc37c903b9685062af23df8c073 100644 (file)
--- 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 c3b184a6615aeb42b79a6ec23193f29e595b4093..6bcda4f76edc2c72d7d0f545ae54a6a8c155dd27 100755 (executable)
--- 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 <auto> 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 "<H3>Output:</H3>"
 echo "<textarea cols=120 rows=10 id='output'></textarea>"
 echo "<P/>"
 echo "<script type='text/javascript'>nb_patches=$seq_num; review=$review; init_review(); init_ref(); updt_table(0); updt_output();</script>"
+# 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 "<script type='text/javascript'>fetch_ref(1);</script>"
+fi
 echo "</BODY></HTML>"