fix(konta): awaria magazynu tłumaczy się zamiast dawać gołe 500
Testy / Testy warstwy logicznej (silnik) (pull_request) Successful in 10m29s
Testy / Testy warstwy prezentacji (dostęp do baz) (pull_request) Failing after 4m55s
Testy / Testy warstwy bazodanowej (ochrona baz) (pull_request) Successful in 9m28s
Testy / Build obrazu silnika B (swisseph) (pull_request) Successful in 17s
Testy / Kontrola składni wszystkich warstw (pull_request) Successful in 8s
Testy / Testy warstwy logicznej (silnik) (push) Successful in 10m37s
Testy / Testy warstwy prezentacji (dostęp do baz) (push) Failing after 4m53s
Testy / Testy warstwy bazodanowej (ochrona baz) (push) Successful in 9m29s
Testy / Build obrazu silnika B (swisseph) (push) Successful in 19s
Testy / Kontrola składni wszystkich warstw (push) Successful in 8s
Testy / Testy warstwy logicznej (silnik) (pull_request) Successful in 10m29s
Testy / Testy warstwy prezentacji (dostęp do baz) (pull_request) Failing after 4m55s
Testy / Testy warstwy bazodanowej (ochrona baz) (pull_request) Successful in 9m28s
Testy / Build obrazu silnika B (swisseph) (pull_request) Successful in 17s
Testy / Kontrola składni wszystkich warstw (pull_request) Successful in 8s
Testy / Testy warstwy logicznej (silnik) (push) Successful in 10m37s
Testy / Testy warstwy prezentacji (dostęp do baz) (push) Failing after 4m53s
Testy / Testy warstwy bazodanowej (ochrona baz) (push) Successful in 9m29s
Testy / Build obrazu silnika B (swisseph) (push) Successful in 19s
Testy / Kontrola składni wszystkich warstw (push) Successful in 8s
Ekran „Konta" wywalał się na produkcji błędem 500 bez słowa wyjaśnienia. Odtworzone lokalnie: `_read()` łapał wyłącznie brak pliku i zły JSON, więc każdy inny błąd systemu plików — a na udziale NFS to głównie prawa — leciał na wierzch jako nieobsłużony wyjątek. To jest szczególnie zły sposób na awarię AKURAT TUTAJ: ekran kont jest jedynym miejscem, z którego administrator może taki problem naprawić, a gołe 500 nie mówi mu ani co, ani gdzie. Teraz każdy błąd magazynu ma twarz: osobny wyjątek AccountsUnavailable niosący ŚCIEŻKĘ i powód z systemu operacyjnego, plus podpowiedź najczęstszej przyczyny (prawa katalogu na udziale albo wolumen zamontowany tylko do odczytu). Strona renderuje się normalnie z tym komunikatem u góry. Objęte są wszystkie cztery drogi zapisu, a nie tylko odczyt. W szczególności mkstemp: przy katalogu tylko do odczytu wywala się ONO pierwsze, jeszcze zanim dojdzie do zapisu i podmiany — więc obudowanie samego os.replace nic by nie dało (złapane testem, nie przeglądem kodu). USZKODZONY PLIK NIE JEST NADPISYWANY. Wcześniej niepoprawny JSON dawał pusty zbiór kont, co przy pierwszym zapisie skasowałoby WSZYSTKIE konta bez śladu. Teraz to odmowa z komunikatem — plik zostaje nietknięty, a test tego pilnuje. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -28,6 +28,16 @@ from app import features
|
||||
_lock = threading.Lock()
|
||||
|
||||
|
||||
class AccountsUnavailable(RuntimeError):
|
||||
"""Pliku kont nie da się odczytać albo zapisać — problem z magazynem.
|
||||
|
||||
Osobny wyjątek, bo to NIE JEST błąd danych: konta mogą być w porządku, tylko
|
||||
katalog jest niedostępny (złe prawa na udziale, nieprzemontowany wolumen,
|
||||
brak miejsca). Ekran ma wtedy POWIEDZIEĆ, co i gdzie jest nie tak — gołe 500
|
||||
zostawia administratora z niczym, a to jedyny ekran, z którego może to
|
||||
naprawić."""
|
||||
|
||||
|
||||
def store_path() -> str:
|
||||
"""Ścieżka pliku kont.
|
||||
|
||||
@@ -38,27 +48,62 @@ def store_path() -> str:
|
||||
|
||||
|
||||
def _read() -> dict:
|
||||
path = store_path()
|
||||
try:
|
||||
with open(store_path(), encoding="utf-8") as fh:
|
||||
with open(path, encoding="utf-8") as fh:
|
||||
data = json.load(fh)
|
||||
except (FileNotFoundError, json.JSONDecodeError):
|
||||
except FileNotFoundError:
|
||||
# Pierwsze uruchomienie: pliku jeszcze nie ma i to jest normalne.
|
||||
return {"users": {}}
|
||||
except json.JSONDecodeError:
|
||||
# Plik jest, ale nieczytelny. NIE nadpisujemy go pustym zbiorem —
|
||||
# to skasowałoby wszystkie konta przy pierwszym zapisie.
|
||||
raise AccountsUnavailable(
|
||||
f"Plik kont {path} jest uszkodzony (niepoprawny JSON). "
|
||||
f"Nic nie zostało zmienione — napraw albo usuń plik ręcznie.") from None
|
||||
except OSError as e:
|
||||
raise AccountsUnavailable(
|
||||
f"Nie mogę odczytać pliku kont {path}: {e.strerror or e}. "
|
||||
f"Najczęstsza przyczyna: prawa do katalogu na udziale albo "
|
||||
f"niezamontowany wolumen.") from e
|
||||
users = data.get("users")
|
||||
return {"users": users if isinstance(users, dict) else {}}
|
||||
|
||||
|
||||
def _write(data: dict) -> None:
|
||||
path = store_path()
|
||||
os.makedirs(os.path.dirname(path) or ".", exist_ok=True)
|
||||
try:
|
||||
os.makedirs(os.path.dirname(path) or ".", exist_ok=True)
|
||||
except OSError as e:
|
||||
raise AccountsUnavailable(
|
||||
f"Nie mogę utworzyć katalogu na plik kont ({os.path.dirname(path)}): "
|
||||
f"{e.strerror or e}.") from e
|
||||
# Plik tymczasowy MUSI powstać w tym samym katalogu — os.replace jest
|
||||
# niepodzielne tylko w obrębie jednego systemu plików.
|
||||
fd, tmp = tempfile.mkstemp(dir=os.path.dirname(path) or ".", suffix=".tmp")
|
||||
# mkstemp też musi być w klamrze: przy katalogu tylko do odczytu wywala się
|
||||
# ONO pierwsze, jeszcze zanim dojdzie do zapisu i podmiany.
|
||||
try:
|
||||
fd, tmp = tempfile.mkstemp(dir=os.path.dirname(path) or ".", suffix=".tmp")
|
||||
except OSError as e:
|
||||
raise AccountsUnavailable(
|
||||
f"Nie mogę zapisać pliku kont {path}: {e.strerror or e}. "
|
||||
f"Najczęstsza przyczyna: udział zamontowany tylko do odczytu albo "
|
||||
f"prawa katalogu. Nic nie zostało zmienione.") from e
|
||||
try:
|
||||
with os.fdopen(fd, "w", encoding="utf-8") as fh:
|
||||
json.dump(data, fh, ensure_ascii=False, indent=1, sort_keys=True)
|
||||
fh.flush()
|
||||
os.fsync(fh.fileno())
|
||||
os.replace(tmp, path)
|
||||
except OSError as e:
|
||||
try:
|
||||
os.unlink(tmp)
|
||||
except OSError:
|
||||
pass
|
||||
raise AccountsUnavailable(
|
||||
f"Nie mogę zapisać pliku kont {path}: {e.strerror or e}. "
|
||||
f"Najczęstsza przyczyna: udział zamontowany tylko do odczytu albo "
|
||||
f"prawa katalogu. Nic nie zostało zmienione.") from e
|
||||
except BaseException:
|
||||
try:
|
||||
os.unlink(tmp)
|
||||
|
||||
@@ -677,8 +677,15 @@ def timezone_lookup(lat: float, lon: float, date: str = "", time: str = "12:00")
|
||||
# dla całej aplikacji, sprawdzana testem, który przechodzi po WSZYSTKICH trasach.
|
||||
|
||||
def _accounts_context(request: Request, error: str = "", done: str = "") -> dict:
|
||||
# Problem z magazynem NIE MOŻE dawać gołego 500: to jedyny ekran, z którego
|
||||
# administrator może go naprawić, więc musi na nim przeczytać, co i gdzie
|
||||
# jest nie tak.
|
||||
try:
|
||||
users = accounts_store.all_users()
|
||||
except accounts_store.AccountsUnavailable as e:
|
||||
users, error = {}, str(e)
|
||||
return {
|
||||
"users": accounts_store.all_users(),
|
||||
"users": users,
|
||||
"catalog": features.ALL,
|
||||
"screens": features.SCREENS,
|
||||
"extras": features.EXTRAS,
|
||||
@@ -708,7 +715,7 @@ def accounts_create(request: Request, login: str = Form(""), password: str = For
|
||||
note: str = Form(""), granted: list[str] = Form([])):
|
||||
try:
|
||||
accounts_store.create(login, password, granted, note)
|
||||
except ValueError as e:
|
||||
except (ValueError, accounts_store.AccountsUnavailable) as e:
|
||||
return _accounts_redirect(error=str(e))
|
||||
return _accounts_redirect(done=f"Założono konto „{login.strip()}”.")
|
||||
|
||||
@@ -718,7 +725,7 @@ def accounts_update(request: Request, login: str = Form(...), password: str = Fo
|
||||
note: str = Form(""), granted: list[str] = Form([])):
|
||||
try:
|
||||
accounts_store.update(login, granted=granted, password=password, note=note)
|
||||
except ValueError as e:
|
||||
except (ValueError, accounts_store.AccountsUnavailable) as e:
|
||||
return _accounts_redirect(error=str(e))
|
||||
changed = "uprawnienia i hasło" if password.strip() else "uprawnienia"
|
||||
return _accounts_redirect(done=f"Zapisano {changed} konta „{login}”.")
|
||||
@@ -728,7 +735,7 @@ def accounts_update(request: Request, login: str = Form(...), password: str = Fo
|
||||
def accounts_delete(request: Request, login: str = Form(...)):
|
||||
try:
|
||||
accounts_store.delete(login)
|
||||
except ValueError as e:
|
||||
except (ValueError, accounts_store.AccountsUnavailable) as e:
|
||||
return _accounts_redirect(error=str(e))
|
||||
return _accounts_redirect(done=f"Skasowano konto „{login}”.")
|
||||
|
||||
|
||||
@@ -309,3 +309,62 @@ def test_deleting_an_account_cannot_touch_the_administrator(env, monkeypatch):
|
||||
assert r.status_code == 303
|
||||
assert store.exists("ala"), "kasowanie nieistniejącego konta ruszyło inne"
|
||||
assert c.get("/", headers=admin).status_code == 200
|
||||
|
||||
|
||||
# ── awaria magazynu kont ─────────────────────────────────────────────────
|
||||
# Ekran kont to JEDYNE miejsce, z którego administrator może naprawić problem
|
||||
# z magazynem — więc musi na nim przeczytać, co i gdzie jest nie tak. Gołe 500
|
||||
# (tak było na pierwszym wdrożeniu) zostawia go z niczym.
|
||||
|
||||
def _unreadable(tmp_path, monkeypatch, mode):
|
||||
d = tmp_path / "stan"
|
||||
d.mkdir()
|
||||
monkeypatch.setenv("ACCOUNTS_FILE", str(d / "accounts.json"))
|
||||
d.chmod(mode)
|
||||
return d
|
||||
|
||||
|
||||
def test_unreadable_store_explains_itself_instead_of_500(env, tmp_path, monkeypatch):
|
||||
d = _unreadable(tmp_path, monkeypatch, 0o000)
|
||||
try:
|
||||
c = _client(monkeypatch)
|
||||
r = c.get("/accounts", headers=_auth("szef", "tajne-szefa"))
|
||||
assert r.status_code == 200, "problem z magazynem nie może wywalać strony"
|
||||
assert "Nie mogę odczytać pliku kont" in r.text
|
||||
assert str(d / "accounts.json") in r.text, "komunikat ma podać ŚCIEŻKĘ"
|
||||
finally:
|
||||
d.chmod(0o755)
|
||||
|
||||
|
||||
def test_read_only_store_refuses_to_save_with_a_reason(env, tmp_path, monkeypatch):
|
||||
d = _unreadable(tmp_path, monkeypatch, 0o555)
|
||||
try:
|
||||
c = _client(monkeypatch)
|
||||
r = c.post("/accounts/create", headers=_auth("szef", "tajne-szefa"),
|
||||
follow_redirects=False,
|
||||
data={"login": "ala", "password": "x", "granted": ["chart"]})
|
||||
assert r.status_code == 303
|
||||
# Komunikat jedzie w parametrze zapytania, więc jest zakodowany —
|
||||
# porównanie na surowym nagłówku sprawdzałoby procenty, nie treść.
|
||||
from urllib.parse import unquote_plus
|
||||
|
||||
assert "Nie mogę zapisać pliku kont" in unquote_plus(r.headers["location"])
|
||||
finally:
|
||||
d.chmod(0o755)
|
||||
|
||||
|
||||
def test_a_corrupt_file_is_never_silently_overwritten(env, tmp_path, monkeypatch):
|
||||
"""Nadpisanie uszkodzonego pliku pustym zbiorem skasowałoby WSZYSTKIE konta.
|
||||
Lepiej odmówić i powiedzieć, co jest nie tak."""
|
||||
path = tmp_path / "accounts.json"
|
||||
path.write_text("{to nie jest json", encoding="utf-8")
|
||||
monkeypatch.setenv("ACCOUNTS_FILE", str(path))
|
||||
|
||||
c = _client(monkeypatch)
|
||||
r = c.get("/accounts", headers=_auth("szef", "tajne-szefa"))
|
||||
assert r.status_code == 200 and "uszkodzony" in r.text
|
||||
|
||||
c.post("/accounts/create", headers=_auth("szef", "tajne-szefa"),
|
||||
follow_redirects=False, data={"login": "ala", "password": "x"})
|
||||
assert path.read_text(encoding="utf-8") == "{to nie jest json", \
|
||||
"uszkodzony plik został nadpisany — konta by zniknęły"
|
||||
|
||||
Reference in New Issue
Block a user