Changeset 5462bab in Klonkt

Timestamp:

07/31/2026 01:52:04 PM (6 weeks ago)

Author:

Robin <roboburr@…>

Branches:

main

Children:

f434294

Parents:

ccaa530

Message:

Fix 500 op de Uiterlijk-pagina na een halve update (boiert.eu)

Bart kreeg een 500 op /admin/sites/<slug>/edit. Oorzaak was een gemengde
staat: zijn klonkt-update was gegenereerd voor de oude single-service-layout
en herstart nog klonkt.service, de unit die de migratie juist heeft
uitgezet. De code op schijf werd dus wel bijgewerkt, maar het draaiende
proces nooit herstart. De oude route rendert dan het nieuwe template, dat
een variabele verwacht die de oude route niet meegeeft: ReferenceError, en
Express cachet het gecompileerde template tot de volgende herstart.

Drie lagen gefixt:

1. Het aliasveld degradeert bij een ontbrekende variabele naar leeg in plaats van de hele pagina mee te nemen (typeof-guard, zelfde patroon als auth-register). Een deploy-moment mag nooit een 500 opleveren.
2. De nieuw-site-form gaf dezelfde ReferenceError ook met volledig nieuwe code: de /new-route rendert hetzelfde template maar gaf apAliases niet mee. Lokaal gereproduceerd en bevestigd gefixt (beide pagina's 200).
3. De wortel: een gedeeld script dat /usr/local/bin/klonkt-update herschrijft voor de actuele layout. De migratie draait het voortaan zelf, install.sh genereert de updater er ook mee (kan nooit meer uiteenlopen), en al gemigreerde servers repareren het met een los commando.

Changed files:
src/routes/admin-sites.js

• /new geeft apAliases mee aan het template

src/views/pages/admin-site-edit.ejs

• typeof-guard op apAliases met uitleg waarom

scripts/klonkt-migrate-data.sh

• herschrijft de updater na de unit-omschakeling; waarschuwt als het script in een oudere checkout ontbreekt

scripts/install.sh

• inline updater-generatie vervangen door het gedeelde script

deploy/MULTI-INSTANCE.md

• reparatie-instructie voor servers die voor deze fix zijn gemigreerd

New file:
scripts/klonkt-refresh-updater.sh

• idempotent; detecteert de branch uit de checkout; herstart alle klonkt@<slug>-instances, of klonkt.service als er geen zijn

-robo
Co-Authored-By: Claude Fable 5 <noreply@…>

Files:

• deploy/MULTI-INSTANCE.md (modified) (1 diff)
• scripts/install.sh (modified) (1 diff)
• scripts/klonkt-migrate-data.sh (modified) (1 diff)
• scripts/klonkt-refresh-updater.sh (added)
• src/routes/admin-sites.js (modified) (1 diff)
• src/views/pages/admin-site-edit.ejs (modified) (1 diff)

deploy/MULTI-INSTANCE.md

@@ -133 +133 @@
 ```
 
+**Migrated before August 2026?** Then your `klonkt-update` still restarts the
+retired `klonkt.service`: the code updates but the running process never
+follows, and after the next update the site can 500 on pages whose template
+and route no longer match. Repair it once:
+
+```bash
+sudo systemctl restart klonkt@<slug>                        # load the current code now
+sudo bash /opt/klonkt/scripts/klonkt-refresh-updater.sh     # fix the updater for good
+```
+
+The migration script does this by itself nowadays.
+
 That pulls the code once into `/opt/klonkt`, reinstalls dependencies only when
 `package-lock.json` changed, and restarts every instance it finds under

scripts/install.sh

@@ -305 +305 @@
 
 log "Update command 'klonkt-update'…"
-cat > /usr/local/bin/klonkt-update <<EOF
-#!/usr/bin/env bash
-set -euo pipefail
-D="${KLONKT_DIR}"
-B=\$(runuser -u ${KLONKT_USER} -- git -C "\$D" rev-parse HEAD 2>/dev/null || true)
-runuser -u ${KLONKT_USER} -- git -C "\$D" fetch --depth 1 origin ${KLONKT_BRANCH}
-runuser -u ${KLONKT_USER} -- git -C "\$D" checkout -qf -B ${KLONKT_BRANCH} FETCH_HEAD
-A=\$(runuser -u ${KLONKT_USER} -- git -C "\$D" rev-parse HEAD)
-if [ "\$B" = "\$A" ]; then
-  echo "Klonkt is already up to date (\$A) — nothing to do."
-  exit 0
-fi
-if ! runuser -u ${KLONKT_USER} -- git -C "\$D" diff --quiet "\$B" "\$A" -- package-lock.json 2>/dev/null; then
-  runuser -u ${KLONKT_USER} -- env HOME="\$D" bash -c "cd '\$D' && npm ci --omit=dev"
-fi
-# Restart every instance. Each directory under the data root with an .env is one
-# instance sharing this checkout. An install that has not been split yet has no
-# such directories and still runs the single klonkt.service.
-N=0
-for d in ${KLONKT_DATA_ROOT}/*/; do
-  [ -f "\$d/.env" ] || continue
-  s=\$(basename "\$d")
-  systemctl restart "klonkt@\$s" && N=\$((N+1))
-done
-if [ "\$N" = 0 ]; then
-  systemctl restart klonkt
-  echo "Klonkt updated (\$A) + restarted."
-else
-  echo "Klonkt updated (\$A) + restarted \$N instance(s)."
-fi
-EOF
-chmod +x /usr/local/bin/klonkt-update
+# Generated by the shared script so an install and a later layout migration
+# can never drift apart on what the updater restarts.
+KLONKT_DIR="$KLONKT_DIR" KLONKT_USER="$KLONKT_USER" \
+KLONKT_DATA_ROOT="$KLONKT_DATA_ROOT" KLONKT_BRANCH="$KLONKT_BRANCH" \
+  bash "$KLONKT_DIR/scripts/klonkt-refresh-updater.sh"
 ok "klonkt-update"
 

scripts/klonkt-migrate-data.sh

@@ -135 +135 @@
 run "systemctl enable --now 'klonkt@$SLUG'"
 
+step "Rewriting klonkt-update for the new layout"
+# The installer generated an updater that restarts klonkt.service — which we
+# just retired. Left alone it would keep updating the code while never
+# restarting the real process: half old, half new, and a 500 with no obvious
+# cause. Rewrite it so it restarts every klonkt@<slug> instead.
+if [ -f "$KLONKT_DIR/scripts/klonkt-refresh-updater.sh" ]; then
+  run "KLONKT_DIR='$KLONKT_DIR' KLONKT_USER='$KLONKT_USER' KLONKT_DATA_ROOT='$DATA_ROOT' bash '$KLONKT_DIR/scripts/klonkt-refresh-updater.sh'"
+else
+  say "WARNING: scripts/klonkt-refresh-updater.sh missing in this checkout."
+  say "         Update the code and run it once by hand, or every klonkt-update"
+  say "         from now on will update code WITHOUT restarting the process."
+fi
+
 step "Verifying"
 if [ "$DRY" = 1 ]; then

src/routes/admin-sites.js

@@ -211 +211 @@
     platforms: listPlatforms(),
     parsedLinks: [],
+    apAliases: '',
     error: null,
   });

src/views/pages/admin-site-edit.ejs

@@ -217 +217 @@
       <p class="form-hint"><%= t('asite.aliases_hint') %></p>
       <label>
-        <textarea name="ap_aliases" rows="3" placeholder="@oud@mastodon.social&#10;https://andere-klonkt.example/ap/users/naam"><%= apAliases %></textarea>
+<%# typeof-guard on purpose: during a deploy an old route can render this new
+    template for one request (view cache fills on first hit). A missing local
+    must degrade to an empty field, not take the whole page down with a 500. %>
+        <textarea name="ap_aliases" rows="3" placeholder="@oud@mastodon.social&#10;https://andere-klonkt.example/ap/users/naam"><%= typeof apAliases !== 'undefined' ? apAliases : '' %></textarea>
       </label>
     </fieldset>