[Detail Bug] KNX GUI: Loading malformed .knxprod crashes the app instead of showing an in-app import error
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 4
- Forks
- 0
- Avg merge
- 15h 38m
- Merged PRs (30d)
- 37
Description
Detail Bug Report
Introduced in e066b2be9f21453920a1851fe9853334e1a3f96b by @kewde on May 9, 2026
Summary
- Context:
KnxGuiApp._load_knxprod(apps/knx-gui/src/knx_gui/main.py) is invoked from the File → "Load knxprod…" menu when a user selects a.knxprodfile; it registers a background task onTaskServiceand imports the file into the catalog. - Bug:
_load_knxprodalready has exception handlers that convert import failures into recoverable in-app error badges + logged error lines —except ArchiveErrorandexcept (OSError, ValueError)each callself._log.error(...)andself._task_service.update(task_id, status="error", ...). The code's own intent, expressed by these existing handlers, is that a failed import recovers within the GUI rather than crashing. The bug is that this recovery is incomplete:CatalogService.import_knxprodcan also raisexknxmono.models.schema.VersionError,zipfile.BadZipFile, andxknxmono.product.errors.ParseError— all confirmed siblings ofArchiveError/OSError/ValueError, not subclasses — so none are caught, and the exception propagates out of_load_knxprod→gui_menu→callbacks.show_menus→hello_imgui.run(), which re-raises and terminates the GUI process (verified end-to-end underxvfb, Evidence §2; reproducer scripts included in Appendix A). - Actual vs. expected: Expected — a failed import produces a red
ERRORlog row in the Logger panel and an error status-bar badge, exactly as the existing handlers already do forArchiveError/OSError/ValueError. Actual — loading a malformed.knxprodvia the File menu crashes the GUI application instead. - Impact: The harm is a UX regression: loading a malformed
.knxprodcrashes the GUI application (process exits) where the codebase's own existing handlers for this exact call (except ArchiveError,except (OSError, ValueError)) already establish the pattern of recovering with an in-app error badge + logged error line.
Code with Bug
def _load_knxprod(self, path: str) -> None:
self._log.info("loading knxprod", path=path)
task_id = self._task_service.add(f"Loading {Path(path).name}")
try:
added = self._catalog_service.import_knxprod(Path(path))
if added:
self._log.info("added applications to catalog", count=len(added))
else:
self._log.info("no new applications", path=path)
self._task_service.remove(task_id)
except ArchiveError as e:
self._log.error("archive error", path=path, error=str(e))
self._task_service.update(task_id, status="error", detail=str(e))
except (OSError, ValueError) as e:
self._log.error("import error", path=path, error=f"{type(e).__name__}: {e}")
self._task_service.update(task_id, status="error", detail=str(e))
# <-- BUG 🔴 VersionError/BadZipFile/ParseError are not caught, so they escape and crash the GUI
Explanation
CatalogService.import_knxprodcalls intoxknxmonoparsing code that can raisexknxmono.models.schema.VersionError(version detection fails on structurally-valid-but-invalid XML),zipfile.BadZipFile(CRC mismatch when reading a ZIP member), andxknxmono.product.errors.ParseError(semantic XML validation failure). These are notArchiveError,OSError, orValueErrorsubclasses, so they are not handled.- In the resolved
imgui-bundleversion (1.92.700), an exception raised from the menu callback path is re-raised byhello_imgui.run(), so the process terminates rather than continuing the GUI loop. - Because the
tryblock has nofinally, an uncaught exception also skips bothTaskService.remove()andTaskService.update(), leaving the task instatus="running"until the process exits.
Recommended Fix
Add a narrow handler for the three demonstrated escaping exception types (do not broaden to except Exception, since the import path also does DB and filesystem work):
import zipfile
from xknxmono.models.schema import VersionError
from xknxmono.product.errors import ArchiveError, ParseError
...
except ArchiveError as e:
self._log.error("archive error", path=path, error=str(e))
self._task_service.update(task_id, status="error", detail=str(e))
except (OSError, ValueError) as e:
self._log.error("import error", path=path, error=f"{type(e).__name__}: {e}")
self._task_service.update(task_id, status="error", detail=str(e))
except (VersionError, ParseError, zipfile.BadZipFile) as e:
self._log.error("import error", path=path, error=f"{type(e).__name__}: {e}")
self._task_service.update(task_id, status="error", detail=str(e))
History
This bug was introduced in commit e066b2b. That commit ("feat(knx-gui): add menu entry to load .knxprod files with stub popup", Sat May 9 2026) stubbed the initially-stub _load_knxprod — opening a .knxprod file and rendering a popup with its contents — and chose exactly except ArchiveError / except (OSError, ValueError) to cover the failure modes of the then-xknx ProductArchive API; the bug slipped in because that handler set never accounted for VersionError (from version detection on a structurally-valid-but-corrupt archive), the product.errors.ParseError (distinct from the caught xsdata.exceptions.ParserError), or zipfile.BadZipFile (from CRC mismatch on a member that passes the namelist check), so when the import path was later rewritten onto xknxmono.catalog/xknxmono.product the same narrow shape was carried forward unchanged and remains the missing-from-the-handler-set bug today. (git blame points to 480c139 on the handler lines, but that commit is a mechanical chore: ruff knx gui mass-reformat that preserves the logic verbatim; PR #36 / ac577ad later added only the task_id lifecycle wiring on top, mirroring the existing narrow handler shape rather than widening it.)
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in apps/knx-gui/src/knx_gui/main.py at KnxGuiApp._load_knxprod and review its existing exception handlers and TaskService updates. Reproduce the malformed .knxprod cases described in Appendix A, then verify that VersionError, ParseError, and BadZipFile produce the existing error log and status-bar badge without terminating the GUI.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- desktop
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 88/100