fix(installer): graceful error boundary — PowerShell never crashes with a raw stack (#577) - #588
Merged
Merged
Conversation
…with a stack (#577) The main installer (install-k8s.ps1) had NO top-level try/catch, so any unhandled throw (or PS runtime error) rendered PowerShell's raw source line + stack and terminated the session — exactly Anubha's screen. Wrap the whole main run (inside the TB_PESTER guard) in a top-level try/catch that routes any crash through a new Show-FatalError: one clean "Installation stopped" line + the reason (the exception MESSAGE, curated at the throw sites per #576 — never the stack) + the log location + a safe-to-re-run hint, then exit 1. Intentional exits (fast-path, Err, final) pass straight through — verified exit-in-try is not caught while throw-in-try is. Stack traces are never shown OR logged (no internals). Bootstrap (install.ps1) already had a top-level try/catch; enhance its message to the same clean "Installation stopped … safe to re-run" shape. +3 Pester (main wrapped in try/catch -> Show-FatalError; clean render with reason + re-run hint and no stack/source; reason logged, stack never). Manifest regenerated. Built on #576 (needs its no-transcript / curated Log world); PR opens once #576 lands so it can target a clean develop. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…577) Completes the PowerShell side of #577 to Lukas's acceptance. The top-level boundary now has a guaranteed `finally` + a last-resort `trap`, mirroring bash's exit-code-guarded install_cleanup: - $script:OutcomeReported is set on every terminal path (normal finish, Err, caught crash via Show-FatalError, fast-path, help/diagnose). - The finally shows a clean "interrupted" line (Show-Interrupted: log + re-run, no stack) ONLY when nothing reported an outcome — i.e. Ctrl-C / abnormal termination — so the window never just vanishes. - The trap catches anything that terminates OUTSIDE the try (defined inside the TB_PESTER guard so it never fires under the test dot-source). Verified: reported exits stay quiet, crashes route through Show-FatalError, an unreported exit fires Show-Interrupted; exit codes preserved. +3 Pester. Manifest regenerated. Bash symmetry (capture a failing tool's raw stderr -> curated line before the closer) is next. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…derr (#577) Completes #577's bash symmetry. The big steps already curate (spin_cmd_bounded captures tool output to the log + shows a clean ✖ line) and install_cleanup is a clean conditional closer; the residual raw-stderr-before-the-closer leaks were a few DIRECT native calls under `set -euo pipefail`: - cluster.sh: `k3d cluster start` (reuse path) now redirects to the log + surfaces a curated error() on failure. - gpu-plugins.sh: `kubectl apply` (device-plugin manifest) and `kubectl rollout status` now redirect to the log, so only the caller's curated error/warn shows. A failing tool now surfaces our plain-language line, not its raw stderr, before the closer — bash graceful *failure*, not just a graceful closer. Manifest regenerated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…577) Per #577's fatal-vs-recoverable acceptance (optional steps warn + continue): a GPU device-plugin download/apply failure was fatal on BOTH platforms — gpu-plugins.sh `error "Failed to enable GPU acceleration"` and install-k8s.ps1 `Err "Failed to enable GPU acceleration"` both exit — so a GPU hiccup aborted the whole install instead of falling back to CPU mode (which the client fully supports). Now both warn + continue in CPU mode: - bash: `_apply_remote_manifest` failure -> warn + return 0. - PS: wrap the download/apply in a catch so a failure can't reach the top-level boundary; warn + continue; also capture `kubectl apply` raw stderr (#577). Mirrors the NVIDIA-container-toolkit timeout, which already warns and carries on. +1 Pester. Manifest regenerated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Bugbot (High): the GPU device-plugin apply/rollout discarded output into $null and never checked $LASTEXITCODE. Because a native kubectl non-zero exit does not throw, a failed apply fell through to Ok "GPU acceleration enabled." - a false success the operator would trust, and (unlike bash) the failure was neither warned nor logged. Capture each kubectl call's output to the install log and gate the success message on $LASTEXITCODE; on any non-zero exit, warn + continue in CPU mode (mirrors the bash gpu-plugins.sh path). Adds a regression test asserting the success message is exit-code-gated and the false-success $null-discard pattern is gone. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Bugbot (Medium): the reboot-pending path prints reboot/resume guidance and exits 2, but never set $script:OutcomeReported. The new top-level finally then treated that intentional stop as an interruption and appended Show-Interrupted, so on a common Step 1 path the operator saw a contradictory second outcome. Set $script:OutcomeReported before the reboot block's exits (covers both exit 2 paths and the Restart-Computer path), so the finally leaves the clean reboot guidance as the sole outcome. Adds a regression test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…an't hang (#577) Bugbot flagged two hang risks introduced by the #577 log redirects: piping a call's output to the log hides console progress, so an unbounded call now hangs silently instead of failing into the new curated error/warn paths. - cluster.sh: `k3d cluster start` waits for the server with no deadline by default; add `--wait --timeout 5m` (parity with the Windows installer's 5-minute start deadline) so a wedged Docker fails into the curated error instead of hanging. - gpu-plugins.sh: the shared `_apply_remote_manifest` (nvidia + amd) now bounds `kubectl apply` with `--request-timeout=30s`, mirroring the node-probe in the same file, so a wedged API server falls through to the recoverable CPU-mode warn. - install-k8s.ps1: same `--request-timeout=30s` on the PS GPU apply for parity (the PS `k3d cluster start` is already bounded by Wait-ProcessWithDeadline). Adds regression tests: cluster start is --timeout-bounded (bats), the GPU apply carries --request-timeout (bats + Pester). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…rrupt gap (#577) Bugbot round 4: - gpu-plugins.sh (Medium): after a non-zero `kubectl rollout status`, bash still printed success "GPU acceleration enabled." while the failure went only to the log - the false-success bug already fixed on the PS side, still present in bash. Gate the success on the rollout exit code (nvidia AND amd paths); on failure warn + continue in CPU mode, matching the PS peer. - install-k8s.ps1 (Low): $script:OutcomeReported was set to $true BEFORE the long Invoke-DiagnoseBundle ran, so an interrupt mid-collection skipped Show-Interrupted - the silent death the boundary exists to prevent. Set the flag AFTER the bundle completes. Adds regression tests: nvidia success is rollout-gated (bats); the -Diagnose flag is set only after the bundle (Pester). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 20ca276. Configure here.
…ode (#577) Bugbot round 5 (Medium): making the GPU device-plugin step recoverable (#577) means a failed apply/download now continues the install - but the caller still ran the GPU verify step unconditionally. Operators saw "continuing in CPU mode", then waited ~90s and got a contradictory "still initializing" warning for a plugin never deployed. Have the deploy signal whether the plugin was actually deployed, and gate verify on it: - gpu-plugins.sh: _deploy_nvidia_plugin / _deploy_amd_plugin return non-zero on every CPU-mode path (apply failure, unconfirmed rollout, master-fallback failure); 0 when deployed/already-present. - install-k8s.sh: `if deploy_gpu_device_plugin; then verify_gpu; fi` (the `if` also keeps a non-zero deploy from tripping set -e). - install-k8s.ps1: Install-GpuDevicePlugin returns $true/$false (Invoke-WithRetry sunk to $null to avoid return-value pollution); caller runs Confirm-GpuNode only on $true. Adds regression tests: verify is gated (bats + Pester), and a functional bats test that _deploy_nvidia_plugin returns non-zero on a CPU-mode path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
saadqbal
previously approved these changes
Aug 4, 2026
saadqbal
left a comment
saadqbal
left a comment
Contributor
There was a problem hiding this comment.
Careful PR — the try/catch/finally boundary, the OutcomeReported bookkeeping (every terminal path sets it, incl. reboot-pending and post-diagnose), and the GPU fatal→recoverable reclassification all check out. Two small optional follow-ups inline; neither blocks. 👍
…er fallback (#577) Review follow-ups (saadqbal) on the GPU path: - Bound the last unbounded kubectl probes so a wedged API server can't hang before the bounded apply is reached: the nvidia/amd existence checks (bash + PS) and PS Confirm-GpuNode's node probe now carry --request-timeout=5s, matching bash's verify_gpu. Closes the "can't hang" goal symmetrically. - Gate the amd master fallback on rollout like the primary paths: a master apply that never rolls out now warns + continues in CPU mode instead of returning a false success that made the caller's verify poll ~90s. - Extract the shared _gpu_rollout_gate helper (nvidia, amd-primary, amd-master) so the "no false enabled / no dead verify wait" behaviour is identical everywhere and not duplicated three times. Tests: adds _gpu_rollout_gate functional tests (rollout fail -> warn+non-zero; ok -> success+0), asserts the existence probes are --request-timeout-bounded, that both amd paths gate on rollout, and that the PS probes carry --request-timeout. Updates the in-branch nvidia-gating test to the refactored (helper-delegated) form. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
saadqbal
approved these changes
Aug 4, 2026
LukasWodka
added a commit
that referenced
this pull request
Aug 4, 2026
Resolves the install-k8s.ps1 + manifest.sha256 conflict from #588 (graceful error boundary). Re-applies the two -Encoding UTF8 edits to the log writers on top of develop and regenerates the R8 manifest hash.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Closes #577.
When the install failed, the PowerShell installer could terminate ungracefully — a raw
terminating-error dump (stack, source line,
NativeCommandError) instead of a clean"here's what happened and what to do next." A network hiccup surfaced as a crash. This
PR makes the installer always end with a branded, human-readable outcome on every
path — success, handled failure, unexpected crash, and interruption — and brings the
bash installer to the same graceful-failure bar.
Changes
Top-level error boundary (PowerShell) —
install-k8s.ps1,install.ps1mainruns insidetry { … } catch { Show-FatalError $_; exit 1 } finally { … }plus atrap, so nothrowor unhandled native-command error can ever reach the console as araw stack. The
catchrenders one clean block: what failed, the exact next step, andwhere the log is.
finallyreports the outcome even when the run is interrupted (Ctrl-C) ordies abnormally. A
$script:OutcomeReportedflag is set on every terminal path so theuser sees exactly one outcome — never zero (silent death) and never a duplicate.
Show-FatalError/Show-Interruptedrender the boundary messages; both are ASCII-onlysource (glyphs via
[char]0xNNNN, per the installer(ps1): cosign bootstrap is silent for minutes — looks frozen; users kill it #468 hygiene guard).Fatal vs recoverable classification
continues in CPU mode instead of aborting the whole install (both platforms). Its
kubectl applystderr is captured to the log rather than thrown.bash symmetry
cluster.sh:k3d cluster startredirects native output to the log and fails with aclean
errormessage instead of leaking raw stderr.gpu-plugins.sh: the NVIDIA plugin apply/rollout redirect to the log, and a failurewarns + returns 0 (GPU is recoverable) rather than aborting.
Tests
install-k8s.Tests.ps1Describes cover: the boundary reports an outcome exactlyonce,
Show-FatalError/Show-Interruptedrender cleanly, and the GPU step isnon-fatal on failure.
(check-facts, check-style), manifest
--check, and PS parse all clean.Notes
install-k8s.ps1is manifested;scripts/manifest.sha256is regenerated in this PR.(Installer must complete on restricted/corporate networks (proxy, TLS-inspection, blocked hosts) — handle network conditions internally #578 and children). This is the substrate those rely on — it converts a hard failure
into a diagnosis instead of a crash.
Note
Medium Risk
Touches the main install control flow and cluster/GPU bootstrap on all platforms; mis-set
OutcomeReportedor exit-code gating could hide real failures or skip GPU verification incorrectly.Overview
PowerShell installers (
install-k8s.ps1,install.ps1) wrap the main run intry/catch/finallyplus atrap, using$script:OutcomeReportedso users get exactly one terminal message:Show-FatalErrorfor unhandled crashes (message only, no stack),Show-Interruptedwhen nothing else reported (e.g. Ctrl-C), with the same flag onErr, help/diagnose, fast-path exit, reboot-pending, and successful summary.GPU setup is treated as optional on both platforms: failed download/apply/rollout warns and continues in CPU mode instead of aborting; deploy functions return success/failure so
verify_gpu/Confirm-GpuNoderun only after a successful deploy, avoiding a long poll when no plugin was applied. Apply/rollout and existence probes usekubectl --request-timeout; Windows gates “GPU acceleration enabled” on$LASTEXITCODEand logs raw kubectl output.Bash (
cluster.sh,gpu-plugins.sh,install-k8s.sh) mirrors this: boundedk3d cluster start --wait --timeout 5mwith curated errors, shared_gpu_rollout_gate, non-fatal NVIDIA/AMD plugin paths, and conditionalverify_gpu. Manifest hashes and new Pester/Bats guards cover the boundary and GPU behavior.Reviewed by Cursor Bugbot for commit 62b2162. Bugbot is set up for automated code reviews on this repo. Configure here.