kiwix: a corrupt zim must not bring down the whole server

A corrupt or partial *.zim (e.g. an interrupted download) is rejected by
kiwix-manage ("Cannot add zim ..."). start_kiwix_serve.sh could then leave
Kiwix down and reported a misleading "Could not find any Zim files" — and
if the bad zim was the only one, exit 2 killed the service with no restart.

- Import each zim independently; log and skip a failed add so the remaining
  valid zims are still served.
- Always ensure a library.xml exists (empty if needed) and exec kiwix-serve
  unconditionally, so a corrupt/absent zim never stops the server starting.
- Fix the mount-wait loop, which never incremented COUNT (infinite wait
  instead of a 30s timeout).
- Add MEDIA_DIRECTORY override so the script is testable.
- Add Restart=on-failure to wrolpi-kiwix.service (matches wrolpi-api.service)
  so transient failures self-heal.
- New test harness runs the script with fake kiwix-manage/kiwix-serve shims.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

etc/raspberrypios/wrolpi-kiwix.service

@@ -7,6 +7,7 @@ ExecStart=/opt/wrolpi/scripts/start_kiwix_serve.sh
 Group=wrolpi
 User=wrolpi
 WorkingDirectory=/opt/wrolpi/
+Restart=on-failure
 # Below the API, but above the Map.
 Nice=8
 

modules/zim/test/test_start_kiwix_serve.py

@@ -0,0 +1,125 @@
+"""Tests for scripts/start_kiwix_serve.sh.
+
+The script imports every *.zim in the Zim directory into a Kiwix library, then
+starts kiwix-serve.  These tests exercise the script directly with fake
+`kiwix-manage`/`kiwix-serve` binaries on PATH so they need no real Kiwix
+installation.  The key invariant: a single corrupt/incomplete zim must never
+prevent the remaining valid zims from being served, nor stop the server from
+starting.
+"""
+import os
+import pathlib
+import stat
+import subprocess
+
+import pytest
+
+SCRIPT = pathlib.Path(__file__).parents[3] / 'scripts' / 'start_kiwix_serve.sh'
+
+# Fake kiwix-manage: refuses any zim whose contents contain "CORRUPT" (mimicking
+# kiwix-manage's "Cannot add zim ..." rejection of partial/corrupt files),
+# otherwise appends a <book> entry to the library file.
+FAKE_KIWIX_MANAGE = """#!/usr/bin/env bash
+library="$1"; shift
+[ "$1" = "add" ] && shift
+zim="$1"
+if grep -q CORRUPT "${zim}" 2>/dev/null; then
+  echo "Cannot add zim ${zim}" >&2
+  exit 1
+fi
+if [ ! -f "${library}" ]; then
+  printf '<?xml version="1.0" encoding="UTF-8"?>\\n<library version="20110515">\\n' > "${library}"
+fi
+printf '  <book path="%s"/>\\n' "${zim}" >> "${library}"
+exit 0
+"""
+
+# Fake kiwix-serve: records that it was started (and with which library) so the
+# test can assert the server came up, then exits 0.
+FAKE_KIWIX_SERVE = """#!/usr/bin/env bash
+library=""
+while [ $# -gt 0 ]; do
+  case "$1" in
+    --library) library="$2"; shift 2;;
+    *) shift;;
+  esac
+done
+echo "${library}" > "${STARTED_MARKER}"
+exit 0
+"""
+
+
+@pytest.fixture
+def kiwix_env(tmp_path):
+    """Set up a fake media directory and fake kiwix binaries on PATH."""
+    media = tmp_path / 'media'
+    zims = media / 'zims'
+    zims.mkdir(parents=True)
+
+    bin_dir = tmp_path / 'bin'
+    bin_dir.mkdir()
+    for name, body in (('kiwix-manage', FAKE_KIWIX_MANAGE), ('kiwix-serve', FAKE_KIWIX_SERVE)):
+        p = bin_dir / name
+        p.write_text(body)
+        p.chmod(p.stat().st_mode | stat.S_IEXEC | stat.S_IXGRP | stat.S_IXOTH)
+
+    started_marker = tmp_path / 'started'
+
+    def run():
+        env = {
+            **os.environ,
+            'MEDIA_DIRECTORY': str(media),
+            'PATH': f'{bin_dir}:{os.environ["PATH"]}',
+            'STARTED_MARKER': str(started_marker),
+        }
+        return subprocess.run(['bash', str(SCRIPT)], env=env, capture_output=True, text=True, timeout=60)
+
+    return dict(media=media, zims=zims, library=zims / 'library.xml', started_marker=started_marker, run=run)
+
+
+def _write_zim(path: pathlib.Path, corrupt: bool = False):
+    path.write_text('CORRUPT' if corrupt else 'ZIMDATA')
+
+
+def test_valid_zim_starts_server(kiwix_env):
+    """A valid zim is imported and the server starts."""
+    _write_zim(kiwix_env['zims'] / 'wikipedia.zim')
+
+    result = kiwix_env['run']()
+
+    assert result.returncode == 0, result.stderr
+    assert kiwix_env['started_marker'].exists(), 'kiwix-serve was not started'
+    assert 'wikipedia.zim' in kiwix_env['library'].read_text()
+
+
+def test_corrupt_zim_does_not_prevent_serving_valid_zims(kiwix_env):
+    """A corrupt zim must be skipped while the valid zims are still served."""
+    _write_zim(kiwix_env['zims'] / 'good.zim')
+    _write_zim(kiwix_env['zims'] / 'sudan_broken.zim', corrupt=True)
+
+    result = kiwix_env['run']()
+
+    assert result.returncode == 0, result.stderr
+    assert kiwix_env['started_marker'].exists(), 'kiwix-serve was not started'
+    library = kiwix_env['library'].read_text()
+    assert 'good.zim' in library
+    assert 'sudan_broken.zim' not in library
+
+
+def test_only_corrupt_zim_still_starts_server(kiwix_env):
+    """Even when the only zim is corrupt, kiwix must still start (empty library)."""
+    _write_zim(kiwix_env['zims'] / 'sudan_broken.zim', corrupt=True)
+
+    result = kiwix_env['run']()
+
+    assert result.returncode == 0, result.stderr
+    assert kiwix_env['started_marker'].exists(), 'kiwix-serve was not started'
+    assert kiwix_env['library'].exists()
+
+
+def test_no_zims_still_starts_server(kiwix_env):
+    """With no zim files at all, kiwix still starts with an empty library."""
+    result = kiwix_env['run']()
+
+    assert result.returncode == 0, result.stderr
+    assert kiwix_env['started_marker'].exists(), 'kiwix-serve was not started'

