VM silent: return the failure instead of exiting

.github/workflows/verify-split-files.yml

@@ -40,6 +40,7 @@ on:
       - "tests/incus-storage-space.sh"
       - "tests/release-stamp-pending.sh"
       - "tests/pct-start-reason.sh"
+      - "tests/vm-silent.sh"
       - ".github/workflows/verify-split-files.yml"
   push:
     branches: [main]
@@ -71,6 +72,7 @@ on:
       - "tests/incus-storage-space.sh"
       - "tests/release-stamp-pending.sh"
       - "tests/pct-start-reason.sh"
+      - "tests/vm-silent.sh"
       - ".github/workflows/verify-split-files.yml"
   workflow_dispatch:
 
@@ -143,6 +145,8 @@ jobs:
         run: bash tests/release-stamp-pending.sh
       - name: pct start reason tests
         run: bash tests/pct-start-reason.sh
+      - name: VM silent helper tests
+        run: bash tests/vm-silent.sh
 
       - name: Prefetch lists cover every engine file
         run: |

pve/vm-core.func

@@ -314,14 +314,17 @@ get_active_logfile() {
 # silent()
 #
 # - Executes command with output redirected to active log file
-# - On error: displays last 20 lines of log and exits with original exit code
-# - Temporarily disables error trap to capture exit code correctly
-# - Sources explain_exit_code() for detailed error messages
+# - On error: records the command, line and log, and returns its exit code;
+#   the ERR trap reports it. `$STD cmd || true` used to end the run instead.
+# - Temporarily disables error trap to capture exit code correctly, then
+#   restores the script's own trap
 # ------------------------------------------------------------------------------
 silent() {
   local cmd="$*"
   local caller_line="${BASH_LINENO[0]:-unknown}"
   local logfile="$(get_active_logfile)"
+  local err_trap
+  err_trap="$(trap -p ERR)"
 
   set +Eeuo pipefail
   trap - ERR
@@ -330,40 +333,33 @@ silent() {
   local rc=$?
 
   set -Eeuo pipefail
-  trap 'error_handler' ERR
+  eval "${err_trap:-trap - ERR}"
 
   if [[ $rc -ne 0 ]]; then
-    # Source explain_exit_code if needed
-    if ! declare -f explain_exit_code >/dev/null 2>&1; then
-      if declare -f _cs_source_func >/dev/null 2>&1; then
-        _cs_source_func "core/error_handler.func" 2>/dev/null || true
-      else
-        # shellcheck disable=SC1090
-        source <(curl -fsSL "${_CS_CURL_RETRY[@]}" "${COMMUNITY_SCRIPTS_CORE_URL:-https://raw.githubusercontent.com/community-scripts/core/main}/core/error_handler.func") 2>/dev/null || true
-      fi
-    fi
-
-    local explanation=""
-    if declare -f explain_exit_code >/dev/null 2>&1; then
-      explanation="$(explain_exit_code "$rc")"
-    fi
-
-    printf "\e[?25h"
-    if [[ -n "$explanation" ]]; then
-      msg_error "in line ${caller_line}: exit code ${rc} (${explanation})"
-    else
-      msg_error "in line ${caller_line}: exit code ${rc}"
-    fi
-    msg_custom "→" "${YWB}" "${cmd}"
+    export _SILENT_FAILED_RC="$rc" _SILENT_FAILED_CMD="$cmd" _SILENT_FAILED_LINE="$caller_line" _SILENT_FAILED_LOG="$logfile"
+    return "$rc"
+  fi
+  unset _SILENT_FAILED_RC _SILENT_FAILED_CMD _SILENT_FAILED_LINE _SILENT_FAILED_LOG
+}
 
-    if [[ -s "$logfile" ]]; then
-      echo -e "\n${TAB}--- Last 20 lines of log ---"
-      tail -n 20 "$logfile"
-      echo -e "${TAB}----------------------------\n"
-    fi
+# The record silent() left behind only counts for the line it came from: a
+# failure the caller handled must not be reported for a later, unrelated one.
+_vm_show_silent_failure() {
+  local explanation=""
+  if ! declare -f explain_exit_code >/dev/null 2>&1 && declare -f _cs_source_func >/dev/null 2>&1; then
+    _cs_source_func "api/exitcodes.func" 2>/dev/null || true
+  fi
+  declare -f explain_exit_code >/dev/null 2>&1 && explanation="$(explain_exit_code "$_SILENT_FAILED_RC")"
 
-    exit "$rc"
+  printf "\e[?25h"
+  msg_error "in line ${_SILENT_FAILED_LINE}: exit code ${_SILENT_FAILED_RC}${explanation:+ (${explanation})}"
+  msg_custom "→" "${YWB:-}" "${_SILENT_FAILED_CMD}"
+  if [[ -s "${_SILENT_FAILED_LOG:-}" ]]; then
+    echo -e "\n${TAB}--- Last 20 lines of log ---"
+    tail -n 20 "$_SILENT_FAILED_LOG"
+    echo -e "${TAB}----------------------------\n"
   fi
+  unset _SILENT_FAILED_RC _SILENT_FAILED_CMD _SILENT_FAILED_LINE _SILENT_FAILED_LOG
 }
 
 # ------------------------------------------------------------------------------
@@ -679,6 +675,11 @@ error_handler() {
   local line_number="${1:-unknown}"
   local command="${2:-unknown}"
 
+  if [[ -n "${_SILENT_FAILED_RC:-}" && "${_SILENT_FAILED_LINE:-}" == "$line_number" ]]; then
+    command="$_SILENT_FAILED_CMD"
+    _vm_show_silent_failure
+  fi
+
   if declare -f post_update_to_api >/dev/null 2>&1; then
     post_update_to_api "failed" "$exit_code"
   fi
@@ -2310,6 +2311,10 @@ vm_mark_created() {
 
 vm_post_create_error() {
   local rc=$? line="${1:-unknown}" cmd="${2:-unknown}"
+  if [[ -n "${_SILENT_FAILED_RC:-}" && "${_SILENT_FAILED_LINE:-}" == "$line" ]]; then
+    cmd="$_SILENT_FAILED_CMD"
+    _vm_show_silent_failure
+  fi
   msg_warn "A step after VM creation failed (line ${line}, exit ${rc}): ${cmd}"
   msg_warn "VM ${VMID:-?} is left in place -- finish the remaining configuration by hand."
   declare -f post_update_to_api >/dev/null 2>&1 && post_update_to_api "failed" "$rc"

tests/vm-silent.sh

@@ -0,0 +1,76 @@
+#!/usr/bin/env bash
+# The VM engine's silent() hands a failure back to the caller. It used to exit
+# the run itself, so `$STD cmd || true` ended every VM script whose host could
+# not run apt-get update, and the trap it re-armed had lost the script's line.
+set -uo pipefail
+
+COMMUNITY_SCRIPTS_CORE_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")/.." && pwd)"
+export COMMUNITY_SCRIPTS_CORE_DIR
+pveversion() { :; }
+# shellcheck disable=SC1091
+source "${COMMUNITY_SCRIPTS_CORE_DIR}/pve/vm-core.func"
+set +e -uo pipefail
+trap - ERR
+
+TEST_DIR="$(mktemp -d)"
+trap 'rm -rf -- "$TEST_DIR"' EXIT
+BUILD_LOG="$TEST_DIR/build.log"
+CL="" TAB="  " YWB="" RD="" YW=""
+VMID=""
+passed=0 failed=0
+
+msg_error() { echo "ERROR: $*"; }
+msg_custom() { echo "CMD: $3"; }
+msg_warn() { echo "WARN: $*"; }
+post_update_to_api() { echo "POST $1 $2"; }
+cleanup_vmid() { :; }
+explain_exit_code() { echo "explained $1"; }
+
+expect() {
+  if [[ "$2" == "$3" ]]; then
+    echo "PASS $1"
+    passed=$((passed + 1))
+  else
+    echo "FAIL $1: expected <$2>, got <$3>" >&2
+    failed=$((failed + 1))
+  fi
+}
+
+# A failure the script handles stays handled.
+out="$( (set -e; trap 'echo TRAPPED; exit 9' ERR; silent sh -c 'echo boom >&2; exit 100' || true; echo "carried on") 2>&1 )"
+expect "a handled failure does not end the run" 1 "$(grep -c 'carried on' <<<"$out")"
+expect "the ERR trap stays quiet for it" 0 "$(grep -c TRAPPED <<<"$out")"
+expect "nothing is reported for it" 0 "$(grep -c 'ERROR:' <<<"$out")"
+
+# An unhandled one reaches the script's own trap with the command's code.
+: >"$BUILD_LOG"
+out="$( (set -e; trap 'error_handler $LINENO "$BASH_COMMAND"' ERR; silent sh -c 'echo boom >&2; exit 100'; echo "never") 2>&1 )"
+rc=$?
+expect "the run ends with the command's code" 100 "$rc"
+expect "the handler posts that code" 1 "$(grep -c 'POST failed 100' <<<"$out")"
+expect "the command is the real one, not the wrapper" 1 "$(grep -c '^CMD: sh -c echo boom' <<<"$out")"
+expect "the explanation is shown" 1 "$(grep -c 'exit code 100 (explained 100)' <<<"$out")"
+expect "the log tail is shown" 1 "$(grep -c '^boom$' <<<"$out")"
+expect "nothing runs after it" 0 "$(grep -c never <<<"$out")"
+
+# The script's trap survives the call.
+out="$( (trap 'echo MINE' ERR; silent true; trap -p ERR) )"
+expect "the script's ERR trap is kept" 1 "$(grep -c MINE <<<"$out")"
+
+# A handled failure is not pinned on the next, unrelated one.
+out="$( (set -e; trap 'error_handler $LINENO "$BASH_COMMAND"' ERR; silent sh -c 'exit 100' || true
+  false; echo "never") 2>&1 )"
+rc=$?
+expect "the later failure keeps its own code" 1 "$rc"
+expect "the handled command is not blamed" 0 "$(grep -c 'CMD:' <<<"$out")"
+expect "the later command is named" 1 "$(grep -c 'while executing command .*false' <<<"$out")"
+
+# The post-creation trap reports the same way.
+out="$( (set -e; VM_CREATED=1; trap 'vm_post_create_error $LINENO "$BASH_COMMAND"' ERR; silent sh -c 'echo late >&2; exit 3') 2>&1 )"
+rc=$?
+expect "a failure after creation keeps the code" 3 "$rc"
+expect "and names the real command" 1 "$(grep -c '^CMD: sh -c echo late' <<<"$out")"
+expect "and leaves the VM in place" 1 "$(grep -c 'left in place' <<<"$out")"
+
+echo "$passed passed, $failed failed"
+((failed == 0))