mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-10-19 15:54:27 +02:00
Keep a dispatch's retries out of the FTPS cool-off (issue #2898)
A failed TLS handshake arms a 300s per-IP cool-off, and connect()
consulted it for every caller. A print dispatch retries after 2s, so
once the cool-off was armed all four attempts were answered from the
gate rather than the network, and every further job queued for that
printer failed the same way for the rest of the window. The reporter's
farm lost three jobs to one handshake error, with the retry budget
contributing nothing to any of them.
The gate was serving two callers that want opposite things from it. The
background sweeps -- the post-print 3MF, cover and timelapse fetches --
walk ~110 candidate paths against one wedged printer with nobody
waiting, and backing off for minutes is right for them. A dispatch is
one delete plus at most four upload attempts with someone watching a
progress bar. So the split is by caller: a client built with
respect_handshake_cooloff=False goes to the printer regardless, and the
dispatch's delete and upload -- and a firmware upload, same shape --
opt out. Everything else keeps #2780's behaviour untouched.
In the reported trace it is the pre-upload delete that takes the SSL
error and arms the cool-off, 8ms before the upload's first attempt, so
exempting the upload alone would have left one dispatch's worth of the
problem in place.
Callers that do respect the cool-off no longer sleep out a retry loop
against it: with_ftp_retry takes the printer's IP and stops at the
attempt that armed the gate, instead of spending three more attempts
and six seconds on connections that cannot happen. It also reports the
attempts it really made -- "failed after 4 attempts" for one attempt is
part of how this read as a network problem.
Two diagnosis fixes go with it. The cool-off skip was the one connect()
failure path that reported without naming its cause, and at DEBUG, so
four identical reason-free warnings were all the operator saw. It now
says at WARNING that nothing was sent and how long the printer has
left, once per cool-off rather than once per attempt -- not every
caller is gated, and a download-zip of 200 files would otherwise repeat
the sentence 200 times, which is the flood #2780 set out to stop.
And a dispatch that fails this way no longer tells anyone to check
whether the SD card is inserted and formatted -- nothing reached the
printer's filesystem, so the card is the one part of the machine that
was working. The message names the file service and rules the card out.
It is used only when a handshake failed during the dispatch itself,
read from the cool-off deadline MOVING rather than merely being armed:
the dispatch ignores the gate, so it can be running underneath one an
unrelated background fetch left behind, and blaming TLS for an upload
that really hit a full disk would repeat the mistake in the other
direction.
Tests count sockets rather than return values, since "returned False"
looks identical whether or not anything was attempted -- which is what
made the original report a log dive. Reverting any one of the five
behaviours above fails a distinct test.
This commit is contained in:
@@ -2569,6 +2569,7 @@ async def scan_timelapse(
|
||||
max_retries=ftp_retry_count,
|
||||
retry_delay=ftp_retry_delay,
|
||||
operation_name=f"Download timelapse {matching_file['name']}",
|
||||
cooloff_ip=printer.ip_address,
|
||||
)
|
||||
else:
|
||||
timelapse_data = await download_file_bytes_async(
|
||||
@@ -2691,6 +2692,7 @@ async def select_timelapse(
|
||||
max_retries=ftp_retry_count,
|
||||
retry_delay=ftp_retry_delay,
|
||||
operation_name=f"Download timelapse {filename}",
|
||||
cooloff_ip=printer.ip_address,
|
||||
)
|
||||
else:
|
||||
timelapse_data = await download_file_bytes_async(
|
||||
|
||||
@@ -3717,6 +3717,7 @@ async def on_print_start(printer_id: int, data: dict):
|
||||
max_retries=ftp_retry_count,
|
||||
retry_delay=ftp_retry_delay,
|
||||
operation_name=f"Download 3MF from {remote_path}",
|
||||
cooloff_ip=printer.ip_address,
|
||||
non_retry_exceptions=(FileNotOnPrinterError,),
|
||||
)
|
||||
else:
|
||||
@@ -3796,6 +3797,7 @@ async def on_print_start(printer_id: int, data: dict):
|
||||
max_retries=ftp_retry_count,
|
||||
retry_delay=ftp_retry_delay,
|
||||
operation_name=f"Download 3MF from {remote_full_path}",
|
||||
cooloff_ip=printer.ip_address,
|
||||
)
|
||||
else:
|
||||
downloaded = await download_file_async(
|
||||
@@ -3860,6 +3862,7 @@ async def on_print_start(printer_id: int, data: dict):
|
||||
max_retries=ftp_retry_count,
|
||||
retry_delay=ftp_retry_delay,
|
||||
operation_name=f"Re-download 3MF from {remote_path}",
|
||||
cooloff_ip=printer.ip_address,
|
||||
non_retry_exceptions=(FileNotOnPrinterError,),
|
||||
)
|
||||
else:
|
||||
|
||||
@@ -225,6 +225,10 @@ class BambuFTPClient:
|
||||
# their cool-off expires. See ``_HANDSHAKE_COOLOFF_SECONDS``.
|
||||
_handshake_blocked_until: dict[str, float] = {}
|
||||
|
||||
# Which cool-off deadline each printer's "not attempted" warning was last
|
||||
# logged for, so the warning is said once per cool-off. See ``connect``.
|
||||
_handshake_skip_logged: dict[str, float] = {}
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
ip_address: str,
|
||||
@@ -232,12 +236,26 @@ class BambuFTPClient:
|
||||
timeout: float | None = None,
|
||||
printer_model: str | None = None,
|
||||
force_prot_c: bool = False,
|
||||
respect_handshake_cooloff: bool = True,
|
||||
):
|
||||
"""Set ``respect_handshake_cooloff=False`` for bounded, user-initiated work.
|
||||
|
||||
The cool-off exists to stop an unbounded sweep re-running one doomed
|
||||
handshake a hundred times over (#2780). Dispatching a print is not
|
||||
that: it is one delete plus at most four upload attempts, with someone
|
||||
waiting on the result. Sharing the sweep's gate cost those attempts
|
||||
their whole retry budget, and failed every further job queued for that
|
||||
printer for the rest of the 300s window (#2898).
|
||||
|
||||
Leave it at the default everywhere else. Opting out is only defensible
|
||||
because the caller's own connection count is bounded and small.
|
||||
"""
|
||||
self.ip_address = ip_address
|
||||
self.access_code = access_code
|
||||
self.timeout = timeout if timeout is not None else self.DEFAULT_TIMEOUT
|
||||
self.printer_model = printer_model
|
||||
self.force_prot_c = force_prot_c
|
||||
self.respect_handshake_cooloff = respect_handshake_cooloff
|
||||
self._ftp: ImplicitFTP_TLS | None = None
|
||||
|
||||
def _is_a1_model(self) -> bool:
|
||||
@@ -283,6 +301,7 @@ class BambuFTPClient:
|
||||
# Drop it on the way past rather than leaving an entry per printer
|
||||
# this process has ever failed against.
|
||||
del cls._handshake_blocked_until[ip_address]
|
||||
cls._handshake_skip_logged.pop(ip_address, None)
|
||||
return False
|
||||
return True
|
||||
|
||||
@@ -290,13 +309,35 @@ class BambuFTPClient:
|
||||
"""Connect to the printer FTP server (implicit FTPS on port 990).
|
||||
|
||||
Returns False without touching the network while the printer is inside
|
||||
the cool-off a previous TLS handshake failure opened (#2780).
|
||||
the cool-off a previous TLS handshake failure opened (#2780) -- unless
|
||||
this client was built with ``respect_handshake_cooloff=False``.
|
||||
"""
|
||||
if self.handshake_blocked(self.ip_address):
|
||||
logger.debug(
|
||||
"FTP connect to %s skipped: FTPS handshake failed recently, cooling off",
|
||||
self.ip_address,
|
||||
)
|
||||
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
|
||||
# reason-free "FTP connection failed" lines two seconds apart gave
|
||||
# no hint that nothing had been sent (#2898). Every caller reaching
|
||||
# here is already gated by handshake_blocked() at its own sweep
|
||||
# boundary, so this costs about one line per print, not a flood.
|
||||
deadline = self._handshake_blocked_until.get(self.ip_address)
|
||||
remaining = max(0.0, deadline - time.monotonic()) if deadline is not None else 0.0
|
||||
if deadline is not None and self._handshake_skip_logged.get(self.ip_address) != deadline:
|
||||
self._handshake_skip_logged[self.ip_address] = deadline
|
||||
logger.warning(
|
||||
"FTP connect to %s not attempted: its FTPS handshake failed recently and it is "
|
||||
"cooling off for another %.0fs. Nothing was sent to the printer.",
|
||||
self.ip_address,
|
||||
remaining,
|
||||
)
|
||||
else:
|
||||
# Said once already for this cool-off. Repeating it per candidate
|
||||
# path is the log flood #2780 set out to stop -- a download-zip
|
||||
# of 200 files would print the same sentence 200 times.
|
||||
logger.debug(
|
||||
"FTP connect to %s skipped: still cooling off for another %.0fs",
|
||||
self.ip_address,
|
||||
remaining,
|
||||
)
|
||||
return False
|
||||
try:
|
||||
use_prot_c = self._should_use_prot_c()
|
||||
@@ -928,6 +969,18 @@ 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.
|
||||
|
||||
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).
|
||||
"""
|
||||
return BambuFTPClient._handshake_blocked_until.get(ip_address)
|
||||
|
||||
|
||||
def ftps_handshake_blocked(ip_address: str) -> bool:
|
||||
"""True while this printer's FTPS handshake cool-off is still running.
|
||||
|
||||
@@ -1249,6 +1302,7 @@ async def upload_file_async(
|
||||
progress_callback: Callable[[int, int], None] | None = None,
|
||||
socket_timeout: float | None = None,
|
||||
printer_model: str | None = None,
|
||||
respect_handshake_cooloff: bool = True,
|
||||
) -> bool:
|
||||
"""Async wrapper for uploading a file with timeout and progress callback.
|
||||
|
||||
@@ -1266,6 +1320,9 @@ async def upload_file_async(
|
||||
progress_callback: Optional callback for progress updates
|
||||
socket_timeout: FTP socket timeout for slow connections (e.g., A1 printers)
|
||||
printer_model: Printer model for A1-specific workarounds
|
||||
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).
|
||||
"""
|
||||
loop = asyncio.get_event_loop()
|
||||
is_a1 = printer_model in BambuFTPClient.A1_MODELS if printer_model else False
|
||||
@@ -1287,7 +1344,12 @@ async def upload_file_async(
|
||||
f"mode={mode_str}, socket_timeout={socket_timeout}s, deadline={deadline:.0f}s)..."
|
||||
)
|
||||
client = BambuFTPClient(
|
||||
ip_address, access_code, timeout=socket_timeout, printer_model=printer_model, force_prot_c=force_prot_c
|
||||
ip_address,
|
||||
access_code,
|
||||
timeout=socket_timeout,
|
||||
printer_model=printer_model,
|
||||
force_prot_c=force_prot_c,
|
||||
respect_handshake_cooloff=respect_handshake_cooloff,
|
||||
)
|
||||
if client.connect():
|
||||
logger.info("FTP connected to %s", ip_address)
|
||||
@@ -1465,6 +1527,7 @@ async def delete_file_async(
|
||||
socket_timeout: float | None = None,
|
||||
printer_model: str | None = None,
|
||||
timeout: float = 60.0,
|
||||
respect_handshake_cooloff: bool = True,
|
||||
) -> DeleteResult:
|
||||
"""Async wrapper for deleting a file.
|
||||
|
||||
@@ -1477,11 +1540,21 @@ async def delete_file_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).
|
||||
respect_handshake_cooloff: see ``BambuFTPClient.__init__``. The delete
|
||||
that clears the way for a dispatch shares the upload's exemption --
|
||||
it is one connection, and in #2898's trace it is the one that armed
|
||||
the cool-off the upload then spent all four attempts against.
|
||||
"""
|
||||
loop = asyncio.get_event_loop()
|
||||
|
||||
def _delete() -> DeleteResult:
|
||||
client = BambuFTPClient(ip_address, access_code, timeout=socket_timeout, printer_model=printer_model)
|
||||
client = BambuFTPClient(
|
||||
ip_address,
|
||||
access_code,
|
||||
timeout=socket_timeout,
|
||||
printer_model=printer_model,
|
||||
respect_handshake_cooloff=respect_handshake_cooloff,
|
||||
)
|
||||
if client.connect():
|
||||
try:
|
||||
return client.delete_file(remote_path)
|
||||
@@ -1710,6 +1783,7 @@ async def with_ftp_retry(
|
||||
retry_delay: float = 2.0,
|
||||
operation_name: str = "FTP operation",
|
||||
non_retry_exceptions: tuple[type[BaseException], ...] = (),
|
||||
cooloff_ip: str | None = None,
|
||||
**kwargs,
|
||||
) -> T | None:
|
||||
"""Execute FTP operation with retry logic.
|
||||
@@ -1721,6 +1795,10 @@ async def with_ftp_retry(
|
||||
retry_delay: Seconds to wait between retries (default: 2.0)
|
||||
operation_name: Name for logging purposes
|
||||
non_retry_exceptions: Exception types that should immediately abort retries
|
||||
cooloff_ip: printer IP whose FTPS handshake cool-off should end the loop
|
||||
early. Pass it from any caller that respects the cool-off; leave it
|
||||
unset for one that opted out, or the loop would stop on a gate its
|
||||
own attempts are ignoring (#2898).
|
||||
**kwargs: Keyword arguments for the operation
|
||||
|
||||
Returns:
|
||||
@@ -1731,8 +1809,10 @@ async def with_ftp_retry(
|
||||
another full deadline reaching the same conclusion (#2529).
|
||||
"""
|
||||
last_error = None
|
||||
attempts_made = 0
|
||||
|
||||
for attempt in range(max_retries + 1):
|
||||
attempts_made = attempt + 1
|
||||
try:
|
||||
result = await operation(*args, **kwargs)
|
||||
# Check for "falsy" success indicators
|
||||
@@ -1753,10 +1833,28 @@ async def with_ftp_retry(
|
||||
|
||||
# Don't wait after the last attempt
|
||||
if attempt < max_retries:
|
||||
# A cool-off outlasts this loop by two orders of magnitude, so once
|
||||
# it is armed every remaining attempt returns False without opening
|
||||
# a socket. Spending them anyway bought nothing and cost the caller
|
||||
# `max_retries * retry_delay` seconds of sleeping, then reported the
|
||||
# failure with the wrong reason (#2898).
|
||||
if cooloff_ip and ftps_handshake_blocked(cooloff_ip):
|
||||
logger.warning(
|
||||
"%s: stopping after attempt %s/%s — %s is inside its FTPS handshake cool-off, "
|
||||
"so the remaining attempts would not reach it",
|
||||
operation_name,
|
||||
attempt + 1,
|
||||
max_retries + 1,
|
||||
cooloff_ip,
|
||||
)
|
||||
break
|
||||
logger.info("%s will retry in %ss...", operation_name, retry_delay)
|
||||
await asyncio.sleep(retry_delay)
|
||||
|
||||
logger.error("%s failed after %s attempts", operation_name, max_retries + 1)
|
||||
# attempts_made, not max_retries + 1: the loop can stop early on a cool-off,
|
||||
# and reporting attempts that were never made is how #2898 read as a network
|
||||
# problem when nothing had gone near the network.
|
||||
logger.error("%s failed after %s attempts", operation_name, attempts_made)
|
||||
if last_error:
|
||||
logger.debug("Last error: %s", last_error)
|
||||
return None
|
||||
|
||||
@@ -344,6 +344,11 @@ class FirmwareUpdateService:
|
||||
progress_callback=on_upload_progress,
|
||||
socket_timeout=ftp_timeout,
|
||||
printer_model=model,
|
||||
# Someone pressed "update firmware" and is watching a
|
||||
# progress bar. Bounded and user-initiated, like a print
|
||||
# dispatch, so it does not spend its retries on a cool-off
|
||||
# meant for the background sweeps (#2898).
|
||||
respect_handshake_cooloff=False,
|
||||
max_retries=ftp_retry_count,
|
||||
retry_delay=ftp_retry_delay,
|
||||
operation_name=f"Upload firmware to printer {printer_id}",
|
||||
@@ -357,6 +362,7 @@ class FirmwareUpdateService:
|
||||
progress_callback=on_upload_progress,
|
||||
socket_timeout=ftp_timeout,
|
||||
printer_model=model,
|
||||
respect_handshake_cooloff=False,
|
||||
)
|
||||
|
||||
if not success:
|
||||
|
||||
@@ -33,6 +33,7 @@ from backend.app.services.bambu_ftp import (
|
||||
UploadCancelled,
|
||||
cache_3mf_download,
|
||||
delete_file_async,
|
||||
ftps_handshake_cooloff_deadline,
|
||||
get_ftp_retry_settings,
|
||||
upload_file_async,
|
||||
with_ftp_retry,
|
||||
@@ -6093,6 +6094,14 @@ 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)
|
||||
@@ -6102,6 +6111,13 @@ class PrintScheduler:
|
||||
remote_path,
|
||||
socket_timeout=ftp_timeout,
|
||||
printer_model=printer.model,
|
||||
# This delete and the upload below are one bounded, user-initiated
|
||||
# unit -- at most nine connections -- so neither skips on the
|
||||
# handshake cool-off the opportunistic sweeps rely on. In #2898's
|
||||
# trace this delete took the TLS failure and armed the cool-off,
|
||||
# and the upload's four attempts were then spent against it
|
||||
# without a socket being opened.
|
||||
respect_handshake_cooloff=False,
|
||||
)
|
||||
logger.debug("Queue item %s: Delete result: %s", item.id, delete_result)
|
||||
except Exception as e:
|
||||
@@ -6144,6 +6160,7 @@ class PrintScheduler:
|
||||
socket_timeout=ftp_timeout,
|
||||
printer_model=printer.model,
|
||||
progress_callback=progress_bridge,
|
||||
respect_handshake_cooloff=False,
|
||||
max_retries=ftp_retry_count,
|
||||
retry_delay=ftp_retry_delay,
|
||||
operation_name=f"Upload print to {printer.name}",
|
||||
@@ -6157,6 +6174,7 @@ class PrintScheduler:
|
||||
socket_timeout=ftp_timeout,
|
||||
printer_model=printer.model,
|
||||
progress_callback=progress_bridge,
|
||||
respect_handshake_cooloff=False,
|
||||
)
|
||||
except UploadCancelled as e:
|
||||
uploaded = False
|
||||
@@ -6174,6 +6192,20 @@ 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."
|
||||
|
||||
Reference in New Issue
Block a user