Procházet zdrojové kódy

fix(ftp): raise on ftplib.Error from voidresp instead of proceeding

  bambu_ftp.upload_file (and upload_bytes) wrapped the voidresp() call in a
  broad "except Exception: log warning and proceed" because H2D printers
  can take 30+ seconds to send the 226 and we don't want to fail on that.
  But the same handler was swallowing ftplib.error_temp (e.g. 426 "Failure
  reading network stream") from buggy printer firmware, which explicitly
  means the data stream was cut mid-transfer and the file on the SD card
  is partial.

  Bambuddy then sent the print command anyway, and the printer surfaced a
  generic "unable to parse 3mf file" error 30 seconds into the print
  attempt -- with nothing in the log on the user side to suggest the
  upload had actually failed.

  Split the catch: ftplib.Error subclasses (server-reported failure)
  re-raise so the outer handler returns False; everything else (socket
  timeout etc.) keeps the existing proceed-with-warning behaviour so the
  H2D 226 tolerance survives.

  Two regression tests patch _ftp.voidresp to raise error_temp("426 ...")
  and assert both upload_file() and upload_bytes() return False.

  The underlying P2S firmware / TLS-data-channel issue that triggers the
  426 for the reporter is separate -- this change just stops Bambuddy from
  hiding it.
maziggy před 1 týdnem
rodič
revize
1fac027654

Rozdílová data souboru nebyla zobrazena, protože soubor je příliš velký
+ 0 - 0
CHANGELOG.md


+ 30 - 4
backend/app/services/bambu_ftp.py

@@ -447,9 +447,24 @@ class BambuFTPClient:
                     logger.info("FTP STOR confirmed for %s: %s", remote_path, resp.strip())
                     logger.info("FTP STOR confirmed for %s: %s", remote_path, resp.strip())
                 finally:
                 finally:
                     self._ftp.sock.settimeout(old_timeout)
                     self._ftp.sock.settimeout(old_timeout)
+            except ftplib.Error as e:
+                # The printer's FTP server explicitly told us the transfer
+                # failed (e.g. 426 "Failure reading network stream" seen on
+                # some P2S firmware revisions). The file on the SD card is
+                # truncated — never report success or send a print command
+                # for it. Re-raise so the outer handler returns False.
+                logger.error(
+                    "FTP STOR rejected by printer for %s: %s (%s)",
+                    remote_path,
+                    e,
+                    type(e).__name__,
+                )
+                raise
             except Exception as e:
             except Exception as e:
-                # Timeout or error reading 226 — log but proceed, the data
-                # was fully sent so the file is likely on the SD card.
+                # Timeout or socket-level error reading 226 — the data was sent
+                # on our side and the printer may still have written the file.
+                # H2D can take 30+ seconds to send 226 after the data channel
+                # closes, so we proceed with a warning rather than failing here.
                 logger.warning(
                 logger.warning(
                     "FTP STOR confirmation not received for %s (proceeding): %s (%s)",
                     "FTP STOR confirmation not received for %s (proceeding): %s (%s)",
                     remote_path,
                     remote_path,
@@ -527,7 +542,10 @@ class BambuFTPClient:
                     conn.close()
                     conn.close()
                 except OSError:
                 except OSError:
                     pass
                     pass
-            # Wait for 226 confirmation (see upload_file for rationale)
+            # Wait for 226 confirmation (see upload_file for rationale).
+            # ftplib.Error subclasses (e.g. 426 error_temp) mean the server
+            # rejected the transfer and the file is partial — fail. Other
+            # exceptions (timeout, socket-level) are tolerated as in upload_file.
             try:
             try:
                 old_timeout = self._ftp.sock.gettimeout()
                 old_timeout = self._ftp.sock.gettimeout()
                 self._ftp.sock.settimeout(max(self.timeout, 60))
                 self._ftp.sock.settimeout(max(self.timeout, 60))
@@ -535,8 +553,16 @@ class BambuFTPClient:
                     self._ftp.voidresp()
                     self._ftp.voidresp()
                 finally:
                 finally:
                     self._ftp.sock.settimeout(old_timeout)
                     self._ftp.sock.settimeout(old_timeout)
+            except ftplib.Error as e:
+                logger.error(
+                    "FTP STOR rejected by printer for %s: %s (%s)",
+                    remote_path,
+                    e,
+                    type(e).__name__,
+                )
+                return False
             except Exception:
             except Exception:
-                pass  # Best-effort — data was sent, proceed
+                pass  # Timeout / socket-level — proceed, data was sent.
             return True
             return True
         except (OSError, ftplib.Error):
         except (OSError, ftplib.Error):
             return False
             return False

+ 43 - 0
backend/tests/unit/services/test_bambu_ftp.py

@@ -386,6 +386,49 @@ class TestUpload:
         assert result is False
         assert result is False
         client.disconnect()
         client.disconnect()
 
 
+    def test_upload_426_data_stream_failure_returns_false(self, ftp_client_factory, ftp_server, tmp_path):
+        """426 'Failure reading network stream' from voidresp() must fail.
+
+        Regression for #1401: P2S firmware 01.02.00.00 (and possibly other
+        Bambu firmware revisions) returns 426 after the data channel closes,
+        indicating the printer received only a partial file. Previously the
+        client logged a warning and returned True, so the dispatcher sent a
+        print command for a truncated 3MF and the printer surfaced a
+        confusing 'unable to parse 3mf file' error. The 426 must instead
+        cause the upload to return False.
+        """
+        import ftplib
+
+        local = tmp_path / "test.bin"
+        local.write_bytes(b"data" * 256)
+        client = ftp_client_factory()
+        client.connect()
+
+        def raise_426():
+            raise ftplib.error_temp("426 Failure reading network stream.")
+
+        client._ftp.voidresp = raise_426
+
+        result = client.upload_file(local, "/cache/test.bin")
+        assert result is False, "Upload must fail on 426 to prevent dispatching a truncated file"
+        client.disconnect()
+
+    def test_upload_bytes_426_data_stream_failure_returns_false(self, ftp_client_factory, ftp_server):
+        """upload_bytes() also fails on 426 (same root cause as upload_file)."""
+        import ftplib
+
+        client = ftp_client_factory()
+        client.connect()
+
+        def raise_426():
+            raise ftplib.error_temp("426 Failure reading network stream.")
+
+        client._ftp.voidresp = raise_426
+
+        result = client.upload_bytes(b"x" * 1024, "/cache/bytes.bin")
+        assert result is False
+        client.disconnect()
+
     def test_upload_bytes_success(self, ftp_client_factory, ftp_server):
     def test_upload_bytes_success(self, ftp_client_factory, ftp_server):
         """upload_bytes() writes data to server."""
         """upload_bytes() writes data to server."""
         data = b"Bytes upload content"
         data = b"Bytes upload content"

Některé soubory nejsou zobrazeny, neboť je v těchto rozdílových datech změněno mnoho souborů