فهرست منبع

fix(printers): refuse to add a printer when the MQTT probe fails (#empty-card-reports)

  Several support reports traced back to one root cause: a mistyped access
  code in Add Printer left an empty card on the dashboard. POST /printers/
  was persisting the row first, then firing connect_printer() fire-and-forget.

  Now we test_connection() BEFORE the insert; failure returns HTTP 400 and
  the row is never written.

  Structured error response -- detail={"code", "message"} -- so the toast
  shows the localized message instead of the English fallback. New
  ApiError.code field on the frontend; printers.toast.connectionFailedNotAdded
maziggy 1 هفته پیش
والد
کامیت
b51598ea69

+ 3 - 0
CHANGELOG.md

@@ -4,6 +4,9 @@ All notable changes to Bambuddy will be documented in this file.
 
 ## [0.2.5b1] - Unreleased
 
+### Fixed
+- **Adding a printer with a wrong access code (or unreachable IP) no longer creates an empty card** — Several support reports traced back to a single root cause: the user mistyped their access code in the Add Printer dialog, `POST /printers/` happily persisted the row, the subsequent `printer_manager.connect_printer()` call was fire-and-forget so the failure was invisible, and the dashboard ended up showing a printer card that could never display state. The create route now runs `printer_manager.test_connection()` (the same MQTT probe the standalone Test Connection button has always used) BEFORE inserting the row, and refuses with HTTP 400 if the probe fails. The Printer row is never written on failure. **Structured error response**: backend returns `{detail: {code: "printer_connection_failed", message: "..."}}` rather than a plain English string — the new `ApiError.code` field on the frontend lets the toast layer pick a localized `printers.toast.connectionFailedNotAdded` key instead of surfacing the English fallback. **Existing tests** kept green via an autouse `_mock_printer_test_connection` fixture in `test_printers_api.py` that defaults the probe to success; a new `test_create_printer_rejects_when_mqtt_probe_fails` asserts the failure path returns 400, surfaces the stable code, AND verifies the row was not persisted (the critical part — earlier versions of the regression would have passed even if we'd left the row behind). 8 new i18n translations for `printers.toast.connectionFailedNotAdded` across all 8 locales; parity holds at 4831 leaves. 28 printer-route tests green.
+
 ### Changed
 - **GitHub backup: save-failure messages render inline on the card instead of as a toast** — The new "repository is not private" rejection message is ~250 chars listing every credential the backup carries, which clips badly in a toast. Both the initial-setup save and the debounced autosave now stash the backend's error message into a new `saveError` state and render it as a red inline banner above the test-result block, with `whitespace-pre-wrap` so the full message stays readable. The banner clears on a successful save, on the next save attempt, and as soon as the user starts editing the URL / token / provider (the three fields whose changes invalidate the privacy check) — so it doesn't linger after the user has already addressed the cause. Short success toasts ("Settings saved", "Token updated", "Backup enabled") are unchanged. Manual dismiss button included for users who want to clear it without retrying.
 

+ 28 - 1
backend/app/api/routes/printers.py

@@ -67,12 +67,39 @@ async def create_printer(
     _=RequirePermissionIfAuthEnabled(Permission.PRINTERS_CREATE),
     db: AsyncSession = Depends(get_db),
 ):
-    """Add a new printer."""
+    """Add a new printer.
+
+    Verifies the MQTT connection succeeds before persisting. A wrong access
+    code or unreachable IP would otherwise create a printer row that shows
+    as an empty / never-connecting card on the dashboard — those reports
+    were turning into support tickets that all traced back to a mistyped
+    access code.
+    """
     # Check if serial number already exists
     result = await db.execute(select(Printer).where(Printer.serial_number == printer_data.serial_number))
     if result.scalar_one_or_none():
         raise HTTPException(400, "Printer with this serial number already exists")
 
+    test_result = await printer_manager.test_connection(
+        ip_address=printer_data.ip_address,
+        serial_number=printer_data.serial_number,
+        access_code=printer_data.access_code,
+    )
+    if not test_result.get("success"):
+        # The frontend renders the user-facing message via i18n on `code`;
+        # `message` is an English fallback for non-UI clients (curl / scripts).
+        raise HTTPException(
+            status_code=400,
+            detail={
+                "code": "printer_connection_failed",
+                "message": (
+                    "Could not connect to the printer. Verify IP address, serial number, "
+                    "and access code, and confirm LAN-only mode is enabled. "
+                    "The printer was not added."
+                ),
+            },
+        )
+
     printer = Printer(**printer_data.model_dump())
     db.add(printer)
     await db.commit()

+ 55 - 0
backend/tests/integration/test_printers_api.py

@@ -11,6 +11,23 @@ from httpx import AsyncClient
 from sqlalchemy import select
 
 
+@pytest.fixture(autouse=True)
+def _mock_printer_test_connection():
+    """Default mock: connection test returns success.
+
+    POST /printers/ now refuses to persist a printer when the MQTT
+    connection probe fails (would otherwise leave an empty card in the
+    dashboard for a mistyped access code). Existing tests assume the
+    save succeeds, so we mock the probe green by default; the failure
+    branch is exercised by a dedicated test below.
+    """
+    with patch(
+        "backend.app.services.printer_manager.printer_manager.test_connection",
+        new=AsyncMock(return_value={"success": True, "state": "IDLE", "model": "X1C"}),
+    ) as m:
+        yield m
+
+
 class TestPrintersAPI:
     """Integration tests for /api/v1/printers/ endpoints."""
 
@@ -135,6 +152,44 @@ class TestPrintersAPI:
         # Should fail due to duplicate serial
         assert response.status_code in [400, 409, 422, 500]
 
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_create_printer_rejects_when_mqtt_probe_fails(self, async_client: AsyncClient, db_session):
+        """Wrong access code / unreachable IP must NOT persist the printer.
+
+        Regression: users were reporting empty / never-connecting printer
+        cards that traced back to a mistyped access code. The create route
+        now runs an MQTT probe up front and returns 400 if it fails — the
+        row is never written.
+        """
+        data = {
+            "name": "Bad Code Printer",
+            "serial_number": "00M09A999999999",
+            "ip_address": "192.168.1.250",
+            "access_code": "WRONG-CODE",
+            "is_active": True,
+            "model": "X1C",
+        }
+
+        with patch(
+            "backend.app.services.printer_manager.printer_manager.test_connection",
+            new=AsyncMock(return_value={"success": False, "state": None, "model": None}),
+        ):
+            response = await async_client.post("/api/v1/printers/", json=data)
+
+        assert response.status_code == 400
+        detail = response.json()["detail"]
+        # Backend returns a stable code for the frontend i18n layer to map;
+        # the message field is an English fallback for non-UI clients.
+        assert detail["code"] == "printer_connection_failed"
+        assert "connect" in detail["message"].lower()
+
+        # And critically: the printer row was never persisted.
+        from backend.app.models.printer import Printer
+
+        result = await db_session.execute(select(Printer).where(Printer.serial_number == "00M09A999999999"))
+        assert result.scalar_one_or_none() is None, "Failed-probe printer must not be persisted"
+
     # ========================================================================
     # Get single endpoint
     # ========================================================================

+ 13 - 2
frontend/src/api/client.ts

@@ -4,10 +4,15 @@ const API_BASE = '/api/v1';
 
 export class ApiError extends Error {
   status: number;
-  constructor(message: string, status: number) {
+  /** Stable error code from a structured backend detail (`{code, message}`).
+   *  Frontend uses this to look up an i18n key instead of showing the raw
+   *  English fallback. Null when the backend returned a plain-string detail. */
+  code: string | null;
+  constructor(message: string, status: number, code: string | null = null) {
     super(message);
     this.name = 'ApiError';
     this.status = status;
+    this.code = code;
   }
 }
 
@@ -105,6 +110,7 @@ async function request<T>(
     const error = await response.json().catch(() => ({}));
     const detail = error.detail;
     let message: string;
+    let code: string | null = null;
     if (typeof detail === 'string') {
       message = detail;
     } else if (Array.isArray(detail)) {
@@ -117,6 +123,11 @@ async function request<T>(
         .filter(Boolean)
         .join('; ');
       message = joined || JSON.stringify(detail) || `HTTP ${response.status}`;
+    } else if (detail && typeof detail === 'object') {
+      // Structured detail `{code, message}` — frontend uses the code to
+      // pick an i18n key, message is the English fallback.
+      code = typeof detail.code === 'string' ? detail.code : null;
+      message = typeof detail.message === 'string' ? detail.message : `HTTP ${response.status}`;
     } else {
       message = `HTTP ${response.status}`;
     }
@@ -136,7 +147,7 @@ async function request<T>(
       }
     }
 
-    throw new ApiError(message, response.status);
+    throw new ApiError(message, response.status, code);
   }
 
   // Handle empty responses (204 No Content, etc.)

+ 1 - 0
frontend/src/i18n/locales/de.ts

@@ -289,6 +289,7 @@ export default {
       printerUpdated: 'Drucker aktualisiert',
       failedToDelete: 'Drucker konnte nicht gelöscht werden',
       failedToAdd: 'Drucker konnte nicht hinzugefügt werden',
+      connectionFailedNotAdded: 'Keine Verbindung zum Drucker möglich. Prüfen Sie IP-Adresse, Seriennummer und Zugangscode und stellen Sie sicher, dass der Nur-LAN-Modus aktiv ist. Der Drucker wurde nicht hinzugefügt.',
       failedToUpdate: 'Drucker konnte nicht aktualisiert werden',
       commandSent: 'Befehl gesendet',
       failedToSendCommand: 'Befehl konnte nicht gesendet werden',

+ 1 - 0
frontend/src/i18n/locales/en.ts

@@ -289,6 +289,7 @@ export default {
       printerUpdated: 'Printer updated',
       failedToDelete: 'Failed to delete printer',
       failedToAdd: 'Failed to add printer',
+      connectionFailedNotAdded: 'Could not connect to the printer. Verify the IP, serial number, and access code, and confirm LAN-only mode is on. The printer was not added.',
       failedToUpdate: 'Failed to update printer',
       commandSent: 'Command sent',
       failedToSendCommand: 'Failed to send command',

+ 1 - 0
frontend/src/i18n/locales/fr.ts

@@ -289,6 +289,7 @@ export default {
       printerUpdated: 'Imprimante mise à jour',
       failedToDelete: 'Échec de la suppression',
       failedToAdd: 'Échec de l\'ajout',
+      connectionFailedNotAdded: 'Connexion à l\'imprimante impossible. Vérifiez l\'IP, le numéro de série et le code d\'accès, et confirmez que le mode LAN-only est activé. L\'imprimante n\'a pas été ajoutée.',
       failedToUpdate: 'Échec de la mise à jour',
       commandSent: 'Commande envoyée',
       failedToSendCommand: 'Échec de l\'envoi de la commande',

+ 1 - 0
frontend/src/i18n/locales/it.ts

@@ -289,6 +289,7 @@ export default {
       printerUpdated: 'Stampante aggiornata',
       failedToDelete: 'Impossibile eliminare stampante',
       failedToAdd: 'Impossibile aggiungere stampante',
+      connectionFailedNotAdded: 'Impossibile connettersi alla stampante. Verifica IP, numero di serie e codice di accesso e conferma che la modalità solo LAN sia attiva. La stampante non è stata aggiunta.',
       failedToUpdate: 'Impossibile aggiornare stampante',
       commandSent: 'Comando inviato',
       failedToSendCommand: 'Impossibile inviare comando',

+ 1 - 0
frontend/src/i18n/locales/ja.ts

@@ -288,6 +288,7 @@ export default {
       printerUpdated: 'プリンターを更新しました',
       failedToDelete: 'プリンターの削除に失敗しました',
       failedToAdd: 'プリンターの追加に失敗しました',
+      connectionFailedNotAdded: 'プリンターに接続できませんでした。IPアドレス、シリアル番号、アクセスコードを確認し、LAN専用モードが有効になっていることを確認してください。プリンターは追加されていません。',
       failedToUpdate: 'プリンターの更新に失敗しました',
       commandSent: 'コマンドを送信しました',
       failedToSendCommand: 'コマンドの送信に失敗しました',

+ 1 - 0
frontend/src/i18n/locales/pt-BR.ts

@@ -289,6 +289,7 @@ export default {
       printerUpdated: 'Impressora atualizada',
       failedToDelete: 'Falha ao excluir impressora',
       failedToAdd: 'Falha ao adicionar impressora',
+      connectionFailedNotAdded: 'Não foi possível conectar à impressora. Verifique o IP, o número de série e o código de acesso, e confirme que o modo somente LAN está ativado. A impressora não foi adicionada.',
       failedToUpdate: 'Falha ao atualizar impressora',
       commandSent: 'Comando enviado',
       failedToSendCommand: 'Falha ao enviar comando',

+ 1 - 0
frontend/src/i18n/locales/zh-CN.ts

@@ -289,6 +289,7 @@ export default {
       printerUpdated: '打印机已更新',
       failedToDelete: '删除打印机失败',
       failedToAdd: '添加打印机失败',
+      connectionFailedNotAdded: '无法连接到打印机。请检查 IP 地址、序列号和访问码,并确认已启用仅局域网(LAN-only)模式。该打印机未被添加。',
       failedToUpdate: '更新打印机失败',
       commandSent: '命令已发送',
       failedToSendCommand: '发送命令失败',

+ 1 - 0
frontend/src/i18n/locales/zh-TW.ts

@@ -289,6 +289,7 @@ export default {
       printerUpdated: '印表機已更新',
       failedToDelete: '刪除印表機失敗',
       failedToAdd: '新增印表機失敗',
+      connectionFailedNotAdded: '無法連線到印表機。請檢查 IP 位址、序號和存取碼,並確認已啟用僅區域網路(LAN-only)模式。該印表機未被新增。',
       failedToUpdate: '更新印表機失敗',
       commandSent: '命令已傳送',
       failedToSendCommand: '傳送命令失敗',

+ 10 - 2
frontend/src/pages/PrintersPage.tsx

@@ -63,7 +63,7 @@ import {
 } from 'lucide-react';
 
 import { useNavigate } from 'react-router-dom';
-import { api, discoveryApi, firmwareApi, withStreamToken } from '../api/client';
+import { api, discoveryApi, firmwareApi, withStreamToken, ApiError } from '../api/client';
 import { formatDateOnly, formatETA, formatDuration, parseUTCDate } from '../utils/date';
 import type { Printer, PrinterCreate, PrinterStatus, AMSUnit, DiscoveredPrinter, FirmwareUpdateInfo, FirmwareUploadStatus, LinkedSpoolInfo, SpoolAssignment, HMSError, InventorySpool, SmartPlug } from '../api/client';
 import { Card, CardContent } from '../components/Card';
@@ -6607,7 +6607,15 @@ export function PrintersPage() {
       queryClient.invalidateQueries({ queryKey: ['maintenanceOverview'] });
       setShowAddModal(false);
     },
-    onError: (error: Error) => showToast(error.message || t('printers.toast.failedToAdd'), 'error'),
+    onError: (error: Error) => {
+      // Localized message when the backend returns a stable error code;
+      // the raw message is an English fallback for non-UI clients.
+      if (error instanceof ApiError && error.code === 'printer_connection_failed') {
+        showToast(t('printers.toast.connectionFailedNotAdded'), 'error');
+        return;
+      }
+      showToast(error.message || t('printers.toast.failedToAdd'), 'error');
+    },
   });
 
   const powerOnMutation = useMutation({

تفاوت فایلی نمایش داده نمی شود زیرا این فایل بسیار بزرگ است
+ 0 - 0
static/assets/index-D4VfWhg_.js


+ 1 - 1
static/index.html

@@ -26,7 +26,7 @@
 
     <!-- Splash screens for iOS -->
     <link rel="apple-touch-startup-image" href="/img/android-chrome-512x512.png" />
-    <script type="module" crossorigin src="/assets/index-DxQAbwU0.js"></script>
+    <script type="module" crossorigin src="/assets/index-D4VfWhg_.js"></script>
     <link rel="stylesheet" crossorigin href="/assets/index-KYwGxnG9.css">
   </head>
   <body>

+ 4 - 5
test_backend.sh

@@ -3,9 +3,8 @@
 cd backend
 ruff check && ruff format --check
 
-#if [ "$1" = "--full" ]; then
+if [ "$1" = "--full" ]; then
 ../venv/bin/python3 -m pytest tests/ -v -n 30
-#else
-#../venv/bin/python3 -m pytest tests/ -v -n 30 --ignore=tests/unit/services/test_bambu_ftp.py
-#fi
-#cd ..
+else
+../venv/bin/python3 -m pytest tests/ -v -n 30 --ignore=tests/unit/services/test_bambu_ftp.py
+fi

برخی فایل ها در این مقایسه diff نمایش داده نمی شوند زیرا تعداد فایل ها بسیار زیاد است