From 13b47b576f5cb1ca0e4b1b685d99ce1f5db14e78 Mon Sep 17 00:00:00 2001 From: Alexander Nicolay Date: Wed, 12 Aug 2026 08:02:29 +0200 Subject: [PATCH] Degrade to in-memory state when the state dir is unwritable --- README.md | 8 ++++++++ docker-compose.example.yml | 4 +++- src/matebot/cli.py | 6 ++++++ src/matebot/commands.py | 5 +++-- src/matebot/state.py | 33 ++++++++++++++++++++++++++++++++- tests/test_state.py | 28 ++++++++++++++++++++++++++++ 6 files changed, 80 insertions(+), 4 deletions(-) create mode 100644 tests/test_state.py diff --git a/README.md b/README.md index 1c8f99d..dfee413 100644 --- a/README.md +++ b/README.md @@ -156,6 +156,14 @@ video with the clip on top and the chart animating below it, playhead tracking the x axis — and sends it to the chat (ready for sharing). Disable with `MATEBOT_REEL=0`; it needs ffmpeg and the `plots` extra (matplotlib). +## Troubleshooting + +**"That didn't work" on /newbag (or bags/defaults reset on restart)** - the +state dir isn't writable. In Docker the container runs as uid 1000, but a +bind-mounted `./data` created by Docker belongs to root: fix with +`mkdir -p data && sudo chown -R 1000:1000 data` and restart. The bot logs the +exact path at startup when this happens. + ## Configuration Environment variables, or the same keys in `~/.config/matebot/config.toml`: diff --git a/docker-compose.example.yml b/docker-compose.example.yml index db4d36e..4d7f446 100644 --- a/docker-compose.example.yml +++ b/docker-compose.example.yml @@ -14,6 +14,8 @@ services: # --- optional: git-backed shot journal + GitHub Pages site --- # MATEBOT_DATA_REPO: /journal volumes: - - ./data:/data # bot state (last shot, defaults) + # bot state (last shot, defaults, bean bags). The container runs as uid + # 1000 - create the dir writable first: mkdir -p data && chown 1000:1000 data + - ./data:/data # - ./journal:/journal # your data repo (git clone) # - ~/.ssh:/home/matebot/.ssh:ro # ssh key for git push diff --git a/src/matebot/cli.py b/src/matebot/cli.py index d581a32..0bdf1f9 100644 --- a/src/matebot/cli.py +++ b/src/matebot/cli.py @@ -95,6 +95,12 @@ async def _run(config: Config, *, replay: str | None, dry_run: bool) -> int: log = logging.getLogger("matebot") state = State(pathlib.Path(config.state_dir) / "state.json") + if not state.persistent: + log.error( + "state dir %s is not writable - bags/defaults/resume won't survive " + "restarts (Docker: chown -R 1000:1000 the mounted data dir)", + config.state_dir, + ) async with GaggiMateClient(config.machine_host) as client: if dry_run: diff --git a/src/matebot/commands.py b/src/matebot/commands.py index 3cdff78..552b5af 100644 --- a/src/matebot/commands.py +++ b/src/matebot/commands.py @@ -66,9 +66,10 @@ async def handle(self, text: str) -> bool: await handler() except MachineError as exc: await self.messenger.send(f"⚠️ Can't reach the machine ({exc}). Is it plugged in?") - except Exception: # noqa: BLE001 + except Exception as exc: # noqa: BLE001 log.exception("command %s failed", cmd) - await self.messenger.send("⚠️ That didn't work — check the logs.") + detail = f"{type(exc).__name__}: {exc}"[:150] + await self.messenger.send(f"⚠️ That didn't work — {detail}") return True # ------------------------------------------------------------- commands diff --git a/src/matebot/state.py b/src/matebot/state.py index ec1f094..ca5770c 100644 --- a/src/matebot/state.py +++ b/src/matebot/state.py @@ -1,18 +1,27 @@ -"""Tiny atomic JSON state file (questionnaire defaults + resume data).""" +"""Tiny atomic JSON state file (questionnaire defaults + resume data). + +An unwritable state dir (classic case: a root-owned ``./data`` bind mount +while the Docker image runs as uid 1000) must not take the bot down — state +degrades to in-memory with a loud log line instead of every write raising. +""" from __future__ import annotations import json +import logging import os import tempfile from pathlib import Path from typing import Any +log = logging.getLogger(__name__) + class State: def __init__(self, path: str | Path) -> None: self.path = Path(path) self._data: dict[str, Any] = {} + self._warned = False if self.path.exists(): try: self._data = json.loads(self.path.read_text()) @@ -30,7 +39,29 @@ def update(self, **kwargs: Any) -> None: self._data.update(kwargs) self._flush() + @property + def persistent(self) -> bool: + """True when the state file is actually writable (probes once).""" + try: + self._write() + except OSError: + return False + return True + def _flush(self) -> None: + try: + self._write() + except OSError as exc: + if not self._warned: + self._warned = True + log.error( + "state dir %s is not writable (%s) - keeping state in memory only. " + "If this runs in Docker, make the mounted state dir writable by " + "uid 1000: chown -R 1000:1000 ", + self.path.parent, exc, + ) + + def _write(self) -> None: self.path.parent.mkdir(parents=True, exist_ok=True) fd, tmp = tempfile.mkstemp(dir=self.path.parent, prefix=".state-") try: diff --git a/tests/test_state.py b/tests/test_state.py new file mode 100644 index 0000000..b6cf183 --- /dev/null +++ b/tests/test_state.py @@ -0,0 +1,28 @@ +"""State resilience: an unwritable state dir must not break the bot.""" + +import logging + +from matebot.state import State + + +def test_state_roundtrip(tmp_path): + s = State(tmp_path / "state.json") + s.set("bags", [{"name": "x"}]) + assert s.persistent + s2 = State(tmp_path / "state.json") + assert s2.get("bags") == [{"name": "x"}] + + +def test_unwritable_dir_degrades_to_memory(tmp_path, monkeypatch, caplog): + s = State(tmp_path / "state.json") + + def boom(*a, **k): + raise PermissionError(13, "Permission denied") + + monkeypatch.setattr("matebot.state.tempfile.mkstemp", boom) + assert not s.persistent + with caplog.at_level(logging.ERROR): + s.set("bags", [{"name": "x"}]) # must not raise + s.set("bags", [{"name": "y"}]) # warning only once + assert s.get("bags") == [{"name": "y"}] # in-memory state still works + assert sum("not writable" in r.message for r in caplog.records) == 1