From fb0aebc6f65f9ca64173f0dab06cf114100bb824 Mon Sep 17 00:00:00 2001 From: maziggy Date: Fri, 9 Oct 2026 11:02:32 +0200 Subject: [PATCH] Give a large timelapse download time to finish instead of a flat 300 s (#3272) --- CHANGELOG.md | 1 + backend/app/services/bambu_ftp.py | 21 +++-- .../test_ftp_bytes_download_deadline_3272.py | 80 +++++++++++++++++++ 3 files changed, 97 insertions(+), 5 deletions(-) create mode 100644 backend/tests/unit/services/test_ftp_bytes_download_deadline_3272.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 45a407edd..eca692d6a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -161,6 +161,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 +- **A large timelapse could not be attached when it took more than 5 minutes to download (#3272, reported by @dovmesiz)** — Scanning for a timelapse, picking one by hand and the automatic attach after a print all gave the download a flat 300 seconds. A 75 MB video that took 358 seconds over a healthy P1S link failed with "Failed to download timelapse", and each retry started the file again from scratch. The time allowed now follows the file's size, so a slow transfer completes; a link that stops sending still fails after the FTP timeout as before. - **A printing queue job showed the slot AMS Filament Backup had switched away from as empty** — When a spool ran out mid-print and the printer carried on from the backup spool, the job's card in the queue still named the original slot, now marked **Empty**, as if the print had a problem. The card now shows the backup spool the printer is using. It follows the printer's own record of the trays the print has drawn from, including a backup that ran out in turn. A switch between two slots the job uses is a normal colour change and is not taken for a backup. Queued jobs still warn about an empty slot as before. - **Slice dialog hid renamed printer profiles while "Only printers that are online" was on (#3172, reported by @gregspatrick)** — The filter read the printer model from the profile's name, so a copy saved as `Bambu Lab P1S 0.4 nozzle - Copy` read as an unknown model and was hidden even with a P1S online, until **Show all** was picked. It now reads the model from the profile the copy was saved from, as the loaded-spools filter already did. - **Printer cards never showed the last print, nor the Good / Reject prompt for it** — Each card asked for its printer's archives with the arguments in the wrong order, so it requested none and always got an empty list. The **Last:** line on an idle card and the outcome prompt from #1898 never appeared there. diff --git a/backend/app/services/bambu_ftp.py b/backend/app/services/bambu_ftp.py index ccb978731..8b9b30c22 100644 --- a/backend/app/services/bambu_ftp.py +++ b/backend/app/services/bambu_ftp.py @@ -2281,14 +2281,19 @@ async def download_file_bytes_async( printer_model: Printer model for A1-specific workarounds timeout: overall async cap so a saturated ``_ftp_executor`` can't pin the caller (and any DB connection it holds) indefinitely (#2572). - Generous by default because this pulls whole files (timelapse - video, gcode) which can legitimately take minutes over slow Wi-Fi — - the cap only guards against a permanently-starved pool, not a - slow-but-progressing transfer. + Raised to fit ``expected_size`` at the pessimistic floor rate when + that is longer (#3272): a 75 MB timelapse took 358 s on a healthy + P1S link, and a flat cap cannot tell slow from stuck. A dead link + is still caught by ``socket_timeout``. expected_size: size from the directory listing; a mismatch fails the download instead of returning a truncated file. See :meth:`BambuFTPClient.download_file`. """ + # Unlike ``_download_extension`` there is no 300 s ceiling here: that one + # exists because ``on_print_start`` holds a DB session across its 3MF hunt. + # Every caller that passes a size releases its session before the transfer. + if expected_size and expected_size > 0: + timeout = max(timeout, expected_size / _DOWNLOAD_FLOOR_BYTES_PER_SEC) loop = asyncio.get_event_loop() def _download(): @@ -2303,7 +2308,13 @@ async def download_file_bytes_async( try: return await asyncio.wait_for(loop.run_in_executor(_ftp_executor, _download), timeout=timeout) except TimeoutError: - logger.warning("FTP download_bytes exceeded its %ss cap for %s (#2572)", timeout, ip_address) + logger.warning( + "FTP download of %s (%s bytes) exceeded its %.0fs cap for %s", + remote_path, + expected_size if expected_size else "unknown", + timeout, + ip_address, + ) return None diff --git a/backend/tests/unit/services/test_ftp_bytes_download_deadline_3272.py b/backend/tests/unit/services/test_ftp_bytes_download_deadline_3272.py new file mode 100644 index 000000000..fe3755831 --- /dev/null +++ b/backend/tests/unit/services/test_ftp_bytes_download_deadline_3272.py @@ -0,0 +1,80 @@ +"""The whole-file bytes download gives a big file time to arrive (#3272). + +A flat 300 s cap failed a 75 MB P1S timelapse that took 358 s on a healthy +link, and every retry started the file again from scratch. The deadline now +follows the listing's size at the pessimistic floor rate, with no ceiling: the +callers that pass a size hold no DB connection across the transfer. +""" + +import logging +from unittest.mock import MagicMock, patch + +import pytest + +from backend.app.services import bambu_ftp + +IP = "192.0.2.10" + + +async def _deadline_for(**kwargs) -> float: + """Run the download with a client that never connects; return its deadline.""" + seen: list[float] = [] + real_wait_for = bambu_ftp.asyncio.wait_for + + async def recording_wait_for(aw, timeout): + seen.append(timeout) + return await real_wait_for(aw, timeout) + + client = MagicMock() + client.connect.return_value = False + with ( + patch.object(bambu_ftp, "BambuFTPClient", return_value=client), + patch.object(bambu_ftp.asyncio, "wait_for", recording_wait_for), + ): + assert await bambu_ftp.download_file_bytes_async(IP, "12345678", "/timelapse/a.avi", **kwargs) is None + assert len(seen) == 1 + return seen[0] + + +@pytest.mark.asyncio +async def test_unknown_size_keeps_default_deadline(): + assert await _deadline_for() == 300.0 + + +@pytest.mark.asyncio +async def test_small_file_keeps_default_deadline(): + assert await _deadline_for(expected_size=2 * 1024 * 1024) == 300.0 + + +@pytest.mark.asyncio +async def test_large_file_deadline_follows_size_without_ceiling(): + # The reporter's largest timelapse; at the floor rate it needs ~94 minutes. + size = 143_788_322 + deadline = await _deadline_for(expected_size=size) + assert deadline == size / bambu_ftp._DOWNLOAD_FLOOR_BYTES_PER_SEC + assert deadline > bambu_ftp._DOWNLOAD_MAX_TIMEOUT + + +@pytest.mark.asyncio +async def test_explicit_longer_timeout_is_never_shortened(): + assert await _deadline_for(expected_size=1024, timeout=900.0) == 900.0 + + +@pytest.mark.asyncio +async def test_cap_log_names_file_and_size(caplog): + async def expire(aw, timeout): + # Let the worker finish while the client is still patched, so no real + # connection is attempted after the patch is gone. + await aw + raise TimeoutError + + client = MagicMock() + client.connect.return_value = False + with ( + patch.object(bambu_ftp, "BambuFTPClient", return_value=client), + patch.object(bambu_ftp.asyncio, "wait_for", expire), + caplog.at_level(logging.WARNING, logger=bambu_ftp.logger.name), + ): + result = await bambu_ftp.download_file_bytes_async(IP, "12345678", "/timelapse/a.avi", expected_size=75_647_046) + assert result is None + assert "/timelapse/a.avi (75647046 bytes) exceeded its 2955s cap" in caplog.text