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.
This commit is contained in:
maziggy
2026-05-17 14:03:23 +02:00
parent 74dcaf5142
commit 1fac027654
3 changed files with 74 additions and 4 deletions
+1
View File
@@ -8,6 +8,7 @@ All notable changes to Bambuddy will be documented in this file.
- **Settings → Filament: "Spool Catalog" now shows the same UI in Spoolman mode as in internal-inventory mode** — Previously, switching to Spoolman mode hijacked the Spool Catalog card and replaced it with a Spoolman filament list (Vendor — Name / Material / Weight / Spool Weight) with inline edit for name + spool_weight. Two separate concepts had been merged into one card: a Bambuddy-local **spool tare catalog** (the actual purpose of the card — name + weight definitions used to compute spool tare) vs a **filament editor** for Spoolman's `Filament` entity. The filament-editor view replaced the spool tare table entirely in Spoolman mode, with no way to see or manage the spool catalog. Now the card always renders the local Spool Catalog (Add / Edit / Delete / Export / Import / Reset / bulk-delete) regardless of inventory mode. The Spoolman-filament inline editor is removed — Spoolman users edit filament name / spool_weight in Spoolman's own UI. Side effect of the rewrite: the noisy `GET /api/v1/spoolman/inventory/filaments → 400 Bad Request` that fired on the Filament settings page even when Spoolman is disabled is gone, because the component no longer issues the probe at all. Files affected: `frontend/src/components/SpoolCatalogSettings.tsx` (rewrite, ~750 → ~445 lines), `frontend/src/components/SpoolWeightUpdateModal.tsx` (deleted — only used by the removed editor), test file rewritten to match the simplified component. No backend changes — `PATCH /spoolman/inventory/filaments/{id}` route still exists for API consumers, just no longer wired to a UI.
### Fixed
- **FTP upload no longer silently treats 426 "Failure reading network stream" as success (#1401, second root cause reported by @iitazz)** — Looking at the support bundle from @iitazz showed every FTP upload to their P2S (firmware 01.02.00.00) ending the same way: data channel sendall completes in ~200 ms at an impossibly high "speed" (7+ MB/s for files the printer can only actually receive at ~1–2 MB/s), then voidresp returns `426 Failure reading network stream. (error_temp)` from the printer, and Bambuddy proceeds — `WARNING FTP STOR confirmation not received for X (proceeding): 426 ...` followed immediately by `INFO FTP upload complete`. The print command then gets dispatched, the printer tries to parse what's actually a partial 3MF (the reporter's downloaded-from-printer 3MF was 458752 bytes — exactly `7 × 65536`, our FTP chunk size — for a 668025-byte source), and surfaces the "unable to parse 3mf file" error the reporter sees. Two stacked failures: a P2S firmware / TLS-data-channel quirk that severs the FTP data stream mid-transfer (separate investigation; #1401 doesn't fix that), AND the voidresp handler in `backend/app/services/bambu_ftp.py` swallowing the resulting 426 because the original comment assumed *"the data was fully sent so the file is likely on the SD card"* — true for socket-level timeouts where we just didn't HEAR the 226 in time (H2D needs 30+ s tolerance and we want to keep that), false for `426` where the printer is explicitly telling us *the data stream itself was cut*. Fix splits the broad `except Exception` into two branches: `except ftplib.Error` (covers `error_reply`, `error_temp`, `error_perm`, `error_proto` — the server *responded* with a failure on the control channel) logs at ERROR and re-raises, so the outer `except (OSError, ftplib.Error)` returns False and the dispatcher sees a real upload failure instead of green-lighting a print of a truncated file; `except Exception` keeps the existing proceed-with-warning behaviour for socket timeouts so the H2D 30-second voidresp tolerance survives. Same split applied to `upload_bytes()` since it had the same `except Exception: pass` shape. The reporter will still hit the underlying 426 (we haven't fixed the P2S transport problem yet — that's separate), but they'll now see an upload failure surfaced honestly rather than a confusing parse error 30 seconds into the print attempt. Tests: two new regressions in `TestUpload` patch `_ftp.voidresp` to raise `ftplib.error_temp("426 ...")` and assert both `upload_file()` and `upload_bytes()` return False. 18 upload-related tests green. The earlier-this-section validation fix is unrelated and stays — it still catches genuinely raw `.gcode` files at the upload step.
- **Upload validation rejects unprintable 3MF / raw-gcode files at the upload step instead of letting them fail at the printer (#1401, reported by @iitazz)** — Reporter sliced in OrcaSlicer, uploaded the result to Bambuddy, clicked Print, and the printer rejected with "Printing stopped because the printer was unable to parse the 3mf file" — every time, for multiple files, on both library uploads and SD-card-browsed files. Trace through the support bundle showed: (a) the stored library file ended in `.gcode` (not `.gcode.3mf`), and (b) `background_dispatch.py` constructs the FTP destination filename by appending `.3mf` when the source doesn't already end in `.gcode.3mf` / `.3mf` — so raw gcode gets shipped to the printer named `whatever.gcode.3mf` and the firmware's 3MF parser chokes on the missing zip header. The same shape also manifests as `Failed to parse plates from archive ... File is not a zip file` warnings on Bambuddy's side. Whether the user manually re-extensioned a file or their slicer saved as `.gcode` instead of `.gcode.3mf`, the right place to catch this is the upload, not the printer 30 seconds later. **New `validate_print_file_upload()` helper** in `backend/app/api/routes/library.py` runs two checks: (1) reject any filename ending in `.gcode` (but not `.gcode.3mf`) with a clear message — "Raw .gcode files can't be printed on Bambu printers in network mode — they need a .gcode.3mf zip container (gcode plus metadata). Re-export from your slicer and make sure the file ends in '.gcode.3mf', not just '.gcode'. If your OS hides extensions, double-check the file with the extension visible." (2) For any filename ending in `.3mf` (incl. the compound `.gcode.3mf`), verify the file body starts with `PK\x03\x04` (ZIP magic bytes); reject otherwise with a message pointing at the slicer's "Export Plate Sliced File" action. Suffix-based check rather than `os.path.splitext` because compound extensions like `.gcode.3mf` show up as just `.3mf` after splitext — both must trigger the same validation. **Applied to every relevant upload route**: `POST /library/files` (covers File Manager upload AND the printer-card drag-drop, which routes through the same endpoint), `POST /archives/upload` (single archive), `POST /archives/upload-bulk` (rejects bad files per-row instead of aborting the batch — one bad file in a 10-file drag-drop doesn't lose the other nine), `POST /archives/{archive_id}/source` (per-archive source 3MF), `POST /archives/upload-source` (slicer-post-processing match-by-name). Validation runs AFTER `_resolve_upload_destination` so folder-permission rejections (403 readonly, 400 missing-path, 409 collision) still take precedence — preserves existing error ordering. STL / image / other non-print uploads bypass the validator entirely; Bambuddy is also a library, not just a print dispatcher. **Frontend visibility fix** in `FileUploadModal.tsx` (same component used by File Manager + Printers page + Archives): the modal auto-closed after `setIsUploading(false)` regardless of per-file results, so a 400 rejection from the new validator was technically captured but never shown — the modal vanished too quickly. Now (a) errors render inline as red text under the file row instead of as a hover-only `title` tooltip, and (b) the modal stays open if any file ended with status='error', so the user can read the backend's actual remediation message before clicking Close. The bulk archive `UploadModal.tsx` was already showing inline errors and not auto-closing — that one didn't need the fix. **Tests**: 7 new integration tests in `TestPrintFileUploadValidation` cover: raw `.gcode` rejection at the library route (asserts the error message names the remedy), non-zip `.3mf` rejection, non-zip `.gcode.3mf` rejection (compound-extension code path), happy-path valid `.gcode.3mf` accepted, STL / non-print extensions still bypass, `POST /archives/upload` non-zip rejection, `POST /archives/upload-bulk` per-file error collection with mixed good/bad files in one request. Plus one fixture update in `test_external_folders_api.py` — `test_upload_persists_correct_db_shape` was uploading `model.3mf` with placeholder bytes `b"x"` to exercise the DB-shape path; updated to use a minimal real zip so the new validator doesn't block the unrelated test. 4968 backend tests green, 41 FileUploadModal frontend tests green, ruff + frontend build clean.
### Added
+30 -4
View File
@@ -447,9 +447,24 @@ class BambuFTPClient:
logger.info("FTP STOR confirmed for %s: %s", remote_path, resp.strip())
finally:
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:
# 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(
"FTP STOR confirmation not received for %s (proceeding): %s (%s)",
remote_path,
@@ -527,7 +542,10 @@ class BambuFTPClient:
conn.close()
except OSError:
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:
old_timeout = self._ftp.sock.gettimeout()
self._ftp.sock.settimeout(max(self.timeout, 60))
@@ -535,8 +553,16 @@ class BambuFTPClient:
self._ftp.voidresp()
finally:
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:
pass # Best-effort — data was sent, proceed
pass # Timeout / socket-level — proceed, data was sent.
return True
except (OSError, ftplib.Error):
return False
@@ -386,6 +386,49 @@ class TestUpload:
assert result is False
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):
"""upload_bytes() writes data to server."""
data = b"Bytes upload content"