scripts/start_kiwix_serve.sh

@@ -1,30 +1,60 @@
 #! /usr/bin/env bash
 # This file imports all *.zim files in the Zim media directory, then starts kiwix-serve.
+set -u
 
-[ ! -d /media/wrolpi ] && echo "Cannot start kiwix without media directory mounted" && exit 1
+# The media directory is overridable for testing; defaults to the production mount.
+MEDIA_DIRECTORY="${MEDIA_DIRECTORY:-/media/wrolpi}"
 
-LIBRARY=/media/wrolpi/zims/library.xml
+[ ! -d "${MEDIA_DIRECTORY}" ] && echo "Cannot start kiwix without media directory mounted" && exit 1
 
-# Wait for the zims directory to be mounted.
+ZIM_DIRECTORY="${MEDIA_DIRECTORY}/zims"
+LIBRARY="${ZIM_DIRECTORY}/library.xml"
+
+# Wait up to 30 seconds for the zims directory to be mounted.
 COUNT=0
 while [ ${COUNT} -lt 30 ]; do
-  if [ -d /media/wrolpi/zims ]; then
+  if [ -d "${ZIM_DIRECTORY}" ]; then
     break
   fi
+  COUNT=$((COUNT + 1))
   sleep 1
 done
 
 # Create the zim directory, if necessary.
-[ ! -d /media/wrolpi/zims ] && mkdir /media/wrolpi/zims
-# We will replace the library file.
-rm -f ${LIBRARY}
-# Find all Zim files and import them.
-find /media/wrolpi/zims -iname '*.zim' -exec kiwix-manage ${LIBRARY} add {} \;
+[ ! -d "${ZIM_DIRECTORY}" ] && mkdir -p "${ZIM_DIRECTORY}"
 
-[ ! -f ${LIBRARY} ] && echo "Could not find any Zim files to import" && exit 2
+# We will replace the library file.
+rm -f "${LIBRARY}"
+
+# Import every *.zim.  A corrupt or partial zim is rejected by kiwix-manage
+# ("Cannot add zim ..."); it must NOT prevent the remaining valid zims from
+# being served, nor stop the server from starting.  So we add each zim
+# independently and never abort on a single failure.
+found_zim=false
+added_zim=false
+while IFS= read -r -d '' zim; do
+  found_zim=true
+  if kiwix-manage "${LIBRARY}" add "${zim}"; then
+    added_zim=true
+  else
+    echo "Skipping zim which could not be added (corrupt or incomplete?): ${zim}" >&2
+  fi
+done < <(find "${ZIM_DIRECTORY}" -iname '*.zim' -print0)
+
+if [ "${found_zim}" = false ]; then
+  echo "Could not find any Zim files to import"
+elif [ "${added_zim}" = false ]; then
+  echo "Found Zim files but none could be added (all corrupt or incomplete?); starting Kiwix with an empty library" >&2
+fi
+
+# Ensure a library file always exists so kiwix-serve can start even when there
+# are no valid zims.  A single corrupt zim must never take down the server.
+if [ ! -f "${LIBRARY}" ]; then
+  printf '<?xml version="1.0" encoding="UTF-8"?>\n<library version="20110515">\n</library>\n' > "${LIBRARY}"
+fi
 
 # Serve HTTP kiwix on 9085.  Caddy will serve HTTPS on 8085.
 # Bind all interfaces by omitting --address: kiwix-serve defaults to all
 # interfaces, and trixie's kiwix-tools 3.7.0 rejects the literal
 # "--address 0.0.0.0" ("IP address is not available on this system").
-kiwix-serve --library ${LIBRARY} --port 9085
+exec kiwix-serve --library "${LIBRARY}" --port 9085