Skip to content

Propagate errors instead of swallowing them in portal and installer scripts - #6

Open
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1787796815-error-handling
Open

devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1787796815-error-handling

Conversation

@devin-ai-integration

Copy link
Copy Markdown

Summary

Every entrypoint in this repo had at least one path where a failure produced no signal: the portal swallowed clipboard errors with catch{}, the PXE installer downgraded failed boot-file downloads to a WARN and exited 0, and the Proxmox installer hid --reapply failures behind >/dev/null 2>&1 || true. This makes those failures visible and fatal where the result would otherwise be silently broken.

brand.js

  • catch{} on the copy button → logs and shows Kopieren fehlgeschlagen; also detects the common real cause (navigator.clipboard unavailable over plain HTTP).
  • Manifest loading: 15s timeout via AbortSignal.timeout, Array.isArray(data.brands) validation, and the concrete reason (HTTP status / JSON error / timeout) is rendered into the error card instead of only console.error.
  • Rendering no longer throws on a missing DOM node or a malformed manifest entry (a missing #brand-grid used to abort loadManifest and surface as "manifest not available"); missing elements are reported once at load, and render failures fall back to the error card.

scripts/build-assets.py

  • Added BuildError plus a main() wrapper, so a broken brand fails with ERROR: asset build failed: [bnd.zone/corporate] Canonical SVG source missing: ... and exit 1, instead of a raw traceback from cairosvg or a KeyError: 'tagline'.
  • Validates brand.json (valid JSON, required brand/tagline), SVG source existence, accent hex format, and DejaVu font availability.

proxmox/install-webowie-proxmox-branding.sh

  • Assets are downloaded to a mktemp -d stage and only installed into $INSTALL once all six succeed; previously a mid-loop curl failure left $INSTALL half-overwritten, breaking the next --reapply.
  • [[ -e ... ]] && install ... as the last statement of the --reapply branch made the script exit 1 whenever dd_logo.png was absent → converted to if/fi.
  • The HTML injection is verified: no </head> used to mean sed silently did nothing and the install still reported success.
  • The apt post-invoke hook now appends to /var/log/webowie-pve-branding.log (it still can't fail dpkg) instead of discarding all output, and --reapply/--install check that staged assets exist and are non-empty.
  • Refuses to self-install $0 when piped from curl (it would install the shell, not the script), and reports a failed systemctl restart pveproxy.service.

scripts/create-netbootxyz-alpine-lxc.sh

  • CTID="${CTID:-$(pvesh get /cluster/nextid 2>/dev/null || echo 180)}" silently used CTID 180 when pvesh failed; now it fails with instructions.
  • Boot-file downloads are split into ESSENTIAL_FILES (UEFI + legacy BIOS entrypoints) and optional ones: missing essentials abort, missing optionals are summarized. Failed downloads no longer leave truncated files in the webroot.
  • pct start is followed by a wait for status: running and for pct exec readiness, rather than sleep 5 and a cryptic downstream failure.
  • Empty IP detection (host and container) is reported instead of writing HTTP: http:/// into /etc/motd; update-ca-certificates || true now warns.
  • Both scripts got an ERR trap that names the failing line and command.

Verified: python scripts/build-assets.py regenerates all assets byte-identically to main and error paths return BuildError; bash -n + shellcheck clean on both shell scripts (including the container heredoc checked as sh); node --check brand.js.

Link to Devin session: https://app.devin.ai/sessions/2249710876544504a25ff092b69ab79b
Requested by: @puchadave

…cripts

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration

Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant