From f15e54c383b3e47fc22b4414b4e85c0620929c63 Mon Sep 17 00:00:00 2001 From: maziggy Date: Sat, 13 Jun 2026 07:49:31 +0200 Subject: [PATCH] fix(windows): _local_zone falls back to stdlib utc when zoneinfo DB is missing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Windows installer's embedded Python doesn't carry an IANA tz database, and the stdlib zoneinfo has no system DB to read on Windows. ZoneInfo("UTC") raises ZoneInfoNotFoundError on those installs, and the new /api/local-backup/status endpoint 500s on the resulting uncaught exception. Surfaced via a Windows traceback from a user's log: File "...\backend\app\services\local_backup.py", line 32, in _local_zone return ZoneInfo("UTC") zoneinfo._common.ZoneInfoNotFoundError: 'No time zone found with key UTC' _local_zone()'s try/except only covered the TZ-env branch — both fallbacks unconditionally called ZoneInfo("UTC") and re-raised. Fix (two parts): 1. services/local_backup.py — return type widened from ZoneInfo to tzinfo, the UTC fallback is wrapped in its own try, and the last-resort fallback returns datetime.timezone.utc (stdlib, no IANA DB needed). str(timezone.utc) == "UTC" so the response shape on /api/local-backup/status is unchanged. The astimezone call in _calculate_next_run accepts any tzinfo — no other call sites affected. 2. requirements.txt — pin tzdata>=2024.1; sys_platform == "win32" so the next Windows installer build ships the IANA DB, and any non- UTC TZ value (e.g. Europe/Berlin) resolves correctly. The stdlib fallback can only ever give UTC. Linux/macOS unaffected by the platform marker — they already have the system tz database. --- CHANGELOG.md | 1 + backend/app/services/local_backup.py | 24 +++++++++++++++++------- backend/tests/unit/test_local_backup.py | 19 +++++++++++++++++++ requirements.txt | 8 ++++++++ 4 files changed, 45 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e4eb618a..880aa6ef3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ All notable changes to Bambuddy will be documented in this file. ## [0.2.5b1] - Unreleased ### Fixed +- **Windows: `/api/local-backup/status` 500 on `ZoneInfoNotFoundError: 'No time zone found with key UTC'` (from a user's log on the Windows installer)** — Reported via a Windows traceback against the new local-backup status endpoint. The stdlib `zoneinfo` module reads the system IANA tz database on Linux/macOS, but Windows has none — and the embedded Python in our Windows installer doesn't carry the `tzdata` PyPI package either, so even `ZoneInfo("UTC")` raises `ZoneInfoNotFoundError`. `_local_zone()` in `services/local_backup.py` only caught that exception for the `TZ`-env branch; the empty-`TZ` fallback and the unrecognised-`TZ` fallback both unconditionally called `ZoneInfo("UTC")` and re-raised, bubbling out of the FastAPI handler as a 500. **Fix (two parts):** (1) `_local_zone()` is now resilient — return type widened from `ZoneInfo` to `tzinfo`, the `UTC` fallback is wrapped in its own try, and the last-resort fallback returns the stdlib `datetime.timezone.utc` (which needs no IANA DB and satisfies every `astimezone` / `str()` call site downstream — `str(timezone.utc) == "UTC"` matches the previous response shape). Restores function on existing Windows installs without re-bundling. (2) `requirements.txt` now pins `tzdata>=2024.1; sys_platform == "win32"` so the next Windows installer build ships the IANA DB and any non-UTC `TZ` value (e.g. `Europe/Berlin`) resolves correctly — the stdlib fallback can only ever give UTC. Linux/macOS unaffected: the platform marker keeps them on the system tz DB they already have. **Tests:** new `test_zoneinfo_completely_unavailable_falls_back_to_stdlib_utc` in `test_local_backup.py` monkeypatches `ZoneInfo` to always raise `ZoneInfoNotFoundError` and pins that `_local_zone()` returns `datetime.timezone.utc` rather than propagating. 31/31 local_backup tests green; ruff clean. - **Print-modal "off" toggles for `flow_cali` and `nozzle_offset_cali` now actually suppress the calibration stage (live-tested on H2D 01.x)** — The Re-print / Schedule modal toggles for Flow Calibration and Nozzle Offset Calibration accepted the user's "off" choice and flowed it correctly through to the `project_file` MQTT publish — Bambuddy sent `extrude_cali_flag: 2` and `nozzle_offset_cali: 2` per our reading of "1 = run, 2 = skip" inherited from the #1478 / #1682 work. Live test on an H2D running firmware 01.x: with both toggles off in Bambuddy's modal, the printer's `stg` queue (the pre-print stage list firmware publishes via push_status) still included stage **8** ("Calibrating dynamic flow") and stage **39** ("Nozzle offset calibration") — and physically ran them at print start. The `2` value did NOT suppress the stage despite our earlier "skip and reuse stored PA" reading. **Root cause:** the encoding for the "off" wire value is `0`, not `2`. The `2` value appears to mean "skip the explicit calibration pass but still apply / verify the stored PA value via the calibration stage" — close to a no-op in terms of K-factor but the printer still queues the stage and runs the per-print physical sequence. `0` is what actually drops the stage from the `stg` queue. A real BambuStudio Send-dialog capture on the same firmware (proxy-mode VP echo) also showed `0` for both fields when calibrations are unchecked, contradicting the #1478 commit message which read `0` as "never sent by BambuStudio." **Fix:** `bambu_mqtt.py::start_print` — `extrude_cali_flag` is now `1 if flow_cali else 0` (was `2`), and `nozzle_offset_cali` is `1 if (nozzle_offset_cali and is_dual_nozzle) else 0` (was `2`). The dual-nozzle gate stays — single-nozzle prints continue to force-skip the nozzle-offset calibration their head doesn't support (#1682). `1` (run) is unchanged on both fields. **Verification:** live re-test on the same H2D with both toggles still off — `stg: [29, 13, 4, 14, 3]` (cooling, homing, filament change, nozzle cleaning, vibration comp). Stages 8 and 39 dropped out cleanly. **What's NOT fixed:** `vibration_cali` is a JSON `false` bool in both Bambuddy's and BambuStudio's wire format, and the H2D firmware queues stage **3** ("Vibration compensation") regardless of the bool value — this is firmware-side and not solvable at our dispatch layer with the current field. Captured as a follow-up to investigate whether a parallel `vibration_cali_flag` integer field exists. **Tests:** `test_bambu_mqtt.py` — `test_p2s_uses_boolean_format` flipped `extrude_cali_flag == 2` → `== 0`; `test_nozzle_offset_cali_default_is_skip`, `test_nozzle_offset_cali_ignored_on_single_nozzle`, `test_nozzle_offset_cali_false_on_dual_nozzle` flipped `== 2` → `== 0`; docstrings updated to reflect the #1721 finding. The `1 if user_wants` branch in both tests for the "on" case is unchanged. 281/281 bambu_mqtt tests green; full backend suite 5941/5941 green with `-n 30`; ruff clean; frontend untouched (rebuild + i18n parity confirmed clean per `feedback_run_all_ci_checks`). - **Support-bundle log noise: VP bridge nudge + SD-card cleanup (#1721 adjacent, observed on reporter's A1)** — Two warnings polluting every A1 support bundle on a healthy print. Neither was the cause of #1721's timelapse complaint — both are adjacent noise. **(1) `request_status_update: not connected`** — `mqtt_bridge.py::_resolve_client` calls `_request_version` + `request_status_update` immediately after attaching a raw-message handler so the bridge cache populates without waiting for the next periodic pushall. The bind frequently races the real printer's MQTT TLS handshake — a slicer-side reconnect re-resolves the client before the underlying session has reconnected, especially on A1 firmware which reconnects more aggressively than X1/H2/P. `request_status_update` logs `[serial] request_status_update: not connected` at WARNING on the not-connected return path. The nudge is a best-effort optimisation; the fall-through (next periodic pushall) populates the cache anyway, so the WARNING fires on routine, expected, recoverable state. **Fix:** gate both nudges on `current.state.connected` at the bind site. When the client comes up, the next `_resolve_client` tick re-enters this branch on identity change OR the periodic pushall in `bambu_mqtt.py` fills the cache — same end state, no benign WARNING. The WARNING in `bambu_mqtt.py:3224` is unchanged: it's still a real signal for the other callers (`/printers/{id}/refresh-status` user API, bug-reporter helper) where "you asked for a refresh on a dead client" is genuinely worth logging. New `test_post_bind_nudge_skipped_when_target_not_connected` in `test_vp_mqtt_bridge.py::TestBridgeLifecycle` pins the contract. **(2) `SD card cleanup failed after 3 attempts ... (file may linger on SD card)`** — The post-finish helper in `main.py` deletes the uploaded file from the printer's SD card to prevent the ghost-print-on-power-cycle behaviour (#374, #1542). It tries up to three candidate paths (`derive_remote_filename(archive.filename)`, then `{subtask_name}.3mf`, then `{subtask_name}.gcode`), each up to 3 times with 2 s backoff, then logs WARNING if all fail. `delete_file_async` returned `bool` — `True` for success, `False` for ANYTHING else (FTP 550 file-not-found, network error, auth fail, transient FTP error). The A1 firmware (and most other Bambu firmwares post-print) cleans the SD-card upload itself before our cleanup runs, every candidate FTP-DELE returns 550, all three retries × three candidates × 2 s sleeps fire, then WARNING. That WARNING shouldn't exist on a healthy print where the printer self-cleaned. The same shape exists in `_cleanup_forced_timelapse` (#1397) walking the four timelapse dirs. **Fix:** `bambu_ftp.py` now exports a `DeleteResult` enum (`DELETED` / `NOT_FOUND` / `FAILED`). `BambuFTPClient.delete_file` detects the 550 case via `isinstance(e, ftplib.error_perm) and str(e).startswith("550")` (same pattern already used in the download path for the symmetric `FileNotOnPrinterError` sentinel from #972). `delete_file_async` now returns `DeleteResult`. Both post-finish cleanup helpers (`main.py::on_print_finished` SD branch + `_cleanup_forced_timelapse`) only WARN when at least one candidate returned `FAILED`; an all-`NOT_FOUND` outcome logs DEBUG ("nothing to delete — printer likely self-cleaned"). The cleanup helper also no longer burns the 2 s × 3 retry budget on a `NOT_FOUND` result (550 will never recover by waiting); only `FAILED` triggers backoff. `DELETE /printers/{id}/files/...` returns 404 (not 500) on `NOT_FOUND`, more accurate for the user-facing UI. Three other production callers (`print_scheduler` pre-upload delete, two `background_dispatch` fire-and-forget cleanups) are unchanged at the call site — they discard the return value. **Tests:** `test_delete_file` and `test_delete_file_async` in `test_bambu_ftp.py` switched to the enum (3 cases each). 2 new regression tests in `test_cleanup_forced_timelapse.py`: `test_forced_no_warning_when_every_dir_returns_not_found` pins the #1721 path (every candidate dir → 550 → no WARNING, one DEBUG summary), `test_forced_warns_when_any_dir_returns_failed` pins the counterpart (any FAILED keeps the WARNING — that's the signal the maintainer wants). `caplog` asserts the log record's level + content directly. Full backend suite 5941/5941 green; ruff clean; frontend untouched (rebuild + i18n parity confirmed clean per `feedback_run_all_ci_checks`). No migration, no new i18n keys, no schema changes. - **Virtual printer external spool (`vt_tray`) went "invalid" right after a slicer filament pick (#1622 round 5, reported by @shaddowlink)** — On a P1S in non-proxy VP mode, the reporter picked a filament for the external spool slot in BambuStudio's Device tab and the slot immediately rendered as invalid (color only, no profile, no K-profile, no nozzle temps), but recovered after a virtual-printer reload. AMS slot picks worked correctly. Wire dumps (BAMBUDDY_VP_DUMP_WIRE=1) captured the asymmetry: the bridge's outgoing 1 Hz cached-as-base push delivered `vt_tray = {tray_info_idx, tray_color}` — 2 fields — where a real P1S sends ~20 (`tray_type`, `state`, `remain`, `k`, `n`, `cali_idx`, `nozzle_temp_min/max`, `tray_uuid`, `xcam_info`, ...). The same `_out.json` showed AMS slots with the full 24-field dict because `_merge_ams_dict` deep-merged them. **Root cause:** Bambu firmware sends a partial `vt_tray` incremental right after acknowledging an `ams_filament_setting` for `ams_id=255` (external spool) — carrying just the fields the slicer's pick changed. The round-4 per-field accumulate (#1622 / da799447) carried over prev keys NOT present in new, but `vt_tray` IS present in new, so the cached dict was REPLACED wholesale with the 2-field partial. The next 1 Hz cached-as-base push handed the slicer the stripped vt_tray; BambuStudio rendered the slot as invalid. Reloading the VP forced a reconnect → pushall → full vt_tray restored, and the cycle repeated on the next pick. **Fix:** `mqtt_bridge.py::_on_printer_raw` now applies the same per-field accumulate one level deeper: for every top-level key whose prev AND new are both dicts, overlay new onto prev rather than replace. `ams` is explicitly excluded (already deep-merged by `_merge_ams_dict`). The same overlay protects `device`, `online`, `upgrade_state`, `ipcam`, `upload`, `net` against future firmware partials with the same shape; the `net.info` IP rewrite path is unaffected because `_rewrite_net_info_ips` runs against `new_state["net"]` before caching and the rewritten list overrides the cached one on overlay (only `net.conf` and friends, when sent without `info`, draw from prev now). **Tests:** new `test_partial_vt_tray_update_overlays_onto_cached_full_dict` regression case in `test_vp_mqtt_bridge.py::TestPushStatusCache` constructs the exact P1S wire shape — pushall with the full ~20-field vt_tray, followed by the `{tray_info_idx, tray_color}` partial that shaddowlink's dump captured — and asserts `tray_type`, `state`, `remain`, `k`, `n`, `cali_idx`, `nozzle_temp_min/max`, `tray_uuid`, `id` all survive while the two incoming fields take their new values. All 53 bridge tests stay green; 287/287 across the broader VP test surface (mqtt_bridge / mqtt_server / vp_wire / virtual_printer); ruff clean. Bridge code path only; no migration, no new i18n keys, no frontend touch. diff --git a/backend/app/services/local_backup.py b/backend/app/services/local_backup.py index 8e4e1db6e..77904d675 100644 --- a/backend/app/services/local_backup.py +++ b/backend/app/services/local_backup.py @@ -7,7 +7,7 @@ on a configurable schedule with retention management. import asyncio import logging import os -from datetime import datetime, timedelta, timezone +from datetime import datetime, timedelta, timezone, tzinfo from pathlib import Path from zoneinfo import ZoneInfo, ZoneInfoNotFoundError @@ -20,21 +20,31 @@ from backend.app.models.settings import Settings logger = logging.getLogger(__name__) -def _local_zone() -> ZoneInfo: +def _local_zone() -> tzinfo: """Resolve the local timezone for scheduled-backup HH:MM interpretation. Uses the container's ``TZ`` env var (the same value the support package surfaces); falls back to UTC when unset or unrecognised so a missing TZ keeps the legacy behaviour rather than crashing. See #1602 follow-up. + + On Windows the embedded Python in our installer doesn't carry an IANA + tz database, so ``ZoneInfo(...)`` — including ``ZoneInfo("UTC")`` — + raises ``ZoneInfoNotFoundError`` unless the ``tzdata`` PyPI package is + installed. requirements.txt now pins ``tzdata`` on win32, but to keep + this resilient on installs that haven't refreshed deps we fall through + to the stdlib ``datetime.timezone.utc`` as a last resort; it satisfies + every ``astimezone`` / ``str()`` call site without needing the IANA DB. """ tz_name = os.environ.get("TZ", "").strip() - if not tz_name: - return ZoneInfo("UTC") + if tz_name: + try: + return ZoneInfo(tz_name) + except ZoneInfoNotFoundError: + logger.warning("Unrecognised TZ env value %r, scheduling in UTC", tz_name) try: - return ZoneInfo(tz_name) - except ZoneInfoNotFoundError: - logger.warning("Unrecognised TZ env value %r, scheduling in UTC", tz_name) return ZoneInfo("UTC") + except ZoneInfoNotFoundError: + return timezone.utc SCHEDULE_INTERVALS = { diff --git a/backend/tests/unit/test_local_backup.py b/backend/tests/unit/test_local_backup.py index 8094d0155..47607ca5b 100644 --- a/backend/tests/unit/test_local_backup.py +++ b/backend/tests/unit/test_local_backup.py @@ -137,6 +137,25 @@ class TestCalculateNextRun: result = service._calculate_next_run("daily", "21:00") assert result == datetime(2026, 6, 15, 21, 0, 0, tzinfo=timezone.utc) + def test_zoneinfo_completely_unavailable_falls_back_to_stdlib_utc(self, monkeypatch): + """Windows installer ships an embedded Python without the IANA tz DB + (no system tzdata, no ``tzdata`` PyPI package). Even ``ZoneInfo("UTC")`` + raises ``ZoneInfoNotFoundError`` then, and /api/local-backup/status + 500s. The fallback must catch that and return ``datetime.timezone.utc`` + so scheduling still works without the DB. + """ + from zoneinfo import ZoneInfoNotFoundError + + from backend.app.services import local_backup as lb_module + + monkeypatch.delenv("TZ", raising=False) + + def _always_missing(_key): + raise ZoneInfoNotFoundError("no tz database on this platform") + + monkeypatch.setattr(lb_module, "ZoneInfo", _always_missing) + assert lb_module._local_zone() is timezone.utc + def test_dst_spring_forward_gap_does_not_crash(self, monkeypatch): """Europe/Berlin spring-forward 2026-03-29 jumps 02:00 → 03:00 local; 02:30 wall-clock does not exist. ``replace(hour=2, minute=30)`` should diff --git a/requirements.txt b/requirements.txt index 614d37fb8..7a930b96a 100644 --- a/requirements.txt +++ b/requirements.txt @@ -65,6 +65,14 @@ fast-simplification>=0.1.0 # System monitoring psutil>=6.0.0 +# IANA tz database for Windows. The stdlib ``zoneinfo`` module reads the +# system tz database on Linux/macOS, but Windows has none — and the +# embedded Python in our Windows installer doesn't carry one either, so +# even ``ZoneInfo("UTC")`` raises ``ZoneInfoNotFoundError`` and any +# endpoint that resolves a tz (e.g. /api/local-backup/status) 500s. +# ``tzdata`` is the official PyPI package that fills the gap. +tzdata>=2024.1; sys_platform == "win32" + # Authentication PyJWT>=2.13.0 passlib[bcrypt]>=1.7.4