Преглед изворни кода

fix(printers): deleting a printer no longer takes the archive of a print starting meanwhile

On PostgreSQL the delete route's ORM cascade deleted an archive a print
start wrote or adopted for the printer between the route's archive sweep
and its commit, also with "keep archives" and also another printer's or
a virtual-printer queue's archive (measured). The route now locks the
printer from the sweep to the commit: such a write waits and is refused
by the printer's foreign key. on_print_start treats that refusal (and the
StaleDataError of a reprint whose archive the delete removed) as the
printer being deleted: rolled back, forgotten, no error and no start
notification. archive_print removes the folder it copied the 3MF into
when its row is refused. Two camera viewer tests now wait for the
delayed stream reopen instead of letting it fire after teardown.
maziggy пре 1 недеља
родитељ
комит
31c3720782

+ 1 - 0
CHANGELOG.md

@@ -162,6 +162,7 @@ All notable changes to Bambuddy will be documented in this file.
 - **The frontend build no longer warns about `path` and `crypto` being externalized for the STEP previewer (#2976)** — `occt-import-js`, the Emscripten build behind STEP previews, requires both modules, but only inside its `ENVIRONMENT_IS_NODE` branches; in the browser it loads its `.wasm` from the URL the preview worker passes and draws randomness from `crypto.getRandomValues`. Vite still externalized both and printed two warnings on every build. `vite.config.ts` now drops exactly those two warnings for that one package through `build.rolldownOptions.onLog`, so an externalization anywhere else, or of any other module, still shows.
 
 ### Fixed
+- **Deleting a printer no longer deletes an archive of a print starting at that moment (PostgreSQL)** — If a queue or reprint print started on the printer while it was being deleted, the delete could take the print's archive with it, even with "keep archives" and even when the archive belonged to another printer or a virtual printer queue. The delete now locks the printer while it runs, so such a print start waits and is turned away, and its archive stays as it was; this is logged as a print start for a deleted printer, not as an error, and sends no start notification. An archive that could not be written no longer leaves its copied 3MF behind in the archive folder.
 - **A printer deleted while its print was starting no longer gets an archive or tracking** — When a print starts, Bambuddy downloads its 3MF from the printer, which can take minutes. A printer deleted during that download still got an archive afterwards: on SQLite one that showed "printing" forever, since the deleted printer would never report the end, along with a layer timelapse and tracking for it; on PostgreSQL the archive was refused and an error logged. Bambuddy now checks that the printer still exists once the download is done, and again once the archive is written; a queue or reprint print starting just as its printer is deleted is no longer attached to that printer either. Archives the delete was told to keep stay as they are. A queue print dispatched to a printer deleted during its upload no longer records its owner for that printer either.
 - **Closing one camera view no longer cuts off the others** — With an external RTSP or USB camera, closing one view of a printer (a Cam Wall tile turning to snapshot, the Camera page, the camera window's refresh) stopped that printer's camera stream for every other open view too, which then froze until it reconnected. Each view now names itself on its stream and its stop: the stop ends that view's own stream (at once, as before, so a USB camera is released when its view closes) and any stream that has stopped sending pictures, and leaves the other views' streams running.
 - **Closing or minimizing a camera view now ends its stream** — The camera window on the Printers page left its stream open when minimized (one more open connection per minimize and expand), and both it and the Camera page closed only the first picture they had shown: after a reconnect or a restart, closing them left the current stream running. An open stream keeps the printer's camera connection busy, keeps the chamber light on when "Turn on chamber light for camera" is on for that printer, and made the "stop camera" request on closing do nothing. Every picture a camera view drops now ends its stream. A reconnect also no longer counts one extra failed attempt toward the reconnect limit.

+ 9 - 0
backend/app/api/routes/printers.py

@@ -515,6 +515,15 @@ async def delete_printer(
 
     printer_manager.disconnect_printer(printer_id)
 
+    # Locked from the archive sweep to the commit (PostgreSQL; SQLite's single
+    # writer serialises this anyway). A print start adopting or writing an
+    # archive for this printer meanwhile then waits and is refused by the
+    # printer's foreign key, instead of passing it and being deleted by the
+    # ORM delete's cascade below: also an archive the user chose to keep, or
+    # another printer's (measured). Not earlier: retiring a Notify provider
+    # above commits, which ends a lock; nothing commits from here on.
+    await db.execute(select(Printer.id).where(Printer.id == printer_id).with_for_update())
+
     if delete_archives:
         # Delete all archives for this printer
         await db.execute(sql_delete(PrintArchive).where(PrintArchive.printer_id == printer_id))

+ 50 - 8
backend/app/main.py

@@ -17,6 +17,8 @@ from fastapi import FastAPI
 from fastapi.responses import FileResponse
 from fastapi.staticfiles import StaticFiles
 from sqlalchemy import delete, inspect as sa_inspect, or_, select, text, update
+from sqlalchemy.exc import IntegrityError
+from sqlalchemy.orm.exc import StaleDataError
 
 from backend.app.api.routes import (
     ams_history,
@@ -4214,6 +4216,24 @@ async def _drop_archive_of_deleted_printer(db, printer_id: int, archive_id: int)
     return True
 
 
+async def _refused_for_deleted_printer(db, printer_id: int, what: str) -> bool:
+    """After an IntegrityError on a print start's archive write: whether it was
+    the printer being deleted, forgotten then (the caller returns).
+
+    PostgreSQL: the delete route locks the printer from its archive sweep to its
+    commit, so an archive written or adopted for it meanwhile (one that sets
+    printer_id) waits and the printer's foreign key then refuses it. That is
+    the printer being deleted, not an error; anything else is re-raised by the
+    caller.
+    """
+    await db.rollback()
+    if not await _printer_gone(db, printer_id):
+        return False
+    logging.getLogger(__name__).info("Printer %s was deleted while %s; refused", printer_id, what)
+    await forget_printer(printer_id)
+    return True
+
+
 async def _forget_if_printer_gone(printer_id: int) -> None:
     """At the end of a print start: the slow steps after the last check (smart
     plug, notifications, the timelapse baseline's FTP listing) can outlast the
@@ -4698,7 +4718,16 @@ async def on_print_start(printer_id: int, data: dict):
                 # button in ArchivesPage (gated on !archive.printer_id).
                 if archive.printer_id != printer_id:
                     archive.printer_id = printer_id
-                await db.commit()
+                adopting_id = archive.id  # a rollback expires the object
+                try:
+                    await db.commit()
+                except (IntegrityError, StaleDataError):
+                    # The archive stays unchanged, or is gone: "delete archives"
+                    # removed the row under the adoption, which then matched no
+                    # row (StaleDataError) rather than being refused.
+                    if await _refused_for_deleted_printer(db, printer_id, f"archive {adopting_id} was adopted"):
+                        return
+                    raise
 
                 # The printer can be deleted between the check above and this
                 # commit (the route's transaction lasts seconds), and the commit
@@ -5530,7 +5559,14 @@ async def on_print_start(printer_id: int, data: dict):
                 )
 
                 db.add(fallback_archive)
-                await db.commit()
+                try:
+                    await db.commit()
+                except IntegrityError:
+                    # Not "Failed to create fallback archive" below, and no
+                    # start notification for a printer that is gone.
+                    if await _refused_for_deleted_printer(db, printer_id, "its fallback archive was written"):
+                        return
+                    raise
                 await db.refresh(fallback_archive)
 
                 logger.info("Created fallback archive %s for %s (no 3MF available)", fallback_archive.id, print_name)
@@ -5644,12 +5680,18 @@ async def on_print_start(printer_id: int, data: dict):
         try:
             # Archive the file with status "printing"
             service = ArchiveService(db)
-            archive = await service.archive_print(
-                printer_id=printer_id,
-                source_file=temp_path,
-                print_data={**data, "status": "printing"},
-                subtask_id=subtask_id,
-            )
+            try:
+                archive = await service.archive_print(
+                    printer_id=printer_id,
+                    source_file=temp_path,
+                    print_data={**data, "status": "printing"},
+                    subtask_id=subtask_id,
+                )
+            except IntegrityError:
+                # archive_print removed the folder it had copied the 3MF into.
+                if await _refused_for_deleted_printer(db, printer_id, "its archive was written"):
+                    return
+                raise
 
             if archive:
                 # Ask-for-outcome for a print Bambuddy did not dispatch (#1898).

+ 14 - 1
backend/app/services/archive.py

@@ -11,6 +11,7 @@ from pathlib import Path
 
 from defusedxml import ElementTree as ET
 from sqlalchemy import and_, or_, select, text
+from sqlalchemy.exc import IntegrityError
 from sqlalchemy.ext.asyncio import AsyncSession
 
 from backend.app.core.config import settings
@@ -1319,6 +1320,9 @@ class ArchiveService:
         archive_dir = (
             settings.archive_dir / printer_folder / archive_name
         )  # SEC-PATH-OK: printer_folder = str(int|None) → digits or "unassigned"; archive_name = f"{timestamp}_{display_stem}" where resolve_display_stem strips path components via Path(filename).name
+        # Removed again if the row is not written (below); only a folder this
+        # call created, never another archive's of the same name.
+        created_archive_dir = not archive_dir.exists()
         archive_dir.mkdir(parents=True, exist_ok=True)
 
         # Copy 3MF file with an explicit fsync'd loop (avoids a sendfile
@@ -1554,7 +1558,16 @@ class ArchiveService:
         )
 
         self.db.add(archive)
-        await self.db.commit()
+        try:
+            await self.db.commit()
+        except IntegrityError:
+            # Refused, so the row was not written (a printer deleted meanwhile,
+            # by its foreign key): the folder its files went into would be
+            # orphaned. Not on other errors: a connection lost during COMMIT may
+            # still have written the row, whose files these are.
+            if created_archive_dir:
+                shutil.rmtree(archive_dir, ignore_errors=True)
+            raise
         await self.db.refresh(archive)
 
         return archive

+ 303 - 0
backend/tests/integration/test_print_start_printer_deleted_midway.py

@@ -433,3 +433,306 @@ async def test_the_last_check_never_raises(printer_factory):
         await main._forget_if_printer_gone(1)
 
     forget.assert_not_awaited()
+
+
+async def test_a_refused_adoption_is_a_deleted_printer_not_an_error(test_engine, db_session, printer_factory):
+    """PostgreSQL: an adoption rewriting printer_id (a VP-queue or another
+    printer's archive) waits on the delete route's lock on the printer, then
+    the printer foreign key refuses it (measured). That is the printer being
+    deleted, not a failure of the print start."""
+    from sqlalchemy.exc import IntegrityError
+    from sqlalchemy.ext.asyncio import AsyncSession
+
+    from backend.app import main
+
+    printer = await printer_factory()
+    printer_id = printer.id
+    archive_id = await _queued_archive(db_session, printer_id)
+    real_gone = main._printer_gone
+    real_commit = AsyncSession.commit
+    refuse = []
+
+    async def deleted_with_its_archives_kept(db, pid):
+        if not refuse:
+            await _delete_keeping_archives(db_session, printer_id)
+            refuse.append(True)  # the adoption's commit, next
+            return False  # read before the delete committed
+        return await real_gone(db, pid)
+
+    async def commit(self):
+        if refuse and refuse[0]:
+            refuse[0] = False
+            raise IntegrityError("UPDATE print_archives", {}, Exception("violates foreign key constraint"))
+        return await real_commit(self)
+
+    with (
+        patch("backend.app.main.forget_printer", new_callable=AsyncMock) as forget,
+        patch("backend.app.main._printer_gone", new=deleted_with_its_archives_kept),
+        patch.object(AsyncSession, "commit", commit),
+    ):
+        mocks = await _drive_print_start(test_engine, printer, download_ok=False)
+
+    archive = await _archive(db_session, archive_id)
+    assert archive.printer_id is None and archive.status == "completed"
+    assert not [key for key in _active_prints if key[0] == printer_id]
+    mocks["_maybe_start_layer_timelapse"].assert_not_called()
+    forget.assert_awaited_once_with(printer_id)
+
+
+async def test_a_refused_adoption_of_a_printer_still_there_is_still_an_error(test_engine, db_session, printer_factory):
+    """Only the printer's delete explains a refused adoption; anything else
+    is a real error and must not be swallowed as one."""
+    from sqlalchemy.exc import IntegrityError
+    from sqlalchemy.ext.asyncio import AsyncSession
+
+    from backend.app import main
+
+    printer = await printer_factory()
+    await _queued_archive(db_session, printer.id)
+    real_gone = main._printer_gone
+    real_commit = AsyncSession.commit
+    refuse = []
+
+    async def checked(db, pid):
+        refuse.append(True)  # the adoption's commit, next
+        return await real_gone(db, pid)
+
+    async def commit(self):
+        if len(refuse) == 1:
+            refuse.append(False)
+            raise IntegrityError("UPDATE print_archives", {}, Exception("some other constraint"))
+        return await real_commit(self)
+
+    with (
+        patch("backend.app.main.forget_printer", new_callable=AsyncMock),
+        patch("backend.app.main._printer_gone", new=checked),
+        patch.object(AsyncSession, "commit", commit),
+        pytest.raises(IntegrityError),
+    ):
+        await _drive_print_start(test_engine, printer, download_ok=False)
+
+
+@pytest.mark.parametrize("keep", [False, True], ids=["delete archives", "keep archives"])
+async def test_the_delete_locks_the_printer_from_its_archive_sweep_to_its_commit(
+    async_client, db_session, printer_factory, keep
+):
+    """PostgreSQL (measured): a print start adopting a VP-queue or another
+    printer's archive after the sweep passed the foreign key, and the delete's
+    ORM cascade then deleted that archive, also with "keep archives". Locked,
+    the adoption waits and is then refused. (SQLite drops FOR UPDATE; its
+    single writer serialises this anyway, so the statement is what is checked.)"""
+    from sqlalchemy.ext.asyncio import AsyncSession
+    from sqlalchemy.sql import Select
+
+    printer = await printer_factory()
+    real_execute = AsyncSession.execute
+    statements: list = []
+
+    async def recording(self, statement, *args, **kwargs):
+        statements.append(statement)
+        return await real_execute(self, statement, *args, **kwargs)
+
+    with patch.object(AsyncSession, "execute", recording):
+        response = await async_client.delete(
+            f"/api/v1/printers/{printer.id}", params={"delete_archives": str(not keep).lower()}
+        )
+    assert response.status_code == 200, response.text
+
+    def is_lock(s):
+        return isinstance(s, Select) and s._for_update_arg is not None and "printers" in str(s)
+
+    def is_sweep(s):
+        return not isinstance(s, Select) and "print_archives" in str(s)
+
+    locks = [i for i, s in enumerate(statements) if is_lock(s)]
+    sweeps = [i for i, s in enumerate(statements) if is_sweep(s)]
+    assert locks and sweeps and locks[0] < sweeps[0]
+
+
+def _refusing_first_commit_after_the_check(db_session, printer_id, main_module):
+    """The printer deleted right after the handler's check; the next commit is
+    refused by its foreign key (PostgreSQL, the delete holding the printer)."""
+    from sqlalchemy.exc import IntegrityError
+    from sqlalchemy.ext.asyncio import AsyncSession
+
+    real_gone = main_module._printer_gone
+    real_commit = AsyncSession.commit
+    state: list = []
+
+    async def gone(db, pid):
+        if not state:
+            await db_session.execute(delete(Printer).where(Printer.id == printer_id))
+            await db_session.commit()
+            state.append(True)  # refuse the next commit
+            return False  # read before the delete committed
+        return await real_gone(db, pid)
+
+    async def commit(self):
+        if state and state[0]:
+            state[0] = False
+            raise IntegrityError("INSERT INTO print_archives", {}, Exception("violates foreign key constraint"))
+        return await real_commit(self)
+
+    return (
+        patch("backend.app.main._printer_gone", new=gone),
+        patch.object(AsyncSession, "commit", commit),
+    )
+
+
+@pytest.mark.parametrize("download_ok", [False, True], ids=["no-3mf fallback", "downloaded 3mf"])
+async def test_a_refused_new_archive_is_a_deleted_printer_not_an_error(
+    test_engine, db_session, printer_factory, download_ok
+):
+    """With the delete locking the printer, an archive written meanwhile waits
+    and is refused: no error escaping the print start, no "Failed to create
+    fallback archive", no start notification for the deleted printer."""
+    from backend.app import main
+
+    printer = await printer_factory()
+    printer_id = printer.id
+
+    with patch("backend.app.main.forget_printer", new_callable=AsyncMock) as forget:
+        mocks = await _drive_print_start(
+            test_engine,
+            printer,
+            download_ok=download_ok,
+            extra_patches=(
+                patch("backend.app.main.ArchiveService", _ArchiveService),
+                *_refusing_first_commit_after_the_check(db_session, printer_id, main),
+            ),
+        )
+
+    assert await _archives(db_session, printer_id) == []
+    assert not [key for key in _active_prints if key[0] == printer_id]
+    mocks["_maybe_start_layer_timelapse"].assert_not_called()
+    mocks["_send_print_start_notification"].assert_not_awaited()
+    forget.assert_awaited_once_with(printer_id)
+
+
+async def test_an_archive_whose_commit_fails_leaves_no_folder(db_session, printer_factory, tmp_path, monkeypatch):
+    """archive_print copies the 3MF into the new archive's folder before the
+    row is written; a refused commit left that folder behind."""
+    import zipfile
+
+    from sqlalchemy.exc import IntegrityError
+
+    from backend.app.core.config import settings
+    from backend.app.services.archive import ArchiveService
+
+    monkeypatch.setattr(settings, "base_dir", tmp_path)  # archive paths are stored relative to it
+    monkeypatch.setattr(settings, "archive_dir", tmp_path / "archive")
+    source = tmp_path / "Cube.3mf"
+    with zipfile.ZipFile(source, "w") as zf:
+        zf.writestr("Metadata/slice_info.config", "<config><plate></plate></config>")
+    printer = await printer_factory()
+
+    async def refused():
+        raise IntegrityError("INSERT INTO print_archives", {}, Exception("violates foreign key constraint"))
+
+    monkeypatch.setattr(db_session, "commit", refused)
+    with pytest.raises(IntegrityError):
+        await ArchiveService(db_session).archive_print(printer_id=printer.id, source_file=source)
+
+    assert [p for p in (tmp_path / "archive").rglob("*") if p.is_file()] == []
+
+
+async def test_a_failed_archive_leaves_a_folder_it_did_not_create(db_session, printer_factory, tmp_path, monkeypatch):
+    """Same name, same second: the folder was another archive's."""
+    import zipfile
+    from datetime import datetime
+
+    from sqlalchemy.exc import IntegrityError
+
+    from backend.app.core.config import settings
+    from backend.app.services import archive as archive_module
+    from backend.app.services.archive import ArchiveService
+
+    class _Frozen(datetime):
+        @classmethod
+        def now(cls, tz=None):
+            return cls(2026, 1, 2, 3, 4, 5, tzinfo=tz)
+
+    monkeypatch.setattr(archive_module, "datetime", _Frozen)
+    monkeypatch.setattr(settings, "base_dir", tmp_path)
+    monkeypatch.setattr(settings, "archive_dir", tmp_path / "archive")
+    source = tmp_path / "Cube.3mf"
+    with zipfile.ZipFile(source, "w") as zf:
+        zf.writestr("Metadata/slice_info.config", "<config><plate></plate></config>")
+    printer = await printer_factory()
+    other = tmp_path / "archive" / str(printer.id) / "20260102_030405_Cube" / "other.3mf"
+    other.parent.mkdir(parents=True)
+    other.write_bytes(b"another archive's file")
+
+    async def refused():
+        raise IntegrityError("INSERT INTO print_archives", {}, Exception("violates foreign key constraint"))
+
+    monkeypatch.setattr(db_session, "commit", refused)
+    with pytest.raises(IntegrityError):
+        await ArchiveService(db_session).archive_print(printer_id=printer.id, source_file=source)
+
+    assert other.exists()
+
+
+async def test_a_reprint_whose_archive_the_delete_removed_is_a_deleted_printer_not_an_error(
+    test_engine, db_session, printer_factory
+):
+    """ "Delete archives" removes the archive under the adoption (a reprint of
+    the printer's own archive; a queue print is refused earlier by the route's
+    409): the adoption's update matches no row (StaleDataError, not an
+    IntegrityError)."""
+    from backend.app import main
+
+    printer = await printer_factory()
+    printer_id = printer.id
+    archive_id = await _queued_archive(db_session, printer_id)
+    real = main._printer_gone
+    calls = []
+
+    async def deleted_with_its_archives(db, pid):
+        calls.append(pid)
+        if len(calls) == 1:
+            await db_session.execute(delete(PrintArchive).where(PrintArchive.printer_id == printer_id))
+            await db_session.execute(delete(Printer).where(Printer.id == printer_id))
+            await db_session.commit()
+            return False  # read before the delete committed
+        return await real(db, pid)
+
+    with (
+        patch("backend.app.main.forget_printer", new_callable=AsyncMock) as forget,
+        patch("backend.app.main._printer_gone", new=deleted_with_its_archives),
+    ):
+        mocks = await _drive_print_start(test_engine, printer, download_ok=False)
+
+    assert await _archive(db_session, archive_id) is None
+    assert not [key for key in _active_prints if key[0] == printer_id]
+    mocks["_maybe_start_layer_timelapse"].assert_not_called()
+    forget.assert_awaited_once_with(printer_id)
+
+
+async def test_an_archive_whose_commit_may_have_landed_keeps_its_folder(
+    db_session, printer_factory, tmp_path, monkeypatch
+):
+    """A connection lost during COMMIT may still have written the row: its
+    files stay. Only a refusal (IntegrityError) proves the row was not written."""
+    import zipfile
+
+    from sqlalchemy.exc import OperationalError
+
+    from backend.app.core.config import settings
+    from backend.app.services.archive import ArchiveService
+
+    monkeypatch.setattr(settings, "base_dir", tmp_path)
+    monkeypatch.setattr(settings, "archive_dir", tmp_path / "archive")
+    source = tmp_path / "Cube.3mf"
+    with zipfile.ZipFile(source, "w") as zf:
+        zf.writestr("Metadata/slice_info.config", "<config><plate></plate></config>")
+    printer = await printer_factory()
+
+    async def connection_lost():
+        raise OperationalError("COMMIT", {}, Exception("connection was closed in the middle of operation"))
+
+    monkeypatch.setattr(db_session, "commit", connection_lost)
+    with pytest.raises(OperationalError):
+        await ArchiveService(db_session).archive_print(printer_id=printer.id, source_file=source)
+
+    assert [p.name for p in (tmp_path / "archive").rglob("*") if p.is_file()] == ["Cube.3mf"]

+ 3 - 0
frontend/src/__tests__/components/EmbeddedCameraViewer.test.tsx

@@ -102,6 +102,9 @@ describe('EmbeddedCameraViewer stream lifetime', () => {
 
     const stop = fetchMock.mock.calls.map(([url]) => String(url)).find((url) => url.includes('/camera/stop'));
     expect(new URL(stop!, 'http://x').searchParams.get('viewer')).toBe(id);
+    // Refresh reopens the stream 100 ms later: let that happen inside the test,
+    // not after its environment is gone.
+    await waitFor(() => expect(screen.getByAltText('Camera stream')).not.toBe(img));
   });
 
   it('keeps its stream under StrictMode', async () => {

+ 3 - 0
frontend/src/__tests__/pages/CameraPage.test.tsx

@@ -206,6 +206,9 @@ describe('CameraPage', () => {
 
       const stop = fetchMock.mock.calls.map(([url]) => String(url)).find((url) => url.includes('/camera/stop'));
       expect(new URL(stop!, 'http://x').searchParams.get('viewer')).toBe(id);
+      // Restart reopens the stream 100 ms later: let that happen inside the
+      // test, not after its environment is gone.
+      await waitFor(() => expect(document.querySelector('img')).not.toBe(img));
       fetchMock.mockRestore();
     });