From 9cde2e37fd9eb54d4c1750499992520c30829b5d Mon Sep 17 00:00:00 2001 From: maziggy Date: Wed, 13 May 2026 10:43:13 +0200 Subject: [PATCH] feat(slicer): log sidecar reject reason on bundle import failure (#1312) The route mapped sidecar 4xx/5xx to HTTPException with detail but never logged it. Reporters seeing a 400 toast were giving us only the status code, not the reason, and the access-log line was all that landed in support bundles. Add WARNING-level logs on each error branch (400 SlicerInputError, 503 SlicerApiUnavailableError, 502 SlicerApiError) with the sidecar's own message + the filename / byte count / configured URL. Next reporter on this code path produces a support bundle that contains the answer. --- CHANGELOG.md | 2 ++ backend/app/api/routes/slicer_presets.py | 16 +++++++++++++++- backend/tests/unit/test_slicer_presets.py | 8 +++++++- 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 83cfc9450..fd0c40e01 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,8 @@ All notable changes to Bambuddy will be documented in this file. - **Spoolman weight tracking now uses per-print grams for all spools, matching the internal Filament Inventory** ([#1119](https://github.com/maziggy/bambuddy/issues/1119), reported by @Moskito99) — Spoolman previously had two mutually-exclusive weight paths: AMS remain%×tray_weight auto-sync (default; only worked for Bambu Lab spools with valid RFID tray_weight) and per-print 3MF-grams tracking (only enabled when "Disable AMS Weight Sync" was toggled on). Non-BL spools without RFID fell through both paths — AMS auto-sync had no tray_weight to multiply, and the inventory_remaining fallback was wiped because activating Spoolman deletes the internal `spool_assignment` table — so Spoolman never saw a weight update for them. The internal Filament Inventory has no such gap: it always uses per-print 3MF grams as the primary path with AMS-remain% delta as fallback, and it works for every spool type. Spoolman now does the same: per-print tracking runs whenever Spoolman is enabled and is the only writer of `remaining_weight`. AMS auto-sync continues to maintain spool metadata and slot assignments but no longer touches weight (eliminating the double-count that would otherwise occur for BL spools with both paths active). `store_print_data` ([`spoolman_tracking.py:159`](backend/app/services/spoolman_tracking.py)) had its `disable_weight_sync` early-return removed; the three `sync_ams_tray` callsites (`main.py:1450` auto-sync, `spoolman.py:318` per-printer manual, `spoolman.py:517` sync-all) now hard-code `disable_weight_sync=True`. The `spoolman_disable_weight_sync` setting is now deprecated and a no-op — kept in the DB/UI for backwards compat. Behavioral consequence for existing users on the default flag (False): live AMS-based remaining_weight updates between prints stop happening; weight updates now arrive once per print completion with 3MF gram precision. Regression test in `test_spoolman_tracking.py::test_stores_tracking_when_disable_weight_sync_is_false` proves the early-return is gone. ### Fixed +- **Slicer bundle import 400/502/503 errors now land in the log so support bundles tell us why** ([#1312](https://github.com/maziggy/bambuddy/issues/1312), reported by @hasmar04) — Reporter hit `400 Bad Request` from `POST /api/v1/slicer/bundles` when uploading a Bambu Studio Printer Preset Bundle (`.bbscfg`); a second contributor had reported the same shape the day before. Same bundle file uploaded fine on Martin's dev machine, which strongly points at sidecar-side differences (image version, write permissions on `DATA_PATH/bundles`, TrueNAS Docker volume perms, etc.) — but triage was blocked because the sidecar's actual reject reason only made it as far as the FE toast. Bambuddy logged just the uvicorn-access line (`POST /api/v1/slicer/bundles HTTP/1.1 400`), with no detail in the support bundle. The route at `backend/app/api/routes/slicer_presets.py:import_slicer_bundle` now emits a `logger.warning` for each of the three failure shapes: **400 (`SlicerInputError`)** — sidecar's reject string is logged alongside the filename and byte count, so we can see "bundle rejected because `manifest.json` is missing" in the next support bundle without asking the reporter to copy the toast text. **503 (`SlicerApiUnavailableError`)** — logs the configured sidecar URL plus the exception detail (separates "URL wrong" from "sidecar offline"). **502 (`SlicerApiError`)** — logs filename + byte count + error string, useful when the sidecar's `DATA_PATH/bundles` write fails (the typical 5xx cause on this path). The 400 case is `WARNING` rather than `INFO` deliberately — it's an unexpected end-user-visible failure, not a routine event. Existing `test_import_bundle_sidecar_400_passes_through` now also asserts the reject reason AND the filename appear in caplog, so the support-bundle-includes-the-diagnostic contract is pinned. Doesn't fix #1312's actual root cause (sidecar-side, still under investigation with reporter) — but the next reporter we get on this code path will produce a bundle that contains the answer. + - **Restarting Bambuddy mid-print triggered plate-check pause + duplicate archive** ([#1304](https://github.com/maziggy/bambuddy/issues/1304), reported by @kleinwareio) — When a P1S print was in progress and the user updated the Bambuddy container (`latest` → `daily` in the report, but the same path fires on any restart), Bambuddy paused the live print with an "Object detected on build plate" warning AND re-archived the in-progress file as a duplicate. Root cause: the print-start detector at `backend/app/services/bambu_mqtt.py:2780` gated on `self._previous_gcode_state != "RUNNING"`, which is true whether we just saw IDLE→RUNNING (a real print start) OR we just constructed a fresh BambuMQTTClient and `_previous_gcode_state` is still its initial `None` (catch-up push from a printer already running). The fresh-client case fired `on_print_start`, which downstream ran the plate-detection-and-pause flow at `main.py` AND the FTP-download-and-archive flow — exactly the two symptoms in the bug report. Fix: added `self._previous_gcode_state is not None` to the `is_new_print` guard, so the first push from the printer in a new process lifetime never counts as a state transition into RUNNING. `_was_running` still flips to `True` via the unconditional "Track RUNNING state" block at `bambu_mqtt.py:2795`, so print-completion detection keeps working — only the start callback is suppressed. Three existing tests that asserted on the old (buggy) behavior were updated to seed `_previous_gcode_state = "IDLE"` first, matching the realistic lifecycle of a print actually starting (Bambuddy has been observing IDLE/FINISH before RUNNING); they now exercise the correct path. New regression test `test_first_running_push_after_bambuddy_restart_does_not_fire_print_start` pins the contract for the reporter's exact scenario — and asserts that `_was_running` still becomes True so completion still fires when the print ends. The `is_file_change` branch was unaffected (it already required `_previous_gcode_file is not None`, so restart-catch-up never reached it anyway). - **Create User form rejected weak passwords with an opaque "HTTP 422" toast** ([#1303](https://github.com/maziggy/bambuddy/issues/1303), reported by @TrickShotMLG02) — Three independent UX gaps stacked on top of each other. **(1) Discoverability**: the Create User and Edit User modals showed no hint about the backend's password complexity requirements (`min 8 chars` + uppercase + lowercase + digit + special character; enforced in `backend/app/schemas/auth.py:_validate_password_complexity`). Reporter typed an 8-character all-digits password and had no way to know why it failed. **(2) Validation mismatch**: the frontend's pre-submit check at `SettingsPage.tsx` was only `password.length < 6`, accepting passwords the backend would reject — every weak password got bounced after the round-trip instead of getting blocked locally. **(3) Error display fragility**: when the backend returned a 422 with a Pydantic detail array, the API client's error parser at `frontend/src/api/client.ts:107` could fall through to the bare `HTTP ${status}` fallback if the mapped/filtered detail array ended up empty after stripping the `"Value error, "` prefix — masking the real reason as just "HTTP 422". Fixes: (1) added a `passwordRequirements` helper line under both password inputs in Create User / Edit User; (2) extracted `checkPasswordComplexity` into `frontend/src/utils/password.ts`, called from `handleCreateUser` and `handleUpdateUser` before the API request — it returns the same FIRST failing rule the backend's validator would have flagged (uppercase before lowercase before digit before special, matching `_validate_password_complexity`'s order — fixing one rule shouldn't immediately trip a different message), and the submit button is disabled until all rules pass; (3) the API client now falls back to `JSON.stringify(detail)` when the mapped array is empty, so a malformed but non-empty 422 detail surfaces SOMETHING informative instead of a bare status code. New translation keys `settings.passwordRequirements`, `settings.toast.passwordNeeds{Uppercase, Lowercase, Digit, Special}`, plus the existing `passwordTooShort` text updated from "6 characters" to "8 characters". English + German fully translated (German reporter's locale); FR/IT/PT-BR translated using straightforward equivalents; JA/ZH-CN/ZH-TW seeded with English for the new complexity messages (existing project flow for new strings). 7 new unit tests in `frontend/src/__tests__/utils/password.test.ts` pin the validator's contract, including the reporter's exact `"12345678"` input which now produces a local "Password must contain at least one uppercase letter" toast instead of a 422 round-trip. diff --git a/backend/app/api/routes/slicer_presets.py b/backend/app/api/routes/slicer_presets.py index ee166e39e..881178a32 100644 --- a/backend/app/api/routes/slicer_presets.py +++ b/backend/app/api/routes/slicer_presets.py @@ -427,11 +427,25 @@ async def import_slicer_bundle( except SlicerInputError as e: # Sidecar's 4xx — most likely a non-.bbscfg upload, a corrupt zip, # or a path-traversal entry that the manifest validator caught. - # Surface verbatim so the user sees the actual reason in the toast. + # Log the detail so it lands in the support bundle: the FE-only + # toast was leaving us blind during triage (#1312). + logger.warning( + "Bundle import rejected by sidecar (%s, %d bytes): %s", + filename, + len(contents), + e, + ) raise HTTPException(status_code=400, detail=str(e)) from e except SlicerApiUnavailableError as e: + logger.warning("Bundle import: sidecar unreachable (%s): %s", api_url, e) raise HTTPException(status_code=503, detail=str(e)) from e except SlicerApiError as e: + logger.warning( + "Bundle import: sidecar server error (%s, %d bytes): %s", + filename, + len(contents), + e, + ) # 5xx from the sidecar's import path is rare — usually a disk # write failure inside DATA_PATH/bundles. 502 (bad gateway) is # closer to the truth than 500 here, since we're proxying. diff --git a/backend/tests/unit/test_slicer_presets.py b/backend/tests/unit/test_slicer_presets.py index d537ce01e..5ee1a0053 100644 --- a/backend/tests/unit/test_slicer_presets.py +++ b/backend/tests/unit/test_slicer_presets.py @@ -516,7 +516,7 @@ class TestBundleRoutes: assert exc.value.status_code == 400 @pytest.mark.asyncio - async def test_import_bundle_sidecar_400_passes_through(self): + async def test_import_bundle_sidecar_400_passes_through(self, caplog): from io import BytesIO from fastapi import HTTPException, UploadFile @@ -527,6 +527,7 @@ class TestBundleRoutes: with ( patch.object(sp, "_resolve_slicer_api_url", AsyncMock(return_value="http://ok")), patch.object(sp, "SlicerApiService", return_value=svc), + caplog.at_level("WARNING", logger="backend.app.api.routes.slicer_presets"), pytest.raises(HTTPException) as exc, ): await sp.import_slicer_bundle( @@ -535,6 +536,11 @@ class TestBundleRoutes: _=None, ) assert exc.value.status_code == 400 + # #1312: the sidecar's reject reason MUST land in the log so it + # ends up in support bundles without us having to ask reporters + # to copy the FE toast. + assert any("bad zip" in r.message for r in caplog.records) + assert any("x.bbscfg" in r.message for r in caplog.records) @pytest.mark.asyncio async def test_import_bundle_sidecar_unreachable_returns_503(self):