From 527f8ea471a77f17b6bb80cb4b7f93f4ed8e69cb Mon Sep 17 00:00:00 2001 From: maziggy Date: Sun, 26 Apr 2026 15:00:20 +0200 Subject: [PATCH] fix(mqtt): #1136 reprint fails with 0500_4003 SD R/W after stuck dispatch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reprinting from archives sometimes failed immediately with a MicroSD R/W exception, with the printer's MQTT push referencing a 3MF from a different unrelated archive. Once it started, every subsequent reprint hit the same error until the container was restarted. Root cause from @smandon's support package: paho-mqtt's client-side QoS 1 queue. When the printer's command channel goes half-broken (telemetry flowing, publishes silently dropped — same #887/#936 pattern), background_dispatch.py:993 hits its 15s deadline and calls force_reconnect_stale_session(). That function was force-closing the underlying socket so paho's auto-reconnect would kick in, but the same mqtt.Client instance, same client_id, and same in-process QoS 1 queue stayed alive across the reconnect. Any unacked publish from the broken session — typically the just-sent project_file for the new archive — got replayed verbatim on the new connection. The queue accumulates across multiple stuck dispatches in one Python process, so by the second or third stuck reprint there were several stale project_file/resume/stop/clean_print_error commands queued together; the printer latched onto whichever stale path it processed last, couldn't find the file on its SD card, and emitted 0500_4003. Container restart was the only thing that wiped paho's in-process queue. Replaced socket-close with a context-aware reconnect via a new _reset_client_for_reconnect() router: Async-context callers (dispatch deadline, FastAPI handlers via check_staleness) → hard-reset: client.disconnect() (broker drops session, clean_session=True), client.loop_stop() (kills paho's network thread and its queue), null _client, fresh connect() with incremented client_id. New connection is genuinely empty, no replay. Paho-network-thread callers (dev-mode probe + ams_filament_setting zombie detection inside _update_state) → socket-close fallback. loop_stop() from inside the network thread would self-join and deadlock, so the safe pattern there is "close the socket and let paho's loop detect it and auto-reconnect on the same client". Routing decision uses asyncio.get_running_loop() — paho's callback thread has no loop, every legitimate hard-reset caller does. 7 regression tests: - TestForceReconnectRouting (3): sync-context → socket-close fallback, async-context → hard-reset with disconnect()+loop_stop()+null, state-disconnected broadcast fires once on either path - TestHardResetClientDirect (3): helper directly — old client gets disconnect()+loop_stop(), _client cleared, failing disconnect() doesn't propagate so background_dispatch's await chain can't break - TestZombieSessionDetection / TestDeveloperModeProbeTimeout (updated): paho-thread context still goes through socket-close, preserving the legacy contract for those paths --- CHANGELOG.md | 2 + backend/app/services/bambu_mqtt.py | 97 ++++++++++--- .../tests/unit/services/test_bambu_mqtt.py | 128 +++++++++++++++++- 3 files changed, 207 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f2950e2c5..4dbca7116 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,8 @@ All notable changes to Bambuddy will be documented in this file. - **Per-request trace ID column on every log line, plumbed through HTTP access log + application logs + response headers** — Builds on the new uvicorn-access-log-into-bambuddy.log change below: the access line tells you *who* called an endpoint, but until now there was no way to tie that line to the application records emitted on the server side while handling that request. A new FastAPI middleware (`trace_id_middleware` in `main.py`, sourced from `backend.app.core.trace`) stamps each request with a fresh 8-char hex ID (or honours a sane inbound `X-Trace-Id` header for cross-system correlation), stores it in a `ContextVar` so any code in the request's call stack can read it, echoes it on the response as `X-Trace-Id`, and a new `TraceIDFilter` injects it into every `LogRecord` so the format string `[%(trace_id)s]` resolves to the right ID for the right request. ContextVars (rather than `request.state`) are the right plumbing here because asyncio copies the current context into every `asyncio.create_task`, so background work spawned from inside a request inherits the trace ID without explicit threading; the logging filter has no access to the FastAPI request object regardless. Records emitted outside any request scope (startup, MQTT callbacks, scheduler) get a stable `-` placeholder so the column stays visually aligned and missing values are obvious in `grep`. Inbound `X-Trace-Id` is hard-validated against a strict whitelist (`[A-Za-z0-9_-]+`, max 64 chars) before being honoured — a hostile or buggy caller cannot smuggle log-injection payloads (newlines, control chars, megabyte blobs) into `bambuddy.log` via the trace-ID column; values that fail the gate silently trigger a freshly minted server-side ID rather than failing the request. Middleware is decorated AFTER `auth_middleware` on purpose: Starlette stacks `@app.middleware` decorators LIFO so the last-decorated runs first inbound, making trace stamp the OUTERMOST layer — auth log lines and every record emitted on the way down to and back from the route handler all carry the same ID. Output now looks like `2026-04-26 09:51:39,152 INFO [uvicorn.access] [a4f3b1e7] 192.168.1.42:54812 - "POST /api/v1/printers/1/print/stop HTTP/1.1" 200` paired with the route handler's `2026-04-26 09:51:39,158 INFO [bambu_mqtt] [a4f3b1e7] [SERIAL] Sent stop print command` — one `grep a4f3b1e7` away from the full causality chain. 30 new tests across `tests/unit/test_trace.py` (placeholder when no request scope, filter copies ContextVar value onto records, ID propagates into spawned tasks via asyncio context copy, concurrent requests don't leak IDs into each other, generator produces unique hex IDs, hostile payloads rejected by validator, max-length boundary, dash/underscore variants accepted) plus `tests/integration/test_trace_middleware.py` (X-Trace-Id header echoed on response, body and header IDs match, each request gets a unique ID, generator format stays short hex, safe inbound IDs honoured, hostile inbound IDs replaced, overlong inbound IDs replaced, ContextVar reset cleanly after request). ### Fixed +- **Reprint-from-archive failed with `0500_4003` SD R/W errors after a stuck dispatch, fixable only by restarting the container** ([#1136](https://github.com/maziggy/bambuddy/issues/1136)) — Reported by @smandon: reprinting from archives sometimes fails immediately with MicroSD R/W exception errors, with the printer's MQTT push referencing a 3MF file from a *different unrelated* archive (`WARIO_Wall_decor_-_NO_AMS.3mf` while the user was actually trying to print `Cable_Organiser_Cable_Clip.3mf`). Once it starts happening, every subsequent reprint hits the same error until the container is restarted. Root cause traced from his support package log to paho-mqtt's client-side QoS 1 queue: when the printer's command channel goes half-broken (telemetry still flowing, publishes silently dropped — same #887/#936 pattern), Bambuddy's 15s dispatch deadline expires (`background_dispatch.py:993`) and calls `force_reconnect_stale_session()`. That function was force-closing the underlying socket so paho's auto-reconnect would kick in — but the same `mqtt.Client` instance, same `client_id`, and **same in-process QoS 1 queue** stayed alive across the reconnect. Any unacked publish from the broken session — typically the just-sent `project_file` for the new archive — got replayed verbatim on the new connection. And because the in-process queue accumulates across multiple stuck dispatches within one Python process, by the second or third stuck reprint there were several stale `project_file`/`resume`/`stop`/`clean_print_error` commands queued up and replaying together. The printer received the flood, tried to load whichever stale path the firmware latched onto last, found a file that no longer existed on its SD card → `0500_4003`. Container restart was the only thing that fixed it because it was the only thing that wiped paho's in-process queue. Replaced the socket-close with a context-aware reconnect: `force_reconnect_stale_session()` and `check_staleness()` now go through a routing helper `_reset_client_for_reconnect()` that picks the right teardown strategy based on caller context. **Async-context callers** (the dispatch deadline path — `background_dispatch.py:993` — which is the actual #1136 trigger, plus FastAPI route handlers via `check_staleness`) get the **hard-reset path**: `client.disconnect()` (broker sees DISCONNECT and drops the session immediately, since `clean_session=True`), `client.loop_stop()` (kills the paho network thread, taking its QoS 1 queue with it), nulls out `self._client`, and calls `self.connect()` to construct a fresh `mqtt.Client` with an incremented `client_id`. New connection starts genuinely empty, no replay possible. **Paho-network-thread callers** (the developer-mode probe and `ams_filament_setting` zombie detection inside `_update_state`, lines ~2604 and ~2623) keep the **socket-close fallback** — calling `loop_stop()` from inside the network thread would self-join and deadlock, so the safe pattern there remains "close the socket and let paho's own loop detect it and auto-reconnect on the same client". Theoretical queue replay is still possible on those paths but #1136 specifically traced through the dispatch path, and the legacy socket-close has been battle-tested for the zombie paths since #887. Routing decision is made via `asyncio.get_running_loop()` — paho's callback thread has no loop, every legitimate hard-reset caller does. 7 regression tests across two new test classes: `TestForceReconnectRouting` (3 tests pinning the sync-context → socket-close fallback, async-context → hard-reset path with mock-stubbed `connect()`, and the state-disconnected broadcast firing once on either path) and `TestHardResetClientDirect` (3 tests pinning the helper directly: old client receives `disconnect()` + `loop_stop()`, `_client` reference cleared, failing `disconnect()` doesn't propagate so the await chain in `background_dispatch.py` doesn't break). Existing `TestZombieSessionDetection::test_two_timeouts_force_reconnect` and `TestDeveloperModeProbeTimeout::test_second_timeout_forces_reconnect` updated to assert the socket-close path (matching their paho-thread context), preserving the legacy contract. All 2179 backend unit tests pass. Thanks to @smandon for the precise reproduction logs that made this diagnosable from a single support package. + - **`logs/bambuddy.log` was silently dropping records from named child loggers** — When the trace-ID column was added to the log format (`%(trace_id)s`), the `TraceIDFilter` was attached to the root logger. Per Python's logging semantics, a filter on a `Logger` only fires for records that *originate* at that logger — records propagated up from child loggers (every `backend.app.*` module — most of the application) never trigger it. Result: child-logger records arrived at the file handler with no `trace_id` attribute, the formatter raised `KeyError: 'trace_id'`, and `Handler.handleError` printed to stderr and dropped the record. `bambuddy.log` ended up with INFO/DEBUG records appearing only "partially" — exactly the records emitted directly through `logging.info(...)` (root logger) or `uvicorn.access` (which had its own explicit filter attachment) made it; everything else was discarded. Moved `_trace_id_filter` from `root_logger.addFilter()` to `console_handler.addFilter()` + `file_handler.addFilter()` — handler-level filters fire for every record the handler receives, regardless of which logger emitted it. The filter's own docstring already said "Attach to the file handler (or any handler whose format string references `%(trace_id)s`)" — the implementation was just wrong. New regression test in `test_trace.py::TestFilterMustBeAttachedToHandlerNotLogger` pins the contract: a child logger emits a record, propagation reaches the handler-level filter, the formatter sees a populated `trace_id` field, and the line is written. Existing 23 trace tests keep passing unchanged. Restart-shutdown recursion in journalctl was also a side effect — every shutdown log line was raising the formatter `ValueError`, which got caught and logged… raising again, forever, until the lifespan exit unwound; the new placement breaks the cycle since records now format cleanly. - **User-cancelled prints surfaced as "1 problem" on the printer card AND were archived as "Layer shift" failures** — Cancelling a print left the printer card stuck on a permanent "1 problem" badge, *and* stamped the resulting archive entry with `failure_reason="Layer shift"` — a fake firmware-fault label in the print history. Affects every Bambu printer that emits a cancel-sequence HMS — the user surfaced it on an H2D where the firmware emits both `0300_400C` ("The task was canceled.") and the not-in-the-public-wiki `0C00_001B` echo as part of the cancel sequence. Four compounding causes, all fixed together. **(1) The direct stop endpoint never set the user-stopped flag.** `POST /printers/{id}/print/stop` (`backend/app/api/routes/printers.py`) sent the MQTT stop command but didn't call `mark_printer_stopped_by_user()`, so when the printer reported "failed" via MQTT the on_print_complete override (`main.py:2558`) couldn't reclassify it as "cancelled". The same flag *was* being set from `POST /print-queue/{id}/stop`, which is why queue-driven cancels mostly worked but printer-card cancels didn't. The direct endpoint now mirrors the queue path. **(2) The HMS → failure_reason heuristic was way too broad.** Old code mapped *any module 0x0C HMS* to "Layer shift" (`main.py:3072`), but module 0x0C is "Motion Controller" — covers cameras, visual markers, the BirdsEye assembly *and* the cancel-sequence HMS the firmware emits during a user-cancel. Real layer-shift codes actually live in module 0x03 (`0300_4057`, `0300_4068`, `0300_800C`). The same module-only heuristic was also being used to auto-label "Filament runout" (any 0x07) and "Clogged nozzle" (any 0x05), so the same false-positive class existed on those branches. Replaced the broad module heuristic with a curated short-code → reason map (`_HMS_FAILURE_REASONS`, 23 specific HMS codes from the real wiki); anything not in that map leaves `failure_reason=None` rather than guessing. Also extracted the logic into a pure function `derive_failure_reason(status, hms_errors)` so it's unit-testable without the full archive pipeline. **(3) Cancel-echo HMS codes were polluting `state.hms_errors`.** Even with (1) and (2) fixed, the printer card kept showing "1 problem" because the firmware kept reporting `0300_400C` ("The task was canceled.") in subsequent MQTT pushes — and `bambu_mqtt._update_state` was happily appending it to `state.hms_errors`, where the frontend's `filterKnownHMSErrors` accepted it as a valid known code (it IS in `ERROR_DESCRIPTIONS` — just describing a user action, not a fault). Added a parse-time filter (`_HMS_USER_ACTION_CODES = {"0300_400C", "0500_400E"}`) that drops these short codes before they ever enter the state, mirroring the suppression `main.py:_HMS_NOTIFICATION_SUPPRESS` was already doing for notifications. The card pip, the "X problem" badge, the modal, and any other consumer of `hms_errors` all get consistent behavior automatically. **(4) Frontend counted `gcode_state="FAILED"` without HMS as a problem.** Even with (1)–(3) fixed, the printer card *still* showed "1 problem" because the H2D's `gcode_state` sits at `FAILED` after a cancel until the next print starts, and `PrintersPage.tsx:940` (header badge) + `classifyPrinterStatus` (line 1028) + `BulkPrinterToolbar.tsx:102` all unconditionally bumped the `error` bucket on `case 'FAILED'`. Real failures attach an HMS error; user-cancels don't — so FAILED-without-HMS now buckets as `finished` (same operator meaning: print ended, plate may need clearing) and only escalates to `error` when there's an active known HMS. Same change applied across all three call sites for consistency. 20 regression tests total across three files: `test_failure_reason_derivation.py` (11 tests pinning the cancel-sequence HMS pair to NOT yield "Layer shift", unknown module-0x0C → None, real layer-shift/runout/clog codes still classify, int-vs-hex code-format tolerance, `status="cancelled"` symmetric with `"aborted"`), `test_bambu_mqtt.py::TestHMSUserActionFiltering` (4 tests pinning `0300_400C`/`0500_400E` filtering on both `hms[]` and `print_error` parse paths, real layer-shift `0300_4057` still passes through, mid-cancel concurrent real-fault keeps the real one and drops only the echo), and `PrintersPageBucketing.test.ts` (5 tests pinning FAILED-without-HMS → finished, FAILED-with-known-HMS → error, FAILED-with-only-unknown-HMS → finished, FINISH baseline unchanged, disconnected stays offline). Existing stale state on running printers clears on the next MQTT push that includes an `hms` key (printer firmware re-sends the list, parser filters it out, badge clears). Users with a stuck badge can also click the HMS modal "Clear" button to clear immediately via MQTT command. diff --git a/backend/app/services/bambu_mqtt.py b/backend/app/services/bambu_mqtt.py index 536fa1821..c5c08c567 100644 --- a/backend/app/services/bambu_mqtt.py +++ b/backend/app/services/bambu_mqtt.py @@ -425,31 +425,96 @@ class BambuMQTTClient: self.state.connected = False if self.on_state_change: self.on_state_change(self.state) - # Force-close the underlying socket so paho's loop thread detects - # the broken connection and triggers auto-reconnect. We don't call - # client.disconnect() because that's a clean disconnect and paho - # would NOT auto-reconnect afterwards. - # Set flag so _on_disconnect knows this was intentional and skips - # redundant state broadcast (we already set connected=False above). + # Route based on caller thread — see force_reconnect_stale_session. + # check_staleness is normally called from FastAPI handlers (async, + # gets the hard-reset path) but the dispatcher exists for safety. self._stale_reconnecting = True - if self._client: - try: - sock = self._client.socket() - if sock: - sock.close() - except Exception: - pass # Best-effort; paho loop will reconnect on next iteration + self._reset_client_for_reconnect() return self.state.connected def force_reconnect_stale_session(self, reason: str) -> None: - # Heals the #887 half-broken session: telemetry keeps arriving but our - # publishes no longer reach the printer. Closing the socket makes paho - # drop and re-establish with a fresh session. + # Heals the #887/#936/#1136 half-broken session: telemetry keeps + # arriving but our publishes don't reach the printer. + # + # Two routing paths: + # + # Async-context callers (background_dispatch.py:993 — dispatch deadline) + # → full client teardown + fresh client_id. Wipes paho's client-side + # QoS 1 queue, which is exactly the #1136 reproducer: an unacked + # `project_file` from the broken session would otherwise replay on + # reconnect, mixing stale commands into the next dispatch and + # triggering 0500_4003 SD R/W on the printer. + # + # Paho-network-thread callers (line ~2604/~2623 — dev-mode probe and + # ams_filament_setting zombie detection inside `_update_state`) + # → socket-close fallback. Calling `loop_stop()` from inside the + # network thread would self-join and deadlock; the safe pattern is + # to close the socket and let paho's own loop detect the broken + # connection and auto-reconnect (same instance, same client_id — + # queue replay is theoretically possible here but those paths have + # always done socket-close and #1136 was specifically triggered + # from the dispatch path). logger.warning("[%s] Forcing MQTT reconnect: %s", self.serial_number, reason) self._stale_reconnecting = True self.state.connected = False if self.on_state_change: self.on_state_change(self.state) + self._reset_client_for_reconnect() + + def _reset_client_for_reconnect(self) -> None: + """Route between hard-reset and socket-close based on caller thread. + + Hard-reset (preferred) requires we're not running on paho's network + thread, since `loop_stop()` on the same thread deadlocks. Detect via + ``asyncio.get_running_loop()`` — paho's callback thread has no loop; + every legitimate hard-reset caller (FastAPI handlers, background + async tasks) does.""" + try: + loop = asyncio.get_running_loop() + except RuntimeError: + loop = None + + if loop is not None: + self._loop = loop + self._hard_reset_client() + else: + self._socket_close_for_reconnect() + + def _hard_reset_client(self) -> None: + """Tear down the paho client entirely and rebuild it with a fresh + client_id, so the broker drops the old session and paho's local + QoS 1 queue is gone. Must NOT be called from paho's network thread. + Caller is responsible for setting ``_stale_reconnecting`` and + broadcasting the disconnected state.""" + old_client = self._client + self._client = None + if old_client is not None: + try: + old_client.disconnect() # MQTT DISCONNECT — broker drops session + except Exception: + pass + try: + old_client.loop_stop() # blocks briefly until the network thread exits + except Exception: + pass + # Skip reconnect if no asyncio loop is available (test environment or + # pre-init). The next initial connect() call from PrinterManager will + # set up the client fresh. + if self._loop is None: + return + try: + self.connect(loop=self._loop) + except Exception as e: + logger.error("[%s] Hard reset reconnect failed: %s", self.serial_number, e) + + def _socket_close_for_reconnect(self) -> None: + """Close the underlying socket so paho's loop thread detects the + broken connection and triggers auto-reconnect on the SAME client + instance. Safe to call from paho's own network thread (the loop + polls the socket on every iteration and handles a closed socket + gracefully). Used as a fallback when hard-reset isn't safe; queue + replay remains theoretically possible here but #1136 specifically + traced through the dispatch-deadline path which now hard-resets.""" if self._client: try: sock = self._client.socket() diff --git a/backend/tests/unit/services/test_bambu_mqtt.py b/backend/tests/unit/services/test_bambu_mqtt.py index 9beba2068..be749e86e 100644 --- a/backend/tests/unit/services/test_bambu_mqtt.py +++ b/backend/tests/unit/services/test_bambu_mqtt.py @@ -2983,7 +2983,11 @@ class TestDeveloperModeProbeTimeout: assert mqtt_client.state.connected is True def test_second_timeout_forces_reconnect(self, mqtt_client): - """After two consecutive probe timeouts, force-close the socket.""" + """After two consecutive probe timeouts, force-close the socket. + + Probe timeout detection runs from paho's network thread (no asyncio + loop), so force_reconnect_stale_session routes through socket-close + rather than hard-reset (loop_stop from inside the loop deadlocks).""" import time data = self._make_pushall_data() @@ -3005,9 +3009,8 @@ class TestDeveloperModeProbeTimeout: assert mqtt_client._dev_mode_probe_failures == 2 assert mqtt_client.state.connected is False assert mqtt_client._stale_reconnecting is True - # Socket should have been closed + # Sync test → no running loop → socket-close fallback path mqtt_client._client.socket().close.assert_called() - # on_state_change should have been called assert len(state_change_called) > 0 def test_successful_probe_resets_failure_counter(self, mqtt_client): @@ -4088,7 +4091,13 @@ class TestZombieSessionDetection: assert mqtt_client.state.connected is True def test_two_timeouts_force_reconnect(self, mqtt_client): - """Two consecutive unanswered commands trigger force_reconnect.""" + """Two consecutive unanswered commands trigger force_reconnect. + + Zombie detection runs from paho's network thread (no asyncio loop), so + the routing in force_reconnect_stale_session falls back to socket-close + — which is the safe option since loop_stop() from inside the loop + thread would deadlock. Hard-reset is reserved for async-context callers + (background_dispatch dispatch path).""" import time state_change_called = [] @@ -4107,6 +4116,7 @@ class TestZombieSessionDetection: assert mqtt_client._ams_cmd_unanswered == 0 # reset after reconnect assert mqtt_client.state.connected is False assert mqtt_client._stale_reconnecting is True + # Sync test → no running loop → socket-close fallback path mqtt_client._client.socket().close.assert_called() assert len(state_change_called) > 0 @@ -4223,3 +4233,113 @@ class TestHMSUserActionFiltering: mqtt_client._update_state({"print_error": 0x0500_8061}) assert len(mqtt_client.state.hms_errors) == 1 assert mqtt_client.state.hms_errors[0].code == "0x8061" + + +class TestForceReconnectRouting: + """#1136 — force_reconnect_stale_session routes between hard-reset (full + paho-client teardown, wipes the QoS 1 queue) and socket-close (the legacy + behaviour, safe to call from paho's own network thread). The routing + decision is based on whether an asyncio loop is running: hard-reset + requires loop_stop() which would deadlock if called from inside the + network thread itself.""" + + @pytest.fixture + def mqtt_client(self): + from unittest.mock import MagicMock + + from backend.app.services.bambu_mqtt import BambuMQTTClient + + client = BambuMQTTClient( + ip_address="192.168.1.100", + serial_number="TEST_HARD_RESET", + access_code="12345678", + ) + client.state.connected = True + client._client = MagicMock() + return client + + def test_routing_falls_back_to_socket_close_without_running_loop(self, mqtt_client): + """Sync caller → no asyncio loop → socket-close path (legacy behaviour + preserved for paho-thread callers like zombie detection).""" + mqtt_client.force_reconnect_stale_session("test") + mqtt_client._client.socket().close.assert_called() + # Old client is NOT torn down on this path; same-instance reconnect + # via paho's auto-reconnect handles it. + assert mqtt_client._client is not None + + def test_routing_uses_hard_reset_when_loop_is_running(self, mqtt_client): + """Async caller → loop available → hard-reset path wipes the queue.""" + import asyncio + + original = mqtt_client._client + # Stub connect() so the rebuild doesn't open a real socket. + mqtt_client.connect = lambda loop=None: None + + async def _trigger(): + mqtt_client.force_reconnect_stale_session("test") + + asyncio.run(_trigger()) + original.disconnect.assert_called() + original.loop_stop.assert_called() + # connect() stub didn't repopulate _client, so it's None — the contract + # in production is that connect() builds a fresh mqtt.Client here. + assert mqtt_client._client is None + + def test_marks_state_disconnected_and_broadcasts(self, mqtt_client): + """Both routing paths must broadcast the disconnected state once.""" + broadcasts: list[bool] = [] + mqtt_client.on_state_change = lambda s: broadcasts.append(s.connected) + mqtt_client.force_reconnect_stale_session("test") + assert mqtt_client.state.connected is False + assert mqtt_client._stale_reconnecting is True + assert broadcasts == [False] + + +class TestHardResetClientDirect: + """Lower-level coverage of `_hard_reset_client` itself — the helper called + by the routing layer when a full paho-client teardown is safe. These tests + drive the helper directly so they don't depend on the routing decision.""" + + @pytest.fixture + def mqtt_client(self): + from unittest.mock import MagicMock + + from backend.app.services.bambu_mqtt import BambuMQTTClient + + client = BambuMQTTClient( + ip_address="192.168.1.100", + serial_number="TEST_HARD_DIRECT", + access_code="12345678", + ) + client.state.connected = True + client._client = MagicMock() + # Stub connect() so the rebuild doesn't open a real socket. + client.connect = lambda loop=None: None + return client + + def test_disconnects_and_stops_old_client(self, mqtt_client): + """Old paho client must receive DISCONNECT (broker drops session) + + loop_stop (network thread exits, taking its QoS 1 queue with it).""" + original = mqtt_client._client + mqtt_client._hard_reset_client() + original.disconnect.assert_called() + original.loop_stop.assert_called() + + def test_clears_client_reference(self, mqtt_client): + """Old reference must go to None so subsequent code can't accidentally + publish through the dying client.""" + mqtt_client._hard_reset_client() + assert mqtt_client._client is None + + def test_swallows_disconnect_exception(self, mqtt_client): + """A failing disconnect() (e.g. paho already in error state) must not + propagate — the await chain in background_dispatch.py would otherwise + raise instead of moving on, and a single broken client could brick + every future dispatch.""" + original = mqtt_client._client + original.disconnect.side_effect = RuntimeError("boom") + # No exception escapes the call (test would fail if it did). + mqtt_client._hard_reset_client() + # loop_stop is still attempted after the disconnect failure. + original.loop_stop.assert_called() + assert mqtt_client._client is None