Bladeren bron

Fix WebSocket crash on printers without fun field (#873)

  Race condition in _update_state: dev mode probe released GIL via MQTT
  publish between raw_data overwrite and vt_tray list restoration, letting
  the event loop iterate over raw dict keys (strings) instead of spool
  dicts. Affects A1, P1, and X1Plus firmware that don't send the fun field.

  Fix: normalize vt_tray dict→list before raw_data assignment, restore
  preserved fields before any GIL-releasing work, add defensive guard in
  printer_state_to_dict.
maziggy 1 maand geleden
bovenliggende
commit
d327c70006

+ 3 - 0
CHANGELOG.md

@@ -4,6 +4,9 @@ All notable changes to Bambuddy will be documented in this file.
 
 ## [0.2.3b2] - Unreleased
 
+### Fixed
+- **WebSocket Crash on Printers Without `fun` Field** ([#873](https://github.com/maziggy/bambuddy/issues/873)) — Connecting to printers that don't send the MQTT `fun` field (A1, P1 series, X1Plus firmware) caused a repeating `'str' object has no attribute 'get'` crash in the WebSocket handler, showing the printer as offline with missing AMS and SD card info. The developer mode probe introduced in 0.2.3b1 published an MQTT message inside `_update_state()` between overwriting `raw_data` with the full MQTT dict (where `vt_tray` is a raw dict) and restoring the previously normalized list — the `publish()` call released the GIL, letting the event loop read the un-normalized dict and iterate over string keys instead of spool dicts. Fixed by normalizing `vt_tray` dict→list in the MQTT data before assignment, and moving preserved field restoration before the probe. Added defensive normalization in `printer_state_to_dict` as a belt-and-suspenders guard.
+
 
 ## [0.2.3b1] - 2026-04-02
 

+ 19 - 8
backend/app/services/bambu_mqtt.py

@@ -2407,8 +2407,27 @@ class BambuMQTTClient:
         vt_tray_data = self.state.raw_data.get("vt_tray")
         ams_extruder_map_data = self.state.raw_data.get("ams_extruder_map")
         mapping_data = self.state.raw_data.get("mapping")
+
+        # Normalize vt_tray in data before assigning to raw_data: MQTT sends it
+        # as a dict but consumers expect a list.  Without this, the dev mode probe
+        # below can release the GIL (via publish), letting the event-loop thread
+        # read raw_data["vt_tray"] as a dict and crash iterating over string keys.
+        if "vt_tray" in data and isinstance(data["vt_tray"], dict):
+            data["vt_tray"] = [data["vt_tray"]]
+
         self.state.raw_data = data
 
+        # Restore preserved fields BEFORE any work that may release the GIL
+        # (e.g. _probe_developer_mode publishes an MQTT message).
+        if ams_data is not None:
+            self.state.raw_data["ams"] = ams_data
+        if vt_tray_data is not None:
+            self.state.raw_data["vt_tray"] = vt_tray_data
+        if ams_extruder_map_data is not None:
+            self.state.raw_data["ams_extruder_map"] = ams_extruder_map_data
+        if mapping_data is not None and "mapping" not in data:
+            self.state.raw_data["mapping"] = mapping_data
+
         # Parse developer LAN mode from "fun" field
         if "fun" in data:
             try:
@@ -2422,14 +2441,6 @@ class BambuMQTTClient:
             # Only probe after a full pushall response (30+ keys) to avoid firing on
             # partial incremental updates that arrive before the pushall with "fun".
             self._probe_developer_mode()
-        if ams_data is not None:
-            self.state.raw_data["ams"] = ams_data
-        if vt_tray_data is not None:
-            self.state.raw_data["vt_tray"] = vt_tray_data
-        if ams_extruder_map_data is not None:
-            self.state.raw_data["ams_extruder_map"] = ams_extruder_map_data
-        if mapping_data is not None and "mapping" not in data:
-            self.state.raw_data["mapping"] = mapping_data
 
         # Log mapping data when received (for usage tracking debugging)
         if "mapping" in data:

+ 7 - 1
backend/app/services/printer_manager.py

@@ -685,7 +685,13 @@ def printer_state_to_dict(state: PrinterState, printer_id: int | None = None, mo
 
     # Parse virtual tray (external spool) — now a list
     if "vt_tray" in raw_data:
-        for vt_data in raw_data["vt_tray"]:
+        vt_tray_raw = raw_data["vt_tray"]
+        # Defensive: MQTT sends vt_tray as a dict; normalize to list
+        if isinstance(vt_tray_raw, dict):
+            vt_tray_raw = [vt_tray_raw]
+        elif not isinstance(vt_tray_raw, list):
+            vt_tray_raw = []
+        for vt_data in vt_tray_raw:
             vt_tag_uid = vt_data.get("tag_uid")
             if vt_tag_uid in ("", "0000000000000000"):
                 vt_tag_uid = None

+ 96 - 0
backend/tests/unit/services/test_bambu_mqtt.py

@@ -2788,6 +2788,102 @@ class TestDeveloperModeDetection:
         assert mqtt_client.state.developer_mode is False
 
 
+class TestVtTrayNormalization:
+    """Tests for vt_tray dict→list normalization in _update_state.
+
+    MQTT sends vt_tray as a dict for single-slot printers, but all consumers
+    expect a list.  _update_state must normalize it before any callback can
+    read raw_data, because the dev-mode probe may release the GIL and let
+    the event loop read the partially-updated state.
+    """
+
+    @pytest.fixture
+    def mqtt_client(self):
+        from backend.app.services.bambu_mqtt import BambuMQTTClient
+
+        client = BambuMQTTClient(
+            ip_address="192.168.1.100",
+            serial_number="TEST123",
+            access_code="12345678",
+        )
+        return client
+
+    def test_vt_tray_dict_normalized_in_update_state(self, mqtt_client):
+        """Verify _update_state wraps a raw vt_tray dict into a list."""
+        vt_dict = {
+            "id": "254",
+            "tray_color": "FF0000",
+            "tray_type": "PLA",
+            "tag_uid": "0000000000000000",
+            "tray_uuid": "00000000000000000000000000000000",
+        }
+        data = {"gcode_state": "IDLE", "vt_tray": vt_dict}
+        mqtt_client._update_state(data)
+
+        stored = mqtt_client.state.raw_data.get("vt_tray")
+        assert isinstance(stored, list)
+        assert len(stored) == 1
+        assert stored[0]["tray_color"] == "FF0000"
+
+    def test_vt_tray_list_unchanged_in_update_state(self, mqtt_client):
+        """Verify _update_state keeps an already-list vt_tray unchanged."""
+        vt_list = [
+            {"id": "254", "tray_type": "PLA"},
+            {"id": "255", "tray_type": "PETG"},
+        ]
+        data = {"gcode_state": "IDLE", "vt_tray": vt_list}
+        mqtt_client._update_state(data)
+
+        stored = mqtt_client.state.raw_data.get("vt_tray")
+        assert isinstance(stored, list)
+        assert len(stored) == 2
+
+    def test_preserved_vt_tray_restored_before_probe(self, mqtt_client):
+        """Verify preserved vt_tray is restored before dev-mode probe runs.
+
+        On the first message, the incremental handler wraps vt_tray into a list
+        and stores it.  _update_state then replaces raw_data with the full data
+        dict, but must restore preserved fields BEFORE the probe publishes
+        (which can release the GIL).
+        """
+        # Simulate: incremental handler already stored a wrapped list
+        mqtt_client.state.raw_data = {
+            "vt_tray": [{"id": "254", "tray_type": "PLA", "tray_color": "00FF00"}],
+        }
+
+        # Now _update_state runs with new data that has vt_tray as dict
+        new_data = {
+            "gcode_state": "IDLE",
+            "vt_tray": {"id": "254", "tray_type": "PETG", "tray_color": "FF0000"},
+        }
+        mqtt_client._update_state(new_data)
+
+        # The preserved list (PLA/green) should take priority over new data
+        stored = mqtt_client.state.raw_data["vt_tray"]
+        assert isinstance(stored, list)
+        assert stored[0]["tray_type"] == "PLA"
+        assert stored[0]["tray_color"] == "00FF00"
+
+    def test_first_message_vt_tray_dict_becomes_list(self, mqtt_client):
+        """Verify on the very first message, vt_tray dict is still a list.
+
+        When there's no previously preserved data, the normalized dict should
+        remain as a list in raw_data.
+        """
+        # raw_data starts empty — no preserved vt_tray
+        mqtt_client.state.raw_data = {}
+
+        data = {
+            "gcode_state": "IDLE",
+            "vt_tray": {"id": "254", "tray_type": "ABS"},
+        }
+        mqtt_client._update_state(data)
+
+        stored = mqtt_client.state.raw_data["vt_tray"]
+        assert isinstance(stored, list)
+        assert stored[0]["tray_type"] == "ABS"
+
+
 class TestSendDryingCommand:
     """Tests for send_drying_command MQTT payload construction."""
 

+ 31 - 0
backend/tests/unit/services/test_printer_manager.py

@@ -782,6 +782,37 @@ class TestPrinterStateToDict:
         assert result["vt_tray"][0]["tray_color"] == "00FF00"
         assert result["vt_tray"][0]["tray_type"] == "PETG"
 
+    def test_vt_tray_dict_normalized_to_list(self, mock_state):
+        """Verify vt_tray as a raw dict (from MQTT) is normalized to a list."""
+        mock_state.raw_data = {
+            "vt_tray": {
+                "id": "254",
+                "tray_color": "FF0000",
+                "tray_type": "PLA",
+                "tray_sub_brands": "Generic",
+                "tag_uid": "0000000000000000",
+                "tray_uuid": "00000000000000000000000000000000",
+                "remain": 0,
+            }
+        }
+
+        result = printer_state_to_dict(mock_state)
+
+        assert isinstance(result["vt_tray"], list)
+        assert len(result["vt_tray"]) == 1
+        assert result["vt_tray"][0]["tray_color"] == "FF0000"
+        assert result["vt_tray"][0]["tray_type"] == "PLA"
+        assert result["vt_tray"][0]["tag_uid"] is None
+        assert result["vt_tray"][0]["tray_uuid"] is None
+
+    def test_vt_tray_non_list_non_dict_ignored(self, mock_state):
+        """Verify unexpected vt_tray types (e.g. string) produce empty list."""
+        mock_state.raw_data = {"vt_tray": "unexpected_string"}
+
+        result = printer_state_to_dict(mock_state)
+
+        assert result["vt_tray"] == []
+
     def test_hms_errors_conversion(self, mock_state):
         """Verify HMS errors are converted correctly."""
         error = MagicMock()