Prevent the Tamanu sync from hanging indefinitely

CHANGES.rst

@@ -4,6 +4,7 @@ Changelog
 1.0.0
 -----
 
+- #156 Prevent the Tamanu sync from hanging indefinitely
 - #159 Use FHIR comparator for out-of-detection-limit results sent to Tamanu
 - #155 Disable notification for sample status transition to 'to_be_verified'
 - #144 Adding reference results to Tamanu FHIR Observations

scripts/sync_tamanu.py

@@ -46,6 +46,7 @@
     create_analysisrequest as create_sample
 from bika.lims.workflow import doActionFor
 from requests import ConnectionError
+from requests import Timeout
 from senaite.core.api import dtime
 from senaite.core.catalog import CLIENT_CATALOG
 from senaite.core.catalog import CONTACT_CATALOG
@@ -888,14 +889,18 @@ def main(app):
 
     # Start a session with Tamanu server
     session = TamanuSession(host)
-    logged = session.login(user, password)
+    try:
+        logged = session.login(user, password)
+    except (ConnectionError, Timeout) as e:
+        # a stalled/unreachable server must not hang the run; exit cleanly
+        connection_error(str(e))
     if not logged:
         error("Cannot login, wrong credentials")
 
     try:
         # Call the sync function
         sync_func(session, since)
-    except ConnectionError as e:
+    except (ConnectionError, Timeout) as e:
         connection_error(str(e))
 
     if args.dry:

src/bes/lims/tamanu/session.py

@@ -43,20 +43,33 @@
     ("Content-Type", "application/json"),
 )
 
+# Default (connect, read) timeouts in seconds for every Tamanu HTTP call.
+# Without a timeout, `requests` blocks forever on a stalled socket (server
+# overloaded, half-open connection, no RST), which hangs the whole sync run.
+DEFAULT_TIMEOUT = (10, 60)
+
+# Tighter (connect, read) timeout for login. It is the first call of every
+# run and the gate for everything after it, so we fail fast rather than let a
+# stalled auth request block the run indefinitely.
+LOGIN_TIMEOUT = (10, 30)
+
 
 class TamanuSession(object):
 
     token = "unk"
     _auth = None
 
-    def __init__(self, host):
+    def __init__(self, host, timeout=DEFAULT_TIMEOUT):
         self.host = host
+        # (connect, read) timeout applied to every request unless the caller
+        # passes an explicit `timeout` kwarg to get()/post()
+        self.timeout = timeout
 
     def login(self, email, password):
         # TODO remove _auth
         self._auth = (email, password)
         auth = dict(email=email, password=password)
-        resp = self.post("login", payload=auth)
+        resp = self.post("login", payload=auth, timeout=LOGIN_TIMEOUT)
         self.token = resp.json().get("token")
         if self.token:
             return True
@@ -93,6 +106,9 @@ def post(self, endpoint, payload, **kwargs):
 
         kwargs["headers"] = headers
 
+        # bound the request so a stalled socket cannot block the run forever
+        kwargs.setdefault("timeout", self.timeout)
+
         # Send the POST request
         logger.info("[POST] {}".format(url))
         logger.debug("[POST PAYLOAD] {}".format(repr(payload)))
@@ -117,6 +133,9 @@ def get(self, endpoint, params=None, **kwargs):
         headers["Authorization"] = "Bearer {}".format(self.token)
         kwargs["headers"] = headers
 
+        # bound the request so a stalled socket cannot block the run forever
+        kwargs.setdefault("timeout", self.timeout)
+
         # do the GET request
         logger.info("[GET] {} (params={})".format(url, repr(params)))
         resp = requests.get(url, params=params, **kwargs)

templates/sync_tamanu.in

@@ -1,21 +1,69 @@
 #!/usr/bin/env bash
+#
+# Tamanu sync wrapper.
+#
+#   * One lock for the whole run (no interleave window between steps).
+#   * A held lock is surfaced (exit 75 + log line), not swallowed as success.
+#   * Each step has a hard wall-clock cap via `timeout`, so a hung request is
+#     reaped, the lock is released, and the next cron cycle self-recovers.
+#   * `set -o pipefail` propagates the real step exit code through the `tee`
+#     pipe (a bare `... |& tee` always returns tee's 0), so an external monitor
+#     (e.g. cronitor exec) reflects failures instead of false greens.
+#
+# NOTE: the per-step `timeout` is the safety net. The clean fix is the HTTP
+# timeouts in bes.lims/tamanu/session.py, which let the script exit gracefully
+# via its own ConnectionError handler instead of being SIGKILLed mid-run.
+#
+# Bash refs use the plain $NAME form on purpose. The brace form (dollar +
+# braces) collides with buildout's section:option substitution that renders
+# this template, so it is avoided throughout.
+
+set -uo pipefail
 
 SINCE_SERVICE_REQUEST=${sync_tamanu:sample_since}
 SINCE_PATIENTS=${sync_tamanu:patients_since}
 
 BIN_DIR=${buildout:directory}/bin
 SRC_DIR=${buildout:directory}/src/bes.lims/scripts
 ZEO_DIR=${buildout:var-dir}/${sync_tamanu:zeoclient}
-FLOCK="flock -n /tmp/sync_tamanu.lock"
+ZEOCLIENT=${sync_tamanu:zeoclient}
+LOCK=/tmp/sync_tamanu.lock
 
 # Tamanu host and credentials are read from the control panel
 # (@@tamanu-controlpanel). Configure them there before running this script.
 
+# Acquire the lock once for the whole run. A previous run still holding it is
+# itself the signal that something overran/wedged -> surface it (non-zero).
+exec 9>"$LOCK" || exit 3
+if ! flock -n 9; then
+    echo "$(date -Is) [SKIP] previous sync still holds the lock" \
+        | tee -a "$ZEO_DIR/event.log"
+    exit 75
+fi
+
+# Per-step wall-clock cap (SIGTERM, then SIGKILL after 30s if ignored).
+# Budget sums well under the cron cadence so a stuck step is always reaped
+# before the next run fires (no pile-ups, lock always freed).
+status=0
+
 # Synchronize service requests
-$FLOCK $BIN_DIR/${sync_tamanu:zeoclient} run $SRC_DIR/sync_tamanu.py -r ServiceRequest -s $SINCE_SERVICE_REQUEST -c $ZEO_DIR |& tee -a $ZEO_DIR/event.log
+timeout --signal=TERM --kill-after=30s 100s \
+    "$BIN_DIR/$ZEOCLIENT" run "$SRC_DIR/sync_tamanu.py" \
+    -r ServiceRequest -s "$SINCE_SERVICE_REQUEST" -c "$ZEO_DIR" 2>&1 \
+    | tee -a "$ZEO_DIR/event.log"
+rc=$?; [ "$rc" -ne 0 ] && status=$rc
 
 # Synchronize patients
-$FLOCK $BIN_DIR/${sync_tamanu:zeoclient} run $SRC_DIR/sync_tamanu.py -r Patient -s $SINCE_PATIENTS -c $ZEO_DIR |& tee -a $ZEO_DIR/event.log
+timeout --signal=TERM --kill-after=30s 100s \
+    "$BIN_DIR/$ZEOCLIENT" run "$SRC_DIR/sync_tamanu.py" \
+    -r Patient -s "$SINCE_PATIENTS" -c "$ZEO_DIR" 2>&1 \
+    | tee -a "$ZEO_DIR/event.log"
+rc=$?; [ "$rc" -ne 0 ] && status=$rc
 
 # Run Tamanu specific tasks
-$FLOCK $BIN_DIR/${sync_tamanu:zeoclient} run $SRC_DIR/exec_tamanu_tasks.py -m 10 |& tee -a $ZEO_DIR/event.log
+timeout --signal=TERM --kill-after=30s 45s \
+    "$BIN_DIR/$ZEOCLIENT" run "$SRC_DIR/exec_tamanu_tasks.py" -m 10 2>&1 \
+    | tee -a "$ZEO_DIR/event.log"
+rc=$?; [ "$rc" -ne 0 ] && status=$rc
+
+exit "$status"