Files
peter bdf7c03b59 fix(r5): address the eighth /code-review high round — 7 findings
The worst one was, again, the previous round's own fix.

- Round 7 stopped awaiting the HLS sweep so startup could serve immediately — which
  also made it run CONCURRENTLY with playback, and `sweep_orphans` snapshotted the
  live-session set BEFORE listing the root and rmtree'd outside the lock. Since
  `start_session` reuses a directory path for the same (key, offset, tag), a stale
  name can become a live session at any moment. Two guards now: nothing whose mtime
  is newer than this process's start is ever deleted, and the live check + rmtree
  happen together under the lock, one directory at a time.
- The sweep task is awaited after cancel, so shutdown can't leave it pending.
- `_rate_limiter` used a truthiness test on its timestamp, so a `now()` of exactly
  0.0 read as "never ran" — unreachable in production, but `now` exists to be
  injected and a test clock starting at 0 is the obvious choice.
- The RSS total-outage error now carries the first underlying error: "all N channels
  failed" is also what ONE dead feed id looks like on a small instance, and a red
  card the user can't act on is its own kind of dishonest.
- The WebKit fullscreen handling moved into `lib/fullscreen` and PlexPlayer uses it
  too — it had the same unprefixed-only reads in three places, including an Escape
  branch that would have closed the whole player instead of leaving fullscreen.
- The `--max` CSS comment claimed `inset: 0` did the sizing; Tailwind's `w-full` is
  still on the element and wins. Comment now states the real mechanism.

Verified in real Chrome (the pane can't composite or do native fullscreen):
- F fills the 1920x889 viewport; the exit pill renders top-right on the letterbox
  band and overlaps NO YouTube control — the collision this round suspected does not
  reproduce in this embed (its own buttons sit bottom-left / bottom-right).
- Four real ArrowDown presses while maximised: persisted volume 100 -> 80, with the
  level flash visible on screen.
- A real click on the exit pill un-maximises and returns focus to the card.
- lib/fullscreen driven from a real click: enter (innerHeight 889 -> 1024), the
  change listener fires exactly once per transition, exit returns to 889.

Backend suite 150 -> 158; the new sweep tests are mutation-checked, and they read
their cutoff from the filesystem rather than a clock (the container VM and the
bind-mounted host disagree by enough to make a clock-based assertion flaky).
2026-07-24 04:44:20 +02:00

244 lines
9.9 KiB
Python

import asyncio
import logging
import sys
from contextlib import asynccontextmanager, suppress
from pathlib import Path
from fastapi import FastAPI, HTTPException
# When started via uvicorn --log-config the "siftlode" logger is already configured
# (see log_config.json). This block is a timestamped fallback for other entrypoints
# (tests, scripts) so our logs are never silently dropped.
_siftlode_logger = logging.getLogger("siftlode")
if not _siftlode_logger.handlers:
_handler = logging.StreamHandler(sys.stdout)
_handler.setFormatter(
logging.Formatter("%(asctime)s %(levelname)-5s [%(name)s] %(message)s")
)
_siftlode_logger.addHandler(_handler)
_siftlode_logger.setLevel(logging.INFO)
_siftlode_logger.propagate = False
log = logging.getLogger("siftlode.app")
from fastapi.middleware.cors import CORSMiddleware
from fastapi.responses import FileResponse, HTMLResponse, JSONResponse
from fastapi.staticfiles import StaticFiles
from starlette.middleware.sessions import SessionMiddleware
from app import auth, state
from app.config import settings
from app.db import SessionLocal
from app.plex import stream as plex_stream
from app.routes import (
admin,
audit as audit_routes,
channels,
config as config_routes,
downloads,
feed,
health,
me,
messages,
notifications,
playlists,
plex as plex_routes,
public as public_routes,
quota,
saved_views,
scheduler as scheduler_routes,
search as search_routes,
setup as setup_routes,
sync,
tags,
version,
)
from app.scheduler import shutdown_scheduler, start_scheduler
@asynccontextmanager
async def lifespan(app: FastAPI):
log.info("Siftlode starting up")
# First-run: if the instance isn't configured yet, mint a fresh one-time setup token and print
# the wizard URL so the operator can open it (the app runs in setup mode until they finish).
with SessionLocal() as db:
if not state.is_configured(db):
token = state.rotate_setup_token(db)
url = f"{settings.app_base}/setup?token={token}"
log.warning(
"FIRST-RUN SETUP REQUIRED — this instance isn't configured yet.\n"
" Open the install wizard (internal access only):\n %s",
url,
)
# No HLS session survives a restart (they live in this process), so anything still on disk is
# a crashed run's segments — sweep them or they accumulate forever. FIRE AND FORGET, not
# awaited: uvicorn does not serve a single request until lifespan startup returns, so awaiting
# the sweep would hold the port shut for as long as the rmtree runs (gigabytes of `.ts` after a
# long uptime) — the 502 + failed post-deploy health probe this is meant to avoid. A thread
# alone doesn't buy that; not waiting does. The sweep only touches directories no live session
# owns, so racing it against early playback is safe.
sweep = asyncio.create_task(_sweep_hls_orphans())
start_scheduler()
try:
yield
finally:
log.info("Siftlode shutting down")
# Awaited, not just cancelled: a bare `.cancel()` lets the loop close with the task still
# pending ("Task was destroyed but it is pending!" — noise that reads like a bug during
# triage). Cancelling does NOT interrupt the rmtree thread itself; this only makes our own
# bookkeeping honest.
sweep.cancel()
with suppress(asyncio.CancelledError):
await sweep
shutdown_scheduler()
async def _sweep_hls_orphans() -> None:
"""Background half of the startup HLS sweep (see lifespan). Owns its own errors: a failure here
must never take the app down, and a cancelled shutdown is not one."""
try:
swept = await asyncio.to_thread(plex_stream.sweep_orphans)
except asyncio.CancelledError:
raise
except Exception:
log.exception("HLS orphan sweep failed")
return
if swept:
log.info("swept %d orphaned HLS segment directories", swept)
app = FastAPI(title=settings.app_name, lifespan=lifespan)
app.add_middleware(
SessionMiddleware,
secret_key=settings.secret_key,
same_site="lax", # required so the cookie rides the OAuth redirect back from Google
https_only=settings.session_https_only, # Secure flag when served over HTTPS (prod)
)
if settings.frontend_origin:
app.add_middleware(
CORSMiddleware,
allow_origins=[settings.frontend_origin],
allow_credentials=True,
allow_methods=["*"],
allow_headers=["*"],
)
# Paths reachable while the instance is unconfigured (setup mode). Everything else under /api or
# /auth is locked until the wizard finishes; static/SPA always loads so the wizard page can render.
_SETUP_OPEN_PREFIXES = ("/api/setup", "/api/version")
@app.middleware("http")
async def setup_gate(request, call_next):
"""Lock the app to the install wizard until it's configured. Skips the DB entirely once setup
completed in this process (the cached fast-path), so there's no steady-state overhead."""
if not state.setup_complete():
path = request.url.path
if path.startswith(("/api/", "/auth/")) and not path.startswith(_SETUP_OPEN_PREFIXES):
with SessionLocal() as db:
configured = state.is_configured_cached(db)
if not configured:
return JSONResponse(
{"detail": "This instance isn't set up yet."}, status_code=503
)
return await call_next(request)
@app.middleware("http")
async def static_cache_headers(request, call_next):
"""Long-cache the content-hashed SPA bundles. Vite hashes their filename, so a given
/assets URL never changes content and a browser can hold it forever — this is what makes
repeat loads instant and clears the Lighthouse "efficient cache lifetimes" audit. index.html
stays no-cache (set on its own FileResponse) so a deploy is picked up at once; other
SPA-root static files (welcome images, favicon, robots.txt) get a moderate TTL in the SPA
fallback below."""
response = await call_next(request)
if request.url.path.startswith("/assets/"):
response.headers["Cache-Control"] = "public, max-age=31536000, immutable"
return response
app.include_router(health.router)
app.include_router(auth.router)
app.include_router(sync.router)
app.include_router(tags.router)
app.include_router(feed.router)
app.include_router(search_routes.router)
app.include_router(me.router)
app.include_router(notifications.router)
app.include_router(messages.router)
app.include_router(channels.router)
app.include_router(playlists.router)
app.include_router(saved_views.router)
app.include_router(plex_routes.router)
app.include_router(downloads.router)
app.include_router(downloads.admin_router)
app.include_router(public_routes.router)
app.include_router(admin.router)
app.include_router(config_routes.router)
app.include_router(scheduler_routes.router)
app.include_router(audit_routes.router)
app.include_router(quota.router)
app.include_router(version.router)
app.include_router(setup_routes.router)
# Ensure modern image types resolve to the right Content-Type when FileResponse guesses from the
# filename (the runtime's mimetypes db doesn't always know .webp → it'd fall back to octet-stream).
import mimetypes
mimetypes.add_type("image/webp", ".webp")
mimetypes.add_type("image/avif", ".avif")
# The built SPA (populated by the Docker frontend build stage).
STATIC_DIR = Path(__file__).parent / "static_spa"
# index.html is unhashed and references the content-hashed /assets bundles, so it MUST NOT be
# heuristically cached — otherwise, after a deploy, a browser keeps serving the old index.html
# (pointing at the previous bundle) and runs stale code until a hard refresh. `no-cache` lets it
# stay cached but forces revalidation (cheap 304 via the FileResponse ETag) on every load. The
# hashed bundles under /assets can cache forever (a new build changes their filename).
INDEX_HTML = STATIC_DIR / "index.html"
INDEX_HEADERS = {"Cache-Control": "no-cache"}
app.mount(
"/assets",
StaticFiles(directory=STATIC_DIR / "assets", check_dir=False),
name="assets",
)
@app.get("/")
async def index() -> FileResponse:
return FileResponse(INDEX_HTML, headers=INDEX_HEADERS)
@app.get("/watch/{token}")
async def watch_page(token: str):
"""Serve the SPA for a public share link, but with per-video Open Graph tags injected so the
link unfurls richly (title/channel/thumbnail) in Messenger/social crawlers. A real browser
ignores the extra tags and hydrates WatchPage as usual; invalid/expired/password links fall
back to the generic card."""
from app.downloads import og
with SessionLocal() as db:
rendered = og.render_watch_html(token, INDEX_HTML, db)
if rendered is None:
return FileResponse(INDEX_HTML, headers=INDEX_HEADERS)
return HTMLResponse(rendered, headers=INDEX_HEADERS)
@app.get("/{full_path:path}")
async def spa_fallback(full_path: str) -> FileResponse:
# Client-side routes fall back to index.html; real API/asset paths are matched above.
if full_path.startswith(("api/", "auth/", "healthz", "assets/")):
raise HTTPException(status_code=404)
# Serve real files that live at the SPA root (Vite copies public/ there — e.g. the landing
# screenshots under /welcome/, favicon). /assets is already mounted above; everything else
# that isn't a real file is a client-side route → index.html. Guard against path traversal.
if full_path:
candidate = (STATIC_DIR / full_path).resolve()
if candidate.is_file() and STATIC_DIR.resolve() in candidate.parents:
# Real SPA-root assets (welcome images, favicon, robots.txt) rarely change and have
# stable names, so a moderate cache is safe and satisfies the cache-lifetime audit.
return FileResponse(candidate, headers={"Cache-Control": "public, max-age=604800"})
return FileResponse(INDEX_HTML, headers=INDEX_HEADERS)