Say why an upload failed instead of blaming the SD card (issue #2899)

Every dispatch upload that failed carried one sentence: "Failed to
upload file to printer. Check if SD card is inserted and properly
formatted (FAT32/exFAT)." The reporter got it after a TLS handshake
failure and restarted the printer on the strength of it. That could not
have helped. The handshake never reached the printer's filesystem, and
the cool-off that made the next dispatch fail the same way lives in
Bambuddy's own memory, where power-cycling a printer does not reach.

because the advice was known not to work. It survived in the string
people actually read, so the failure mode that fix closed was still
reachable through the UI.
reachable through the UI.

The information was never missing. connect() separates five failure
classes and upload_file() separates 553/552/550, each with its own log
line -- 553 even logs a spelled-out list of storage causes -- and then
both returned a bare False. The dispatch had nothing left to work with
and guessed storage for all of them.

So the reason now travels with the result. The client records an
FtpFailure (kind, detail, and the reply code where the server gave one)
on every failure branch, and upload_file_async fills in a report object
the CALLER owns. Not a per-IP dict beside _mode_cache: those describe a
printer and are right to share, while this describes one operation, and
a background timelapse fetch running beside a dispatch would overwrite
the dispatch's reason with its own -- reporting the wrong cause with
total confidence, which is this bug again rather than a fix for it.

describe_upload_failure() picks the wording, and lives next to the kinds
so the two cannot drift. A 553 or 552 keeps the card advice, which is
the case it was written for, and quotes the reply code so a queue entry
and a support bundle line up. A handshake failure says the file service
answered without TLS and that the card is not involved. A refusal points
at the access code, a timeout at the network, and anything unclassified
says so and points at the log rather than picking a plausible cause. A
wrong instruction costs more than a vague one: it sends someone to work
on hardware that is fine. Nothing prescribes a power cycle.

The failure notification now carries the same sentence the queue shows,
instead of its own fixed "Failed to upload file to printer", so a push
and the screen cannot disagree about what happened.

Tests cover the classification, the report reaching the caller through
the retry loop, two callers not crossing, the queue entry itself, and
one line per message branch -- without that last one, deleting the
access-code or timeout branch left every other assertion passing, since
they only check that the card is not named and the generic message
satisfies that too. The card-advice test keys on the instruction rather
than the words "SD card", because the handshake message names the card
in order to rule it out.

backend/app/services/bambu_ftp.py

@@ -9,6 +9,7 @@ import time
 import weakref
 from collections.abc import Awaitable, Callable
 from concurrent.futures import ThreadPoolExecutor
+from dataclasses import dataclass
 from enum import Enum
 from ftplib import FTP, FTP_TLS  # nosec B402
 from io import BytesIO
@@ -119,6 +120,65 @@ class DeleteResult(Enum):
 _HANDSHAKE_COOLOFF_SECONDS = 300.0
 
 
+def _ftp_reply_code(error: BaseException) -> str | None:
+    """The three-digit reply code an ftplib error carries, if it carries one.
+
+    ``ftplib`` puts the server's whole reply line in the exception message, so
+    the code is the first token: "553 Could not create file." Anything that is
+    not three digits (an ``OSError``, a library-side message) has no code, and
+    saying so beats inventing one.
+    """
+    head = str(error)[:3]
+    return head if head.isdigit() else None
+
+
+class FtpFailureKind(Enum):
+    """Why an FTP operation failed, at the granularity the client can tell.
+
+    ``connect`` and ``upload_file`` already separate every one of these -- each
+    has its own log line, and 553 even gets a spelled-out list of storage
+    causes -- and then both returned a bare ``False``. So the dispatch that
+    reports the failure to the operator had nothing to go on, and used one
+    string for all of them: "check if SD card is inserted and properly
+    formatted". #2899's reporter acted on that after a TLS handshake failure
+    and restarted the printer, which could not have helped: the handshake never
+    got near the printer's filesystem.
+    """
+
+    COOLOFF = "cooloff"  # skipped without contacting the printer (#2780)
+    HANDSHAKE = "handshake"  # port 990 answered with something that is not TLS
+    AUTH = "auth"  # permanent refusal, typically a rejected access code
+    TIMEOUT = "timeout"
+    STORAGE = "storage"  # 553/552 -- the case the SD-card advice was written for
+    NOT_FOUND = "not_found"  # 550
+    NETWORK = "network"  # socket dropped, or an FTP error with no clearer reading
+    UNKNOWN = "unknown"
+
+
+@dataclass(frozen=True)
+class FtpFailure:
+    """What went wrong, kept next to the log line that already said it."""
+
+    kind: FtpFailureKind
+    detail: str
+    code: str | None = None  # FTP reply code where the server gave one
+
+
+@dataclass
+class FtpFailureReport:
+    """A slot the *caller* owns for the reason its upload failed.
+
+    Deliberately not a per-IP dict on the client, the way ``_mode_cache`` and
+    ``_handshake_blocked_until`` are. Those describe a printer, and are
+    correct to share. This describes one operation, and a background timelapse
+    fetch running beside a dispatch would overwrite the dispatch's reason with
+    its own -- reporting the wrong cause with total confidence, which is the
+    bug being fixed rather than a new way to hit it (#2899).
+    """
+
+    failure: FtpFailure | None = None
+
+
 class FileNotOnPrinterError(Exception):
     """Raised when a remote FTP path returns 550 (file not found).
 
@@ -256,6 +316,10 @@ class BambuFTPClient:
         self.printer_model = printer_model
         self.force_prot_c = force_prot_c
         self.respect_handshake_cooloff = respect_handshake_cooloff
+        # Why the last connect/upload on this client failed, for a caller that
+        # only gets a bool back (#2899). Per instance, so it describes one
+        # operation and cannot be overwritten by work against another printer.
+        self.last_failure: FtpFailure | None = None
         self._ftp: ImplicitFTP_TLS | None = None
 
     def _is_a1_model(self) -> bool:
@@ -312,6 +376,7 @@ class BambuFTPClient:
         the cool-off a previous TLS handshake failure opened (#2780) -- unless
         this client was built with ``respect_handshake_cooloff=False``.
         """
+        self.last_failure = None
         if self.respect_handshake_cooloff and self.handshake_blocked(self.ip_address):
             # WARNING, not DEBUG. This is the one connect() failure path that
             # reported without its cause, so at default log level four
@@ -338,6 +403,10 @@ class BambuFTPClient:
                     self.ip_address,
                     remaining,
                 )
+            self.last_failure = FtpFailure(
+                FtpFailureKind.COOLOFF,
+                f"cooling off for another {remaining:.0f}s after a recent FTPS handshake failure",
+            )
             return False
         try:
             use_prot_c = self._should_use_prot_c()
@@ -374,10 +443,12 @@ class BambuFTPClient:
             return True
         except ftplib.error_perm as e:
             logger.warning("FTP connection permission error to %s: %s", self.ip_address, e)
+            self.last_failure = FtpFailure(FtpFailureKind.AUTH, str(e), _ftp_reply_code(e))
             self._abandon_connection()
             return False
         except TimeoutError as e:
             logger.warning("FTP connection timed out to %s: %s", self.ip_address, e)
+            self.last_failure = FtpFailure(FtpFailureKind.TIMEOUT, str(e))
             self._abandon_connection()
             return False
         except ssl.SSLError as e:
@@ -402,10 +473,12 @@ class BambuFTPClient:
                 _HANDSHAKE_COOLOFF_SECONDS,
             )
             self._handshake_blocked_until[self.ip_address] = time.monotonic() + _HANDSHAKE_COOLOFF_SECONDS
+            self.last_failure = FtpFailure(FtpFailureKind.HANDSHAKE, str(e))
             self._abandon_connection()
             return False
         except (OSError, ftplib.Error) as e:
             logger.warning("FTP connection failed to %s: %s (type: %s)", self.ip_address, e, type(e).__name__)
+            self.last_failure = FtpFailure(FtpFailureKind.NETWORK, str(e), _ftp_reply_code(e))
             self._abandon_connection()
             return False
 
@@ -644,8 +717,10 @@ class BambuFTPClient:
         progress_callback: Callable[[int, int], None] | None = None,
     ) -> bool:
         """Upload a file to the printer with optional progress callback."""
+        self.last_failure = None
         if not self._ftp:
             logger.warning("upload_file: FTP not connected")
+            self.last_failure = FtpFailure(FtpFailureKind.UNKNOWN, "no FTP connection")
             return False
 
         try:
@@ -797,19 +872,30 @@ class BambuFTPClient:
             # Permanent FTP error (4xx/5xx response)
             error_code = str(e)[:3] if str(e) else "unknown"
             logger.error("FTP upload failed for %s: %s (error code: %s)", remote_path, e, error_code)
+            # 553 and 552 are the printer telling us about its own storage --
+            # the one case where advice about the card is worth giving, and
+            # the case the dispatch's blanket SD-card message was written for
+            # before it was applied to every failure alike (#2899).
             if error_code == "553":
                 logger.error(
                     "FTP 553 error - Could not create file. Possible causes: "
                     "1) No SD card inserted, 2) SD card full, 3) SD card not formatted correctly (needs FAT32/exFAT), "
                     "4) Printer busy/not ready, 5) File path issue"
                 )
+                kind = FtpFailureKind.STORAGE
             elif error_code == "550":
                 logger.error("FTP 550 error - File/directory not found or permission denied")
+                kind = FtpFailureKind.NOT_FOUND
             elif error_code == "552":
                 logger.error("FTP 552 error - Storage quota exceeded (SD card full?)")
+                kind = FtpFailureKind.STORAGE
+            else:
+                kind = FtpFailureKind.UNKNOWN
+            self.last_failure = FtpFailure(kind, str(e), _ftp_reply_code(e))
             return False
         except (OSError, ftplib.Error) as e:
             logger.error("FTP upload failed for %s: %s (type: %s)", remote_path, e, type(e).__name__)
+            self.last_failure = FtpFailure(FtpFailureKind.NETWORK, str(e), _ftp_reply_code(e))
             return False
 
     def upload_bytes(self, data: bytes, remote_path: str) -> bool:
@@ -969,16 +1055,63 @@ class BambuFTPClient:
         return result if result else None
 
 
-def ftps_handshake_cooloff_deadline(ip_address: str) -> float | None:
-    """The monotonic deadline of this printer's handshake cool-off, or None.
+def describe_upload_failure(failure: FtpFailure | None) -> str:
+    """One sentence for the operator, chosen from what actually went wrong.
+
+    Every upload failure used to get the same one: "Failed to upload file to
+    printer. Check if SD card is inserted and properly formatted
+    (FAT32/exFAT)." #2899's reporter got that after a TLS handshake failure and
+    restarted the printer, which could not have helped -- the handshake never
+    reached the printer's filesystem, and the state that produced it lives in
+    Bambuddy's own memory. #2780 had already removed advice from this failure's
+    *log* line for the same reason; it survived in the string people read.
 
-    Compare it across an operation to tell "a handshake failed while I was
-    working" from "this printer was already cooling off from something else".
-    Those need different words to the user, and the flag's presence alone
-    cannot separate them: a caller that opted out of the cool-off can be
-    running while an unrelated background fetch has one armed (#2898).
+    So the card is named only where the printer itself raised storage, and
+    where nothing here can say more, this says so and points at the log rather
+    than picking a plausible cause. A wrong instruction costs more than a
+    vague one: it sends someone to work on hardware that is fine.
     """
-    return BambuFTPClient._handshake_blocked_until.get(ip_address)
+    if failure is None:
+        return (
+            "Could not upload the file to the printer. See the server log for the reason — "
+            "it records what the printer's file service said."
+        )
+
+    if failure.kind is FtpFailureKind.STORAGE:
+        return (
+            f"The printer refused to store the file ({failure.code or 'storage error'}). Check that its SD card "
+            "is inserted, has space free, and is formatted FAT32 or exFAT."
+        )
+    if failure.kind is FtpFailureKind.HANDSHAKE:
+        return (
+            "The printer's file service answered, but not with TLS, so no file could be sent to it. "
+            "Its SD card is not involved. This usually clears by itself; if it does not, power-cycling "
+            "the printer has not been found to help either, so please report it."
+        )
+    if failure.kind is FtpFailureKind.COOLOFF:
+        return (
+            "Bambuddy is holding off from this printer's file service after a recent failed TLS handshake, "
+            "so the file was not sent. This clears on its own within a few minutes."
+        )
+    if failure.kind is FtpFailureKind.AUTH:
+        return (
+            "The printer refused the file transfer connection. If the printer's access code changed, "
+            "update it on Bambuddy's Printers page."
+        )
+    if failure.kind is FtpFailureKind.TIMEOUT:
+        return (
+            "The printer's file service did not respond in time, so the file was not sent. "
+            "Check that the printer is on the network and reachable."
+        )
+    if failure.kind is FtpFailureKind.NOT_FOUND:
+        return (
+            "The printer rejected the upload path (550). See the server log — this is a Bambuddy-side "
+            "problem, not something to fix on the printer."
+        )
+    return (
+        "Could not upload the file to the printer. See the server log for the reason — "
+        "it records what the printer's file service said."
+    )
 
 
 def ftps_handshake_blocked(ip_address: str) -> bool:
@@ -1303,6 +1436,7 @@ async def upload_file_async(
     socket_timeout: float | None = None,
     printer_model: str | None = None,
     respect_handshake_cooloff: bool = True,
+    failure: FtpFailureReport | None = None,
 ) -> bool:
     """Async wrapper for uploading a file with timeout and progress callback.
 
@@ -1323,6 +1457,11 @@ async def upload_file_async(
         respect_handshake_cooloff: see ``BambuFTPClient.__init__``. False for a
             user-initiated upload, whose attempts are bounded and were being
             spent against a cool-off that outlives them (#2898).
+        failure: caller-owned slot filled in with why the upload failed, so the
+            caller can say something true about it instead of guessing (#2899).
+            Passed through ``with_ftp_retry`` unchanged, so it ends up holding
+            the last attempt's reason -- which is the one that decided the
+            outcome.
     """
     loop = asyncio.get_event_loop()
     is_a1 = printer_model in BambuFTPClient.A1_MODELS if printer_model else False
@@ -1351,18 +1490,25 @@ async def upload_file_async(
             force_prot_c=force_prot_c,
             respect_handshake_cooloff=respect_handshake_cooloff,
         )
-        if client.connect():
-            logger.info("FTP connected to %s", ip_address)
-            try:
-                result = client.upload_file(local_path, remote_path, _guarded_progress)
-                if result:
-                    # Cache the working mode
-                    BambuFTPClient.cache_mode(ip_address, mode_str)
-                return result
-            finally:
-                client.disconnect()
-        logger.warning("FTP connection failed to %s", ip_address)
-        return False
+        try:
+            if client.connect():
+                logger.info("FTP connected to %s", ip_address)
+                try:
+                    result = client.upload_file(local_path, remote_path, _guarded_progress)
+                    if result:
+                        # Cache the working mode
+                        BambuFTPClient.cache_mode(ip_address, mode_str)
+                    return result
+                finally:
+                    client.disconnect()
+            logger.warning("FTP connection failed to %s", ip_address)
+            return False
+        finally:
+            # In a finally so a transfer that leaves by raising -- a cancelled
+            # upload, a re-raised STOR rejection -- still reports what the
+            # client recorded on its way out.
+            if failure is not None and client.last_failure is not None:
+                failure.failure = client.last_failure
 
     async def _attempt(force_prot_c: bool) -> bool:
         """Run one upload attempt, and make a timeout actually stop the transfer.

backend/app/services/print_scheduler.py

@@ -30,10 +30,11 @@ from backend.app.models.spool_assignment import SpoolAssignment
 from backend.app.models.spoolman_slot_assignment import SpoolmanSlotAssignment
 from backend.app.services import drying_preflight, print_dispatch_context
 from backend.app.services.bambu_ftp import (
+    FtpFailureReport,
     UploadCancelled,
     cache_3mf_download,
     delete_file_async,
-    ftps_handshake_cooloff_deadline,
+    describe_upload_failure,
     get_ftp_retry_settings,
     upload_file_async,
     with_ftp_retry,
@@ -6094,14 +6095,6 @@ class PrintScheduler:
         # pending->printing CAS) transparently open a fresh transaction.
         await db.commit()
 
-        # Where this printer's handshake cool-off stood before we touched it.
-        # The delete and upload below ignore the cool-off, so finding one armed
-        # afterwards proves nothing on its own -- a background timelapse or 3MF
-        # fetch for an earlier print could have armed it minutes ago. A deadline
-        # that MOVED, though, can only mean a handshake failed during this
-        # dispatch, which is what the failure message needs to know (#2898).
-        cooloff_before = ftps_handshake_cooloff_deadline(printer.ip_address)
-
         # Delete existing file if present (avoids 553 error on overwrite)
         try:
             logger.debug("Queue item %s: Deleting existing file %s if present...", item.id, remote_path)
@@ -6149,6 +6142,11 @@ class PrintScheduler:
         # wrong advice for a link that was simply too slow to finish (#2529).
         upload_error: str | None = None
 
+        # Why the upload failed, straight from the client rather than inferred.
+        # Owned here, so a background fetch for another print cannot overwrite
+        # it between the failure and the sentence built from it (#2899).
+        upload_failure = FtpFailureReport()
+
         try:
             if ftp_retry_enabled:
                 uploaded = await with_ftp_retry(
@@ -6161,6 +6159,7 @@ class PrintScheduler:
                     printer_model=printer.model,
                     progress_callback=progress_bridge,
                     respect_handshake_cooloff=False,
+                    failure=upload_failure,
                     max_retries=ftp_retry_count,
                     retry_delay=ftp_retry_delay,
                     operation_name=f"Upload print to {printer.name}",
@@ -6175,6 +6174,7 @@ class PrintScheduler:
                     printer_model=printer.model,
                     progress_callback=progress_bridge,
                     respect_handshake_cooloff=False,
+                    failure=upload_failure,
                 )
         except UploadCancelled as e:
             uploaded = False
@@ -6192,24 +6192,12 @@ class PrintScheduler:
             injected_path.unlink(missing_ok=True)
 
         if not uploaded:
-            # A cool-off armed during this dispatch is proof the printer
-            # answered port 990 with something that was not TLS, so the SD card
-            # is the wrong thing to go and look at -- and it is what three of
-            # #2898's queue items were sent to check. "During this dispatch" is
-            # load-bearing: an unrelated background fetch can leave a cool-off
-            # armed for minutes, and blaming TLS for an upload that actually hit
-            # a full disk would repeat the mistake in the other direction.
-            cooloff_after = ftps_handshake_cooloff_deadline(printer.ip_address)
-            if not upload_error and cooloff_after is not None and cooloff_after != cooloff_before:
-                upload_error = (
-                    "The printer's file service did not answer over TLS, so the file could not be sent to it. "
-                    "Its SD card is not involved. Bambuddy will leave this printer alone for a few minutes "
-                    "before trying again."
-                )
-            error_msg = upload_error or (
-                "Failed to upload file to printer. Check if SD card is inserted and properly formatted (FAT32/exFAT). "
-                "See server logs for detailed diagnostics."
-            )
+            # This used to be one string for every upload failure, telling
+            # everyone to check the SD card. The client knows which of seven
+            # things went wrong and logs each one differently; it just had no
+            # way to say so here, so the card got named even for a TLS
+            # handshake that never reached the printer's filesystem (#2899).
+            error_msg = upload_error or describe_upload_failure(upload_failure.failure)
             item.status = "failed"
             item.error_message = error_msg
             item.completed_at = datetime.now(timezone.utc)
@@ -6224,7 +6212,9 @@ class PrintScheduler:
                 job_name=filename.replace(".gcode.3mf", "").replace(".3mf", ""),
                 printer_id=printer.id,
                 printer_name=printer.name,
-                reason="Failed to upload file to printer",
+                # The same sentence the queue shows. A push notification saying
+                # something different from the UI is its own small bug (#2899).
+                reason=error_msg,
                 db=db,
             )
             try:

backend/tests/unit/services/test_ftp_cooloff_retry_budget_2898.py

@@ -20,10 +20,7 @@ original report a log dive.
 
 import logging
 import ssl
-from contextlib import ExitStack
-from pathlib import Path
-from types import SimpleNamespace
-from unittest.mock import AsyncMock, MagicMock, patch
+from unittest.mock import MagicMock, patch
 
 import pytest
 
@@ -265,148 +262,3 @@ class TestDispatchKeepsItsAttempts:
         _arm()
         assert await bambu_ftp.download_file_bytes_async(IP, "12345678", "/timelapse/a.mp4") is None
         assert refusing_printer.connect.call_count == 0
-
-
-# ---------------------------------------------------------------------------
-# What the operator is told
-# ---------------------------------------------------------------------------
-@pytest.fixture
-async def dispatch_case(tmp_path):
-    """Minimal one-printer, one-queued-job database for ``_start_print``."""
-    from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine
-
-    import backend.app.models  # noqa: F401 - populate Base.metadata
-    from backend.app.core.database import Base
-    from backend.app.models.archive import PrintArchive
-    from backend.app.models.print_queue import PrintQueueItem
-    from backend.app.models.printer import Printer
-
-    engine = create_async_engine("sqlite+aiosqlite:///:memory:", echo=False)
-    async with engine.begin() as conn:
-        await conn.run_sync(Base.metadata.create_all)
-    session_maker = async_sessionmaker(engine, expire_on_commit=False)
-
-    base_dir = tmp_path / "case"
-    archive_rel = Path("archives") / "job.3mf"
-    archive_abs = base_dir / archive_rel
-    archive_abs.parent.mkdir(parents=True, exist_ok=True)
-    archive_abs.write_bytes(b"archive payload")
-
-    async with session_maker() as db:
-        printer = Printer(
-            name="Bambulab P2S-4",
-            serial_number="SERIAL",
-            ip_address=IP,
-            access_code="12345678",
-            model="P2S",
-        )
-        db.add(printer)
-        await db.flush()
-        archive = PrintArchive(
-            printer_id=printer.id,
-            filename="job.3mf",
-            file_path=str(archive_rel),
-            file_size=archive_abs.stat().st_size,
-            status="completed",
-        )
-        db.add(archive)
-        await db.flush()
-        item = PrintQueueItem(printer_id=printer.id, archive_id=archive.id, status="pending")
-        db.add(item)
-        await db.commit()
-        item_id = item.id
-
-    try:
-        yield SimpleNamespace(session_maker=session_maker, base_dir=base_dir, item_id=item_id)
-    finally:
-        await engine.dispose()
-
-
-async def _failed_dispatch_message(dispatch_case, *, handshake_fails: bool) -> str:
-    """Run one dispatch whose upload fails, and return what the user is told.
-
-    ``handshake_fails`` makes the stand-in upload arm the cool-off the way the
-    real one does when the printer answers port 990 with something other than
-    TLS -- which is the only thing that separates the two messages.
-    """
-    import backend.app.services.print_scheduler as scheduler_module
-    from backend.app.models.print_queue import PrintQueueItem
-    from backend.app.services.print_scheduler import PrintScheduler
-    from backend.tests._fixtures.background_tasks import discarding_spawn_patch
-
-    async def _upload(*_args, **_kwargs):
-        if handshake_fails:
-            _arm()
-        return False
-
-    scheduler = PrintScheduler()
-    async with dispatch_case.session_maker() as db:
-        item = await db.get(PrintQueueItem, dispatch_case.item_id)
-        patches = [
-            patch.object(scheduler_module.settings, "base_dir", dispatch_case.base_dir),
-            patch("backend.app.services.print_scheduler.printer_manager.is_connected", MagicMock(return_value=True)),
-            patch("backend.app.services.print_scheduler.printer_manager.get_status", MagicMock(return_value=None)),
-            patch(
-                "backend.app.services.print_scheduler.get_ftp_retry_settings",
-                AsyncMock(return_value=(False, 0, 0, 1.0)),
-            ),
-            patch("backend.app.services.print_scheduler.delete_file_async", AsyncMock(return_value=True)),
-            patch("backend.app.services.print_scheduler.upload_file_async", _upload),
-            patch("backend.app.services.print_scheduler.notification_service.on_queue_job_failed", AsyncMock()),
-            discarding_spawn_patch(),
-            patch.object(scheduler, "_propagate_owner_to_printer_manager", AsyncMock()),
-            patch.object(scheduler, "_power_off_if_needed", AsyncMock()),
-            patch.object(scheduler, "_preheat_and_soak", AsyncMock()),
-        ]
-        with ExitStack() as stack:
-            for p in patches:
-                stack.enter_context(p)
-            await scheduler._start_print(db, item)
-
-        refreshed = await db.get(PrintQueueItem, dispatch_case.item_id)
-        assert refreshed.status == "failed"
-        return refreshed.error_message or ""
-
-
-class TestTheFailureNamesTheRightHardware:
-    async def test_a_handshake_failure_does_not_send_anyone_to_the_sd_card(self, dispatch_case):
-        """Three of the report's queue items were told to check an SD card.
-
-        The printer had answered port 990 with something that was not TLS.
-        Nothing had reached its filesystem, so its card could not have been
-        the problem, and the operator was sent to look at the one part of the
-        machine that was working.
-        """
-        message = await _failed_dispatch_message(dispatch_case, handshake_fails=True)
-
-        assert "SD card is inserted" not in message, message
-        assert "did not answer over TLS" in message, message
-        # It goes further than dropping the advice: the card is the first thing
-        # anyone would reach for next, so the message rules it out by name.
-        assert "SD card is not involved" in message, message
-
-    async def test_an_ordinary_upload_failure_keeps_the_storage_advice(self, dispatch_case):
-        """No cool-off means no evidence about TLS, so the old wording stands.
-
-        This is the half that stops the new message from swallowing every
-        upload failure: the cool-off is read as evidence, not assumed.
-        """
-        assert BambuFTPClient.handshake_blocked(IP) is False
-        message = await _failed_dispatch_message(dispatch_case, handshake_fails=False)
-
-        assert "SD card is inserted" in message, message
-
-    async def test_a_cooloff_left_by_something_else_is_not_taken_as_evidence(self, dispatch_case):
-        """The printer can be cooling off from work this dispatch had no part in.
-
-        A background timelapse or 3MF fetch for an earlier print arms the same
-        gate, and it lasts five minutes. Since the dispatch ignores the gate, an
-        upload running underneath it can still fail on a full disk -- and
-        answering that with "the file service did not answer over TLS" would be
-        the same wrong-hardware mistake pointing the other way.
-        """
-        _arm()  # armed before the dispatch, and nothing re-arms it during
-        message = await _failed_dispatch_message(dispatch_case, handshake_fails=False)
-
-        assert "did not answer over TLS" not in message, message
-        assert "SD card is inserted" in message, message

backend/tests/unit/services/test_upload_failure_reason_2899.py

@@ -0,0 +1,440 @@
+"""Say what actually went wrong, not what usually does (#2899).
+
+Every failed dispatch upload used to carry the same sentence: "Failed to upload
+file to printer. Check if SD card is inserted and properly formatted
+(FAT32/exFAT)." The reporter got it after a TLS handshake failure and restarted
+the printer on the strength of it. That could not have helped -- the handshake
+never reached the printer's filesystem, and the cool-off that produced the
+repeat failure lives in Bambuddy's own memory, where power-cycling a printer
+does not reach.
+
+#2780 had already removed operator advice from this failure's *log* line, for
+exactly this reason. The advice survived in the string people actually read.
+
+The information was never missing. ``connect`` separates five failure classes
+and ``upload_file`` separates 553/552/550, each with its own log line -- and
+both then returned a bare ``False``. These tests pin the reason travelling out
+to the caller, and the card being named only where the printer itself raised
+storage.
+"""
+
+import ftplib  # nosec B402 -- tests construct real ftplib error types
+import ssl
+import time
+from contextlib import ExitStack
+from pathlib import Path
+from types import SimpleNamespace
+from unittest.mock import AsyncMock, MagicMock, patch
+
+import pytest
+
+from backend.app.services.bambu_ftp import (
+    BambuFTPClient,
+    FtpFailure,
+    FtpFailureKind,
+    FtpFailureReport,
+    describe_upload_failure,
+    upload_file_async,
+    with_ftp_retry,
+)
+
+pytestmark = pytest.mark.unit
+
+IP = "192.168.50.142"
+
+
+@pytest.fixture(autouse=True)
+def _clean_state():
+    BambuFTPClient._handshake_blocked_until.clear()
+    BambuFTPClient._handshake_skip_logged.clear()
+    BambuFTPClient._mode_cache.clear()
+    yield
+    BambuFTPClient._handshake_blocked_until.clear()
+    BambuFTPClient._handshake_skip_logged.clear()
+    BambuFTPClient._mode_cache.clear()
+
+
+# ---------------------------------------------------------------------------
+# The client records which of its own branches it took
+# ---------------------------------------------------------------------------
+@pytest.mark.parametrize(
+    ("error", "kind", "code"),
+    [
+        (ssl.SSLError("[SSL: WRONG_VERSION_NUMBER] wrong version number"), FtpFailureKind.HANDSHAKE, None),
+        (TimeoutError("handshake operation timed out"), FtpFailureKind.TIMEOUT, None),
+        (ftplib.error_perm("530 Login incorrect."), FtpFailureKind.AUTH, "530"),
+        (OSError("Connection reset by peer"), FtpFailureKind.NETWORK, None),
+    ],
+    ids=["handshake", "timeout", "auth", "network"],
+)
+def test_connect_records_which_failure_it_hit(error, kind, code):
+    transport = MagicMock()
+    transport.connect.side_effect = error
+    with patch("backend.app.services.bambu_ftp.ImplicitFTP_TLS", return_value=transport):
+        client = BambuFTPClient(IP, "12345678", printer_model="P2S")
+        assert client.connect() is False
+
+    assert client.last_failure is not None
+    assert client.last_failure.kind is kind
+    assert client.last_failure.code == code
+    # The underlying text is kept too -- the sentence is for the operator, the
+    # detail is for whoever reads the log next to it.
+    assert str(error)[:20] in client.last_failure.detail
+
+
+def test_the_cooloff_skip_is_its_own_kind():
+    """ "We did not try" is not the same failure as "we tried and it broke"."""
+    BambuFTPClient._handshake_blocked_until[IP] = time.monotonic() + 300
+    client = BambuFTPClient(IP, "12345678")
+
+    assert client.connect() is False
+    assert client.last_failure is not None
+    assert client.last_failure.kind is FtpFailureKind.COOLOFF
+
+
+@pytest.mark.parametrize(
+    ("reply", "kind"),
+    [
+        ("553 Could not create file.", FtpFailureKind.STORAGE),
+        ("552 Storage quota exceeded.", FtpFailureKind.STORAGE),
+        ("550 Permission denied.", FtpFailureKind.NOT_FOUND),
+        ("500 Unknown command.", FtpFailureKind.UNKNOWN),
+    ],
+    ids=["553", "552", "550", "500"],
+)
+def test_upload_classifies_the_printers_reply_code(reply, kind, tmp_path):
+    """553 and 552 are the printer talking about its own storage.
+
+    That is the one case where naming the SD card is worth anything, and it is
+    the case the blanket message was written for before it was applied to
+    every failure alike.
+    """
+    local = tmp_path / "job.3mf"
+    local.write_bytes(b"x" * 16)
+
+    client = BambuFTPClient(IP, "12345678")
+    client._ftp = MagicMock()
+    client._ftp.transfercmd.side_effect = ftplib.error_perm(reply)
+
+    assert client.upload_file(local, "/job.3mf") is False
+    assert client.last_failure is not None
+    assert client.last_failure.kind is kind
+    assert client.last_failure.code == reply[:3]
+
+
+def test_a_successful_upload_leaves_no_failure_behind(tmp_path):
+    """Otherwise a later failure inherits an earlier one's reason."""
+    local = tmp_path / "job.3mf"
+    local.write_bytes(b"x" * 16)
+
+    client = BambuFTPClient(IP, "12345678")
+    client._ftp = MagicMock()
+    client.last_failure = FtpFailure(FtpFailureKind.STORAGE, "553 stale", "553")
+
+    assert client.upload_file(local, "/job.3mf") is True
+    assert client.last_failure is None
+
+
+# ---------------------------------------------------------------------------
+# The reason reaches the caller
+# ---------------------------------------------------------------------------
+class TestTheReportReachesTheCaller:
+    @pytest.fixture()
+    def refusing_printer(self):
+        transport = MagicMock()
+        transport.connect.side_effect = ssl.SSLError("[SSL: WRONG_VERSION_NUMBER] wrong version number")
+        with patch("backend.app.services.bambu_ftp.ImplicitFTP_TLS", return_value=transport):
+            yield transport
+
+    async def test_upload_file_async_fills_the_slot(self, refusing_printer, tmp_path):
+        local = tmp_path / "job.3mf"
+        local.write_bytes(b"x" * 16)
+        report = FtpFailureReport()
+
+        assert await upload_file_async(IP, "12345678", local, "/job.3mf", timeout=5.0, failure=report) is False
+
+        assert report.failure is not None
+        assert report.failure.kind is FtpFailureKind.HANDSHAKE
+
+    async def test_it_survives_the_retry_loop(self, refusing_printer, tmp_path):
+        """with_ftp_retry forwards the slot untouched, so the last try wins.
+
+        The last attempt is the one that decided the outcome, so its reason is
+        the one the operator should be given.
+        """
+        local = tmp_path / "job.3mf"
+        local.write_bytes(b"x" * 16)
+        report = FtpFailureReport()
+
+        result = await with_ftp_retry(
+            upload_file_async,
+            IP,
+            "12345678",
+            local,
+            "/job.3mf",
+            timeout=5.0,
+            respect_handshake_cooloff=False,
+            failure=report,
+            max_retries=2,
+            retry_delay=0.01,
+        )
+
+        assert result is None
+        assert report.failure is not None
+        assert report.failure.kind is FtpFailureKind.HANDSHAKE
+
+    async def test_two_callers_do_not_cross(self, refusing_printer, tmp_path):
+        """The slot belongs to the caller, not to the printer.
+
+        A per-IP dict on the client would be the obvious way to do this, and
+        it is the way that breaks: a background timelapse fetch running beside
+        a dispatch would overwrite the dispatch's reason with its own, and
+        report the wrong cause with total confidence.
+        """
+        local = tmp_path / "job.3mf"
+        local.write_bytes(b"x" * 16)
+        mine, theirs = FtpFailureReport(), FtpFailureReport()
+
+        await upload_file_async(IP, "12345678", local, "/a.3mf", timeout=5.0, failure=mine)
+        assert theirs.failure is None
+        assert mine.failure is not None
+
+    async def test_a_caller_that_does_not_ask_is_unaffected(self, refusing_printer, tmp_path):
+        """Every other caller passes nothing and must keep working."""
+        local = tmp_path / "job.3mf"
+        local.write_bytes(b"x" * 16)
+        assert await upload_file_async(IP, "12345678", local, "/job.3mf", timeout=5.0) is False
+
+
+# ---------------------------------------------------------------------------
+# The wording
+# ---------------------------------------------------------------------------
+class TestTheWording:
+    def test_only_a_storage_reply_sends_anyone_to_the_card(self):
+        """Advice about the card, not mention of it.
+
+        The handshake message names the card too, to rule it out -- that is
+        the opposite of what this is guarding against, so the marker is the
+        instruction ("formatted FAT32 or exFAT"), not the noun.
+        """
+        advising = [k for k in FtpFailureKind if "FAT32" in describe_upload_failure(FtpFailure(k, "detail"))]
+        assert advising == [FtpFailureKind.STORAGE]
+
+    def test_no_other_failure_asks_anyone_to_touch_the_card(self):
+        """Anything that is not a storage reply must not send them there.
+
+        Checked as "do something to the card" rather than "say the words",
+        since ruling the card out is exactly what the handshake message does.
+        """
+        for kind in FtpFailureKind:
+            if kind is FtpFailureKind.STORAGE:
+                continue
+            message = describe_upload_failure(FtpFailure(kind, "detail"))
+            assert "Check that its SD card" not in message, kind
+            assert "inserted" not in message, kind
+
+    @pytest.mark.parametrize(
+        ("kind", "must_say"),
+        [
+            (FtpFailureKind.COOLOFF, "clears on its own"),
+            (FtpFailureKind.HANDSHAKE, "not with TLS"),
+            (FtpFailureKind.AUTH, "access code"),
+            (FtpFailureKind.TIMEOUT, "did not respond in time"),
+            (FtpFailureKind.STORAGE, "FAT32"),
+            (FtpFailureKind.NOT_FOUND, "Bambuddy-side"),
+            (FtpFailureKind.NETWORK, "server log"),
+            (FtpFailureKind.UNKNOWN, "server log"),
+        ],
+    )
+    def test_every_kind_says_something_of_its_own(self, kind, must_say):
+        """One line per branch, so none can quietly collapse into the generic.
+
+        Without this, deleting the access-code branch or the timeout branch
+        leaves every other assertion here passing -- they only check that the
+        card is not named, which the generic message also satisfies.
+        """
+        assert must_say in describe_upload_failure(FtpFailure(kind, "detail", "553"))
+
+    def test_the_access_code_hint_names_a_screen_that_exists(self):
+        """The Access Code field is on the printer form on the Printers page.
+
+        Naming a screen that is not there would be its own version of this
+        bug: confident, specific, and a waste of the reader's time.
+        """
+        message = describe_upload_failure(FtpFailure(FtpFailureKind.AUTH, "530 Login incorrect.", "530"))
+        assert "Printers page" in message
+
+    def test_a_handshake_failure_says_the_card_is_not_involved(self):
+        message = describe_upload_failure(FtpFailure(FtpFailureKind.HANDSHAKE, "WRONG_VERSION_NUMBER"))
+        assert "not with TLS" in message
+        assert "SD card is not involved" in message
+
+    def test_it_does_not_prescribe_a_power_cycle(self):
+        """#2780 removed that advice from the log because it does not work.
+
+        The reporter of this issue restarted a printer on the strength of the
+        user-facing string, so the string has to carry the same restraint.
+        """
+        for kind in FtpFailureKind:
+            message = describe_upload_failure(FtpFailure(kind, "detail"))
+            assert "restart the printer" not in message.lower(), kind
+            assert "reboot" not in message.lower(), kind
+
+    def test_an_unclassified_failure_points_at_the_log_rather_than_guessing(self):
+        for failure in (None, FtpFailure(FtpFailureKind.UNKNOWN, "500 what")):
+            message = describe_upload_failure(failure)
+            assert "server log" in message
+            assert "SD card" not in message
+
+    def test_the_storage_message_carries_the_reply_code(self):
+        """So a support bundle and the queue entry can be lined up."""
+        message = describe_upload_failure(FtpFailure(FtpFailureKind.STORAGE, "553 Could not create file.", "553"))
+        assert "553" in message
+        assert "FAT32" in message
+
+
+# ---------------------------------------------------------------------------
+# End to end: what the queue entry says
+# ---------------------------------------------------------------------------
+@pytest.fixture
+async def dispatch_case(tmp_path):
+    """Minimal one-printer, one-queued-job database for ``_start_print``."""
+    from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine
+
+    import backend.app.models  # noqa: F401 - populate Base.metadata
+    from backend.app.core.database import Base
+    from backend.app.models.archive import PrintArchive
+    from backend.app.models.print_queue import PrintQueueItem
+    from backend.app.models.printer import Printer
+
+    engine = create_async_engine("sqlite+aiosqlite:///:memory:", echo=False)
+    async with engine.begin() as conn:
+        await conn.run_sync(Base.metadata.create_all)
+    session_maker = async_sessionmaker(engine, expire_on_commit=False)
+
+    base_dir = tmp_path / "case"
+    archive_rel = Path("archives") / "job.3mf"
+    archive_abs = base_dir / archive_rel
+    archive_abs.parent.mkdir(parents=True, exist_ok=True)
+    archive_abs.write_bytes(b"archive payload")
+
+    async with session_maker() as db:
+        printer = Printer(
+            name="Bambulab P2S-4",
+            serial_number="SERIAL",
+            ip_address=IP,
+            access_code="12345678",
+            model="P2S",
+        )
+        db.add(printer)
+        await db.flush()
+        archive = PrintArchive(
+            printer_id=printer.id,
+            filename="job.3mf",
+            file_path=str(archive_rel),
+            file_size=archive_abs.stat().st_size,
+            status="completed",
+        )
+        db.add(archive)
+        await db.flush()
+        item = PrintQueueItem(printer_id=printer.id, archive_id=archive.id, status="pending")
+        db.add(item)
+        await db.commit()
+        item_id = item.id
+
+    try:
+        yield SimpleNamespace(session_maker=session_maker, base_dir=base_dir, item_id=item_id)
+    finally:
+        await engine.dispose()
+
+
+async def _dispatch_failing_with(dispatch_case, failure: FtpFailure | None):
+    """Run one dispatch whose upload fails with *failure*.
+
+    Returns the queue item's message and the reason the notification carried,
+    which have to agree -- a push saying something different from the screen is
+    its own small bug.
+    """
+    import backend.app.services.print_scheduler as scheduler_module
+    from backend.app.models.print_queue import PrintQueueItem
+    from backend.app.services.print_scheduler import PrintScheduler
+    from backend.tests._fixtures.background_tasks import discarding_spawn_patch
+
+    async def _upload(*_args, **kwargs):
+        # Stands in for the real wrapper: fills the caller's slot, then fails.
+        # Reading kwargs["failure"] rather than accepting it as a parameter is
+        # deliberate -- if the dispatch ever stops passing the slot, every
+        # message below falls back to the generic one and these tests fail.
+        if failure is not None and kwargs.get("failure") is not None:
+            kwargs["failure"].failure = failure
+        return False
+
+    notify = AsyncMock()
+    scheduler = PrintScheduler()
+    async with dispatch_case.session_maker() as db:
+        item = await db.get(PrintQueueItem, dispatch_case.item_id)
+        patches = [
+            patch.object(scheduler_module.settings, "base_dir", dispatch_case.base_dir),
+            patch("backend.app.services.print_scheduler.printer_manager.is_connected", MagicMock(return_value=True)),
+            patch("backend.app.services.print_scheduler.printer_manager.get_status", MagicMock(return_value=None)),
+            patch(
+                "backend.app.services.print_scheduler.get_ftp_retry_settings",
+                AsyncMock(return_value=(False, 0, 0, 1.0)),
+            ),
+            patch("backend.app.services.print_scheduler.delete_file_async", AsyncMock(return_value=True)),
+            patch("backend.app.services.print_scheduler.upload_file_async", _upload),
+            patch("backend.app.services.print_scheduler.notification_service.on_queue_job_failed", notify),
+            discarding_spawn_patch(),
+            patch.object(scheduler, "_propagate_owner_to_printer_manager", AsyncMock()),
+            patch.object(scheduler, "_power_off_if_needed", AsyncMock()),
+            patch.object(scheduler, "_preheat_and_soak", AsyncMock()),
+        ]
+        with ExitStack() as stack:
+            for p in patches:
+                stack.enter_context(p)
+            await scheduler._start_print(db, item)
+
+        refreshed = await db.get(PrintQueueItem, dispatch_case.item_id)
+        assert refreshed.status == "failed"
+        return refreshed.error_message or "", notify.await_args.kwargs["reason"]
+
+
+class TestWhatTheQueueEntrySays:
+    async def test_a_handshake_failure_does_not_send_anyone_to_the_sd_card(self, dispatch_case):
+        """The report's own case: a TLS failure, answered with card advice.
+
+        The reporter acted on it and restarted the printer. Nothing in that
+        path reaches the printer's filesystem, and the cool-off that made the
+        next dispatch fail identically lives in Bambuddy's memory, where
+        power-cycling a printer does not reach.
+        """
+        message, reason = await _dispatch_failing_with(
+            dispatch_case, FtpFailure(FtpFailureKind.HANDSHAKE, "WRONG_VERSION_NUMBER")
+        )
+
+        assert "inserted" not in message, message
+        assert "FAT32" not in message, message
+        assert "not with TLS" in message, message
+        assert reason == message
+
+    async def test_a_553_still_gets_the_card_advice(self, dispatch_case):
+        """The advice was written for this case and belongs to it.
+
+        Removing it everywhere would trade one wrong message for a vaguer one;
+        the point is to attach it where the printer actually said storage.
+        """
+        message, reason = await _dispatch_failing_with(
+            dispatch_case, FtpFailure(FtpFailureKind.STORAGE, "553 Could not create file.", "553")
+        )
+
+        assert "FAT32" in message, message
+        assert "553" in message, message
+        assert reason == message
+
+    async def test_an_unclassified_failure_points_at_the_log(self, dispatch_case):
+        """No reason recorded means no reason invented."""
+        message, reason = await _dispatch_failing_with(dispatch_case, None)
+
+        assert "server log" in message, message
+        assert "SD card" not in message, message
+        assert reason == message