Kaynağa Gözat

Give a large timelapse download time to finish instead of a flat 300 s (#3272)

maziggy 1 hafta önce
ebeveyn
işleme
fb0aebc6f6

+ 1 - 0
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.

+ 16 - 5
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
 
 

+ 80 - 0
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