diff --git a/.claude/skills/anthias-release/SKILL.md b/.claude/skills/anthias-release/SKILL.md index fba765355..e587459c9 100644 --- a/.claude/skills/anthias-release/SKILL.md +++ b/.claude/skills/anthias-release/SKILL.md @@ -64,6 +64,8 @@ where known; OPEN items are flagged. ``/`` are the public - **environment tag vs baggage mismatch = test/CI stack leaking into prod Sentry, not a device failure.** An event tagged `environment=production` whose Celery task headers carry `baggage: sentry-environment=test` means the *producer* ran in a test stack but the *worker* fell back to production defaults. Root cause (event in normalize_video_asset, "no such table: assets"): `anthias-celery` in docker-compose.test.yml lacked `ENVIRONMENT=test`/`ANTHIAS_TEST_DB_PATH`. settings.py defaults BOTH the DSN and the DB path off `ENVIRONMENT`, so any compose service missing the var silently becomes "production". Fixed #3013 by sharing the env block via a YAML anchor. When triaging, compare the event's `environment` tag against `sentry-environment` in the task baggage and the hostnames. - **Device context tags (PR #3021 / #3025).** Events are sent from inside containers, so Sentry's stock OS context never sees the host. #3021 tags `device_type` (build-time ENV), `kernel_release`/`kernel_machine` (containers share the host kernel), `board_model` (`/proc/device-tree/model`, absent on x86). #3025 adds `balena: true|false`. Keep `settings.py` import-light — importing anthias_common.utils there crashes django-stubs' mypy plugin in CI's slim env, so `is_balena_deploy()` is inlined. Segment webview-crash cohorts by `kernel_machine` (aarch64 → pi3-64 stream; armv7l → Qt5.15 WebEngine build). - **A late soft-timeout signal kills an idle pool child — the cause behind the ANTHIAS-B/1Q volume.** Celery arms the soft limit against the *job slot*, and `asynpool._on_soft_timeout` fires whenever the job is still in `Pool._cache`. The job only leaves that cache when the parent *drains the result off the outqueue*, which is a separate step from the child finishing the work — so on a memory-pressured board (soft limits observed firing 113s after a 30s timer was armed) the parent signals a child that is already back in `wait_for_job()`. `billiard.pool.soft_timeout_sighandler` raises unconditionally, the raise lands in the idle `workloop` where no task handler can catch it, the child exits 1, and billiard logs `Process 'ForkPoolWorker-N' exited with 'exitcode 1'` at ERROR (ANTHIAS-B; the respawn churn is ANTHIAS-1Q's "Timed out waiting for UP message"). #3352 only taught `before_send` to drop the escaping exception (ANTHIAS-45/5Y) — the child kept dying. Fixed by `lib/soft_timeout_guard.py`: honour the signal only between `task_prerun` and `task_postrun`. It must rebind `billiard.pool.soft_timeout_sighandler` before the pool forks — `Worker.after_fork` runs the pool initializer (which sends `worker_process_init`) and installs its handler *afterwards*, so a receiver on that signal is always overwritten. +- **An empty unresolved queue is not evidence of a healthy fleet — check `accepted` before triaging.** When the project's error allowance is spent, Sentry rejects everything with outcome `rate_limited` / reason `error_usage_exceeded`, every issue's `lastSeen` freezes at the day the allowance ran out, and the board reads as all-clear while devices keep failing. Verify with `GET /api/0/organizations/anthias/stats_v2/?field=sum(quantity)&groupBy=outcome&groupBy=reason&category=error&statsPeriod=14d&project=`: a run of days with `accepted` at zero and `rate_limited` still climbing means the queue is empty because nothing is getting in. The same call splits `client_discard` by reason, which is how to tell a `before_send` rule that is over-matching from one that is doing its job. +- **`shorter` and `longer than Content-Length` are different failures, and only one is noise.** uvicorn raises *shorter* when a response body ends before the declared length (the client-disconnect case) and *longer* when it overruns it (the response really did produce more bytes than it promised — a bug, which must keep reporting). PR #3182 filtered the *shorter* string while crediting ANTHIAS-37, which is the *longer* variant, so that issue was never actually suppressed. Root cause was `recover()` extracting assets straight over their final paths while the viewer was fetching them; fixed by staging + `os.replace` in `backup_helper._extract_asset_atomically`. - **Triage rule:** resolve every unresolved issue not seen on the current release (Sentry auto-reopens as regression if it recurs). Real code defects fixed in the 2026-07 sweep: PR #3180 (`reconcile_stuck_processing` bare `time_limit=300` → SIGKILL trio), PR #3181 (unguarded `await self.send()` in `asset_update` consumer, disconnect-vs-broadcast race → narrow `except RuntimeError`), PR #3182 (before_send drop for client-disconnect `Response content shorter than Content-Length` static noise). ## Telemetry diff --git a/src/anthias_server/django_project/settings.py b/src/anthias_server/django_project/settings.py index ee98fb2a6..6ed22a3d0 100644 --- a/src/anthias_server/django_project/settings.py +++ b/src/anthias_server/django_project/settings.py @@ -355,8 +355,17 @@ def _sentry_before_send(event: Event, hint: Hint) -> Event | None: short by the disconnect — a broken pipe, not a truncated file. Same client-disconnect class as ``CancelledError`` above, but it surfaces as a bare ``RuntimeError``, so it's matched by message - text to avoid swallowing unrelated RuntimeErrors (Sentry - ANTHIAS-37). + text to avoid swallowing unrelated RuntimeErrors. + + Note the message: uvicorn raises *shorter* when the body ends + early and *longer* when it overruns the header, and only the + first is the client-disconnect case described above. This rule + was originally credited with ANTHIAS-37, which is the *longer* + variant and so never matched it — a real overrun is a bug in + whatever produced the response, not noise, and must keep + reporting. ANTHIAS-37's own cause (a restore rewriting an asset + in place under a live ``FileResponse``) is fixed at the source + in ``backup_helper._extract_asset_atomically``. * ``SoftTimeLimitExceeded`` **with no Anthias frame anywhere in its traceback** — celery arms the soft-limit timer for the whole job, not just the task body, so a task that finishes a hair diff --git a/src/anthias_server/lib/backup_helper.py b/src/anthias_server/lib/backup_helper.py index 237c230ca..cfb67e0b4 100644 --- a/src/anthias_server/lib/backup_helper.py +++ b/src/anthias_server/lib/backup_helper.py @@ -8,6 +8,7 @@ import tarfile import tempfile import threading +import uuid from collections.abc import AsyncGenerator, Generator from concurrent.futures import ThreadPoolExecutor from datetime import UTC, datetime @@ -685,6 +686,61 @@ def create_backup(name: str = default_archive_name) -> str: return archive_name +def _is_asset_member(name: str) -> bool: + """True for a tar member that lives inside the media store.""" + parts = name.replace('\\', '/').strip('/').split('/') + return len(parts) > 1 and parts[0] in _ASSET_DIRS + + +def _extract_asset_atomically( + tar: tarfile.TarFile, + member: tarfile.TarInfo, + home: str, + extract_kwargs: dict[str, Any], +) -> None: + """Extract one media-store file via a sibling temp name + ``os.replace``. + + ``tar.extract`` writes straight to the member's final path, so an + asset that already exists on the device is truncated and then + refilled in place — while the viewer may be fetching that exact + file over ``/anthias_assets/``. Django's ``FileResponse`` + declares ``Content-Length`` from the file as it is at the moment + the response is built and then streams the same open fd to EOF, so + a restore that lands a *larger* copy makes the body overrun the + header (uvicorn: "Response content longer than Content-Length", + Sentry ANTHIAS-37) and a *smaller* one makes it fall short + ("Response content shorter than Content-Length"). Either way the + viewer has already been handed a spliced half-old/half-new image. + + Staging to a sibling and renaming makes the swap a single inode + change: an in-flight response keeps reading the inode it opened and + still matches the length it promised, and no request can ever + observe a half-written asset. This is the same contract every other + writer of the media store already keeps (``processing.py``, + ``celery_tasks.py``, ``views.py``, ``integrations/ingest.py``) — + restore was the one path that did not. + + The staging name is dot-prefixed *and* ``.tmp``-suffixed so both of + ``views_files.anthias_assets``' transient-file guards refuse to + serve it, ``_exclude_from_backup`` keeps it out of the next + archive, and the hourly ``cleanup()`` sweep collects it if the + process dies between the extract and the rename. + """ + directory = path.dirname(member.name) + staged_name = f'{directory}/.restore-{uuid.uuid4().hex}.tmp' + staged = member.replace(name=staged_name, deep=False) + staged_path = path.join(home, staged_name) + try: + tar.extract(staged, **extract_kwargs) + os.replace(staged_path, path.join(home, member.name)) + except BaseException: + try: + remove(staged_path) + except OSError: + pass + raise + + def recover(file_path: str) -> None: home = getenv('HOME') if not home: @@ -738,7 +794,15 @@ def recover(file_path: str) -> None: member.name, ) continue - tar.extract(member, **extract_kwargs) + if member.isfile() and _is_asset_member(member.name): + # Swapped in atomically — a live request for this + # asset must never see it half-written. See + # _extract_asset_atomically. + _extract_asset_atomically( + tar, member, home, extract_kwargs + ) + else: + tar.extract(member, **extract_kwargs) if member.isfile() and _is_db_file(member.name): # On an upgraded device ``.screenly`` and # ``screenly.db`` are back-compat symlinks to diff --git a/tests/test_backup_helper.py b/tests/test_backup_helper.py index e94803b98..d362c8fa2 100644 --- a/tests/test_backup_helper.py +++ b/tests/test_backup_helper.py @@ -495,6 +495,46 @@ def test_recover_writes_only_runtime_files(backup_home: str) -> None: assert not path.exists(path.join(a, 'evil.sh')) +def test_recover_swaps_assets_atomically(backup_home: str) -> None: + """A restore must not rewrite an asset in place. + + The viewer fetches assets over ``/anthias_assets/`` while a + restore is running, and Django's ``FileResponse`` declares + ``Content-Length`` from the file as it stands when the response is + built, then streams that same open fd to EOF. Truncating and + refilling the path under a live reader therefore both corrupts what + the viewer renders and breaks the promised length — overrunning it + when the restored copy is larger (uvicorn: "Response content longer + than Content-Length", Sentry ANTHIAS-37) and falling short when it + is smaller. Staging to a sibling and renaming makes the swap one + inode change, so an in-flight response is unaffected. + """ + asset_dir = path.join(backup_home, 'anthias_assets') + target = path.join(asset_dir, 'pic.jpg') + old_bytes = b'OLD' + new_bytes = b'NEW' * 100 # deliberately larger than what it replaces + with open(target, 'wb') as f: + f.write(old_bytes) + old_inode = os.stat(target).st_ino + + archive = path.join(backup_home, 'assets.tar.gz') + with tarfile.open(archive, 'w:gz') as tar: # NOSONAR + _stage_legit_layout(tar) + _add_file_member(tar, 'anthias_assets/pic.jpg', new_bytes) + + # Stands in for the response that is already streaming this asset. + with open(target, 'rb') as inflight: + recover(archive) + assert inflight.read() == old_bytes + assert os.fstat(inflight.fileno()).st_size == len(old_bytes) + + assert os.stat(target).st_ino != old_inode + with open(target, 'rb') as f: + assert f.read() == new_bytes + # The staging name is gone — no debris for the cleanup() sweep. + assert os.listdir(asset_dir) == ['pic.jpg'] + + def test_backup_ships_only_runtime_files(backup_home: str) -> None: """A backup captures the runtime whitelist and nothing else. Config, DB, default-asset manifest, db backups and assets are kept; the @@ -1188,7 +1228,10 @@ def test_recover_clears_the_stale_wal_when_a_later_member_fails( def extract( self: tarfile.TarFile, member: tarfile.TarInfo, **kwargs: object ) -> None: - if member.name == 'anthias_assets/after-the-db.bin': + # Matched by prefix, not by exact name: an asset is extracted + # under a staging name and renamed into place, so the member + # this sees is '.restore-.tmp', not the archived name. + if member.name.startswith('anthias_assets/'): raise OSError('disk full') real_extract(self, member, **kwargs) # type: ignore[arg-type]