diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c262781..457a410c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- **Failed sloppak loads at the highway websocket no longer crash on a `None` song.** + When `sloppak_mod.load_song()` returns `None` (cache corruption, partial + extraction, etc.), the handler previously dereferenced `loaded_slop.song` + before any guard existed, crashing the connection instead of reporting the + failure. The load-failure guard now sits immediately after the load call — + the handler sends a `Failed to load sloppak` error and closes the socket + instead of continuing into arrangement/stem access with a `None` song. + ### Added - **Core reader for source rigs (feedpak 1.18.0).** A pack can declare what a MIDI part should sound like by binding a rig; core now reads that binding and diff --git a/lib/routers/ws_highway.py b/lib/routers/ws_highway.py index fd1ad602..fae2dd3e 100644 --- a/lib/routers/ws_highway.py +++ b/lib/routers/ws_highway.py @@ -212,6 +212,12 @@ async def _send_keepalives(): None, lambda: _ctx.run(sloppak_mod.load_song, filename, dlc, appstate.sloppak_cache_dir), ) + if loaded_slop is None: + _keepalive_active = False + keepalive_task.cancel() + await websocket.send_json({"error": "Failed to load sloppak"}) + await websocket.close() + return song = loaded_slop.song tmp = str(loaded_slop.source_dir) owns_tmp = False diff --git a/tests/test_highway_ws_failed_sloppak_load.py b/tests/test_highway_ws_failed_sloppak_load.py new file mode 100644 index 00000000..d3e859cd --- /dev/null +++ b/tests/test_highway_ws_failed_sloppak_load.py @@ -0,0 +1,69 @@ +"""Regression coverage for sloppak load failures in the highway websocket.""" + +import asyncio +import importlib +import sys + +import pytest + + +class _CapturingWS: + def __init__(self): + self.messages = [] + self.accepted = False + self.closed = False + self.close_calls = 0 + + async def accept(self): + self.accepted = True + + async def send_json(self, data): + self.messages.append(data) + + async def receive_text(self): + await asyncio.sleep(0) + return "" + + async def close(self): + self.close_calls += 1 + self.closed = True + + +@pytest.fixture() +def server(tmp_path, monkeypatch): + (tmp_path / "dlc").mkdir() + monkeypatch.setenv("CONFIG_DIR", str(tmp_path / "config")) + monkeypatch.setenv("DLC_DIR", str(tmp_path / "dlc")) + monkeypatch.setenv("FEEDBACK_SKIP_STARTUP_TASKS", "1") + sys.modules.pop("server", None) + mod = importlib.import_module("server") + yield mod, tmp_path / "dlc", tmp_path / "cache" + conn = getattr(getattr(mod, "meta_db", None), "conn", None) + if conn is not None: + getattr(mod, "_join_background_db_threads", lambda: None)() + conn.close() + sys.modules.pop("server", None) + + +def test_sloppak_loader_returning_none_sends_error_without_touching_stems( + server, monkeypatch +): + _server, dlc, cache = server + (dlc / "broken.feedpak").mkdir() + + import appstate + from routers import ws_highway + + monkeypatch.setattr(appstate, "sloppak_cache_dir", cache) + monkeypatch.setattr(ws_highway.sloppak_mod, "load_song", lambda *a, **kw: None) + + ws = _CapturingWS() + asyncio.run(ws_highway.highway_ws(ws, "broken.feedpak", arrangement=0)) + + assert ws.accepted is True + assert ws.closed is True + assert ws.close_calls == 1 + assert ws.messages == [ + {"type": "loading", "stage": "Extracting..."}, + {"error": "Failed to load sloppak"}, + ]