fix(api): handle missing RAM speed in nested VMs

- Add || true to dmidecode pipelines to prevent error when speed is 'Unknown'
- Validate RAM_SPEED is a valid integer, fallback to 0
- Send ram_speed as integer (not string) in all JSON payloads for PocketBase

Fixes: dmidecode in nested VMs returns 'Configured Memory Speed: Unknown'
which causes grep to fail and triggers catch_errors handler.

CHANGELOG.md

@@ -407,6 +407,8 @@ Exercise vigilance regarding copycat or coat-tailing sites that seek to exploit
 
   - #### ✨ New Features
 
+    - core: validate container IDs cluster-wide across all nodes [@MickLesk](https://github.com/MickLesk) ([#11906](https://github.com/community-scripts/ProxmoxVE/pull/11906))
+    - core: improve error reporting with structured error strings and better categorization + output formatting [@MickLesk](https://github.com/MickLesk) ([#11907](https://github.com/community-scripts/ProxmoxVE/pull/11907))
     - core: unified logging system with combined logs [@MickLesk](https://github.com/MickLesk) ([#11761](https://github.com/community-scripts/ProxmoxVE/pull/11761))
 
 ### ❔ Uncategorized

misc/api.func

@@ -287,6 +287,32 @@ get_error_text() {
   fi
 }
 
+# ------------------------------------------------------------------------------
+# build_error_string()
+#
+# - Builds a structured error string for telemetry reporting
+# - Format: "exit_code=<N> | <explanation>\n---\n<last 20 log lines>"
+# - If no log lines available, returns just the explanation
+# - Arguments:
+#   * $1: exit_code (numeric)
+#   * $2: log_text (optional, output from get_error_text)
+# - Returns structured error string via stdout
+# ------------------------------------------------------------------------------
+build_error_string() {
+  local exit_code="${1:-1}"
+  local log_text="${2:-}"
+  local explanation
+  explanation=$(explain_exit_code "$exit_code")
+
+  if [[ -n "$log_text" ]]; then
+    # Structured format: header + separator + log lines
+    printf 'exit_code=%s | %s\n---\n%s' "$exit_code" "$explanation" "$log_text"
+  else
+    # No log available - just the explanation with exit code
+    printf 'exit_code=%s | %s' "$exit_code" "$explanation"
+  fi
+}
+
 # ==============================================================================
 # SECTION 2: TELEMETRY FUNCTIONS
 # ==============================================================================
@@ -376,14 +402,19 @@ detect_ram() {
 
   if command -v dmidecode &>/dev/null; then
     # Get configured memory speed (actual running speed)
-    RAM_SPEED=$(dmidecode -t memory 2>/dev/null | grep -m1 "Configured Memory Speed:" | grep -oE "[0-9]+" | head -1)
+    RAM_SPEED=$(dmidecode -t memory 2>/dev/null | grep -m1 "Configured Memory Speed:" | grep -oE "[0-9]+" | head -1 || true)
 
     # Fallback to Speed: if Configured not available
     if [[ -z "$RAM_SPEED" ]]; then
-      RAM_SPEED=$(dmidecode -t memory 2>/dev/null | grep -m1 "Speed:" | grep -oE "[0-9]+" | head -1)
+      RAM_SPEED=$(dmidecode -t memory 2>/dev/null | grep -m1 "Speed:" | grep -oE "[0-9]+" | head -1 || true)
     fi
   fi
 
+  # Ensure RAM_SPEED is a valid integer (PocketBase stores it as integer)
+  if [[ -z "$RAM_SPEED" || ! "$RAM_SPEED" =~ ^[0-9]+$ ]]; then
+    RAM_SPEED=0
+  fi
+
   export RAM_SPEED
 }
 
@@ -478,7 +509,7 @@ post_to_api() {
     "gpu_vendor": "${gpu_vendor}",
     "gpu_model": "${gpu_model}",
     "gpu_passthrough": "${gpu_passthrough}",
-    "ram_speed": "${ram_speed}",
+    "ram_speed": ${ram_speed:-0},
     "repo_source": "${REPO_SOURCE}"
 }
 EOF
@@ -582,7 +613,7 @@ post_to_api_vm() {
     "gpu_vendor": "${gpu_vendor}",
     "gpu_model": "${gpu_model}",
     "gpu_passthrough": "${gpu_passthrough}",
-    "ram_speed": "${ram_speed}",
+    "ram_speed": ${ram_speed:-0},
     "repo_source": "${REPO_SOURCE}"
 }
 EOF
@@ -665,13 +696,12 @@ post_update_to_api() {
     else
       exit_code=1
     fi
+    # Get log lines and build structured error string
     local error_text=""
     error_text=$(get_error_text)
-    if [[ -n "$error_text" ]]; then
-      error=$(json_escape "$error_text")
-    else
-      error=$(json_escape "$(explain_exit_code "$exit_code")")
-    fi
+    local full_error
+    full_error=$(build_error_string "$exit_code" "$error_text")
+    error=$(json_escape "$full_error")
     short_error=$(json_escape "$(explain_exit_code "$exit_code")")
     error_category=$(categorize_error "$exit_code")
     [[ -z "$error" ]] && error="Unknown error"
@@ -717,7 +747,7 @@ post_update_to_api() {
     "gpu_vendor": "${gpu_vendor}",
     "gpu_model": "${gpu_model}",
     "gpu_passthrough": "${gpu_passthrough}",
-    "ram_speed": "${ram_speed}",
+    "ram_speed": ${ram_speed:-0},
     "repo_source": "${REPO_SOURCE}"
 }
 EOF
@@ -759,7 +789,7 @@ EOF
     "gpu_vendor": "${gpu_vendor}",
     "gpu_model": "${gpu_model}",
     "gpu_passthrough": "${gpu_passthrough}",
-    "ram_speed": "${ram_speed}",
+    "ram_speed": ${ram_speed:-0},
     "repo_source": "${REPO_SOURCE}"
 }
 EOF
@@ -814,31 +844,52 @@ EOF
 categorize_error() {
   local code="$1"
   case "$code" in
-  # Network errors
-  6 | 7 | 22 | 28 | 35) echo "network" ;;
+  # Network errors (curl/wget)
+  6 | 7 | 22 | 35) echo "network" ;;
 
-  # Storage errors
-  214 | 217 | 219) echo "storage" ;;
+  # Timeout errors
+  28 | 124 | 211) echo "timeout" ;;
 
-  # Dependency/Package errors
-  100 | 101 | 102 | 127 | 160 | 161 | 162) echo "dependency" ;;
+  # Storage errors (Proxmox storage)
+  214 | 217 | 219 | 224) echo "storage" ;;
+
+  # Dependency/Package errors (APT, DPKG, pip, commands)
+  100 | 101 | 102 | 127 | 160 | 161 | 162 | 255) echo "dependency" ;;
 
   # Permission errors
   126 | 152) echo "permission" ;;
 
-  # Timeout errors
-  124 | 28 | 211) echo "timeout" ;;
+  # Configuration errors (Proxmox config, invalid args)
+  128 | 203 | 204 | 205 | 206 | 207 | 208) echo "config" ;;
 
-  # Configuration errors
-  203 | 204 | 205 | 206 | 207 | 208) echo "config" ;;
+  # Proxmox container/template errors
+  200 | 209 | 210 | 212 | 213 | 215 | 216 | 218 | 220 | 221 | 222 | 223 | 225 | 231) echo "proxmox" ;;
+
+  # Service/Systemd errors
+  150 | 151 | 153 | 154) echo "service" ;;
+
+  # Database errors (PostgreSQL, MySQL, MongoDB)
+  170 | 171 | 172 | 173 | 180 | 181 | 182 | 183 | 190 | 191 | 192 | 193) echo "database" ;;
+
+  # Node.js / JavaScript runtime errors
+  243 | 245 | 246 | 247 | 248 | 249) echo "runtime" ;;
+
+  # Python environment errors
+  # (already covered: 160-162 under dependency)
 
   # Aborted by user
   130) echo "aborted" ;;
 
-  # Resource errors (OOM, etc)
-  137 | 134) echo "resource" ;;
+  # Resource errors (OOM, SIGKILL, SIGABRT)
+  134 | 137) echo "resource" ;;
 
-  # Default
+  # Signal/Process errors (SIGTERM, SIGPIPE, SIGSEGV)
+  139 | 141 | 143) echo "signal" ;;
+
+  # Shell errors (general error, syntax error)
+  1 | 2) echo "shell" ;;
+
+  # Default - truly unknown
   *) echo "unknown" ;;
   esac
 }
@@ -901,11 +952,9 @@ post_tool_to_api() {
     [[ ! "$exit_code" =~ ^[0-9]+$ ]] && exit_code=1
     local error_text=""
     error_text=$(get_error_text)
-    if [[ -n "$error_text" ]]; then
-      error=$(json_escape "$error_text")
-    else
-      error=$(json_escape "$(explain_exit_code "$exit_code")")
-    fi
+    local full_error
+    full_error=$(build_error_string "$exit_code" "$error_text")
+    error=$(json_escape "$full_error")
     error_category=$(categorize_error "$exit_code")
   fi
 
@@ -968,11 +1017,9 @@ post_addon_to_api() {
     [[ ! "$exit_code" =~ ^[0-9]+$ ]] && exit_code=1
     local error_text=""
     error_text=$(get_error_text)
-    if [[ -n "$error_text" ]]; then
-      error=$(json_escape "$error_text")
-    else
-      error=$(json_escape "$(explain_exit_code "$exit_code")")
-    fi
+    local full_error
+    full_error=$(build_error_string "$exit_code" "$error_text")
+    error=$(json_escape "$full_error")
     error_category=$(categorize_error "$exit_code")
   fi
 
@@ -1067,11 +1114,9 @@ post_update_to_api_extended() {
     fi
     local error_text=""
     error_text=$(get_error_text)
-    if [[ -n "$error_text" ]]; then
-      error=$(json_escape "$error_text")
-    else
-      error=$(json_escape "$(explain_exit_code "$exit_code")")
-    fi
+    local full_error
+    full_error=$(build_error_string "$exit_code" "$error_text")
+    error=$(json_escape "$full_error")
     error_category=$(categorize_error "$exit_code")
     [[ -z "$error" ]] && error="Unknown error"
   fi

misc/build.func

@@ -277,8 +277,9 @@ install_ssh_keys_into_ct() {
 # ------------------------------------------------------------------------------
 # validate_container_id()
 #
-# - Validates if a container ID is available for use
-# - Checks if ID is already used by VM or LXC container
+# - Validates if a container ID is available for use (CLUSTER-WIDE)
+# - Checks cluster resources via pvesh for VMs/CTs on ALL nodes
+# - Falls back to local config file check if pvesh unavailable
 # - Checks if ID is used in LVM logical volumes
 # - Returns 0 if ID is available, 1 if already in use
 # ------------------------------------------------------------------------------
@@ -290,11 +291,35 @@ validate_container_id() {
     return 1
   fi
 
-  # Check if config file exists for VM or LXC
+  # CLUSTER-WIDE CHECK: Query all VMs/CTs across all nodes
+  # This catches IDs used on other nodes in the cluster
+  # NOTE: Works on single-node too - Proxmox always has internal cluster structure
+  # Falls back gracefully if pvesh unavailable or returns empty
+  if command -v pvesh &>/dev/null; then
+    local cluster_ids
+    cluster_ids=$(pvesh get /cluster/resources --type vm --output-format json 2>/dev/null | 
+      grep -oP '"vmid":\s*\K[0-9]+' 2>/dev/null || true)
+    if [[ -n "$cluster_ids" ]] && echo "$cluster_ids" | grep -qw "$ctid"; then
+      return 1
+    fi
+  fi
+
+  # LOCAL FALLBACK: Check if config file exists for VM or LXC
+  # This handles edge cases where pvesh might not return all info
   if [[ -f "/etc/pve/qemu-server/${ctid}.conf" ]] || [[ -f "/etc/pve/lxc/${ctid}.conf" ]]; then
     return 1
   fi
 
+  # Check ALL nodes in cluster for config files (handles pmxcfs sync delays)
+  # NOTE: On single-node, /etc/pve/nodes/ contains just the one node - still works
+  if [[ -d "/etc/pve/nodes" ]]; then
+    for node_dir in /etc/pve/nodes/*/; do
+      if [[ -f "${node_dir}qemu-server/${ctid}.conf" ]] || [[ -f "${node_dir}lxc/${ctid}.conf" ]]; then
+        return 1
+      fi
+    done
+  fi
+
   # Check if ID is used in LVM logical volumes
   if lvs --noheadings -o lv_name 2>/dev/null | grep -qE "(^|[-_])${ctid}($|[-_])"; then
     return 1
@@ -306,63 +331,30 @@ validate_container_id() {
 # ------------------------------------------------------------------------------
 # get_valid_container_id()
 #
-# - Returns a valid, unused container ID
+# - Returns a valid, unused container ID (CLUSTER-AWARE)
+# - Uses pvesh /cluster/nextid as starting point (already cluster-aware)
 # - If provided ID is valid, returns it
-# - Otherwise increments from suggested ID until a free one is found
+# - Otherwise increments until a free one is found across entire cluster
 # - Calls validate_container_id() to check availability
 # ------------------------------------------------------------------------------
 get_valid_container_id() {
-  local suggested_id="${1:-$(pvesh get /cluster/nextid)}"
-
-  while ! validate_container_id "$suggested_id"; do
-    suggested_id=$((suggested_id + 1))
-  done
-
-  echo "$suggested_id"
-}
-
-# ------------------------------------------------------------------------------
-# validate_container_id()
-#
-# - Validates if a container ID is available for use
-# - Checks if ID is already used by VM or LXC container
-# - Checks if ID is used in LVM logical volumes
-# - Returns 0 if ID is available, 1 if already in use
-# ------------------------------------------------------------------------------
-validate_container_id() {
-  local ctid="$1"
-
-  # Check if ID is numeric
-  if ! [[ "$ctid" =~ ^[0-9]+$ ]]; then
-    return 1
-  fi
-
-  # Check if config file exists for VM or LXC
-  if [[ -f "/etc/pve/qemu-server/${ctid}.conf" ]] || [[ -f "/etc/pve/lxc/${ctid}.conf" ]]; then
-    return 1
-  fi
-
-  # Check if ID is used in LVM logical volumes
-  if lvs --noheadings -o lv_name 2>/dev/null | grep -qE "(^|[-_])${ctid}($|[-_])"; then
-    return 1
-  fi
-
-  return 0
-}
-
-# ------------------------------------------------------------------------------
-# get_valid_container_id()
-#
-# - Returns a valid, unused container ID
-# - If provided ID is valid, returns it
-# - Otherwise increments from suggested ID until a free one is found
-# - Calls validate_container_id() to check availability
-# ------------------------------------------------------------------------------
-get_valid_container_id() {
-  local suggested_id="${1:-$(pvesh get /cluster/nextid)}"
+  local suggested_id="${1:-$(pvesh get /cluster/nextid 2>/dev/null || echo 100)}"
+
+  # Ensure we have a valid starting ID
+  if ! [[ "$suggested_id" =~ ^[0-9]+$ ]]; then
+    suggested_id=$(pvesh get /cluster/nextid 2>/dev/null || echo 100)
+  fi
+
+  local max_attempts=1000
+  local attempts=0
 
   while ! validate_container_id "$suggested_id"; do
     suggested_id=$((suggested_id + 1))
+    attempts=$((attempts + 1))
+    if [[ $attempts -ge $max_attempts ]]; then
+      msg_error "Could not find available container ID after $max_attempts attempts"
+      exit 1
+    fi
   done
 
   echo "$suggested_id"
@@ -4133,8 +4125,7 @@ EOF'
 
     # Show combined log location
     if [[ -n "$CTID" && -n "${SESSION_ID:-}" ]]; then
-      echo ""
-      echo -e "${GN}✔${CL} Installation log: ${BL}${combined_log}${CL}"
+      msg_custom "📋" "${YW}" "Installation log: ${combined_log}"
     fi
 
     # Dev mode: Keep container or open breakpoint shell
@@ -4157,19 +4148,21 @@ EOF'
       exit $install_exit_code
     fi
 
-    # Prompt user for cleanup with 60s timeout (plain echo - no msg_info to avoid spinner)
+    # Prompt user for cleanup with 60s timeout
     echo ""
-    echo -en "${YW}Remove broken container ${CTID}? (Y/n) [auto-remove in 60s]: ${CL}"
+    echo -en "${TAB}❓${TAB}${YW}Remove broken container ${CTID}? (Y/n) [auto-remove in 60s]: ${CL}"
 
     if read -t 60 -r response; then
       if [[ -z "$response" || "$response" =~ ^[Yy]$ ]]; then
         # Remove container
-        echo -e "\n${TAB}${HOLD}${YW}Removing container ${CTID}${CL}"
+        echo ""
+        msg_info "Removing container ${CTID}"
         pct stop "$CTID" &>/dev/null || true
         pct destroy "$CTID" &>/dev/null || true
-        echo -e "${BFR}${CM}${GN}Container ${CTID} removed${CL}"
+        msg_ok "Container ${CTID} removed"
       elif [[ "$response" =~ ^[Nn]$ ]]; then
-        echo -e "\n${TAB}${YW}Container ${CTID} kept for debugging${CL}"
+        echo ""
+        msg_warn "Container ${CTID} kept for debugging"
 
         # Dev mode: Setup MOTD/SSH for debugging access to broken container
         if [[ "${DEV_MODE_MOTD:-false}" == "true" ]]; then
@@ -4185,11 +4178,11 @@ EOF'
       fi
     else
       # Timeout - auto-remove
-      echo -e "\n${YW}No response - auto-removing container${CL}"
-      echo -e "${TAB}${HOLD}${YW}Removing container ${CTID}${CL}"
+      echo ""
+      msg_info "No response - removing container ${CTID}"
       pct stop "$CTID" &>/dev/null || true
       pct destroy "$CTID" &>/dev/null || true
-      echo -e "${BFR}${CM}${GN}Container ${CTID} removed${CL}"
+      msg_ok "Container ${CTID} removed"
     fi
 
     # Force one final status update attempt after cleanup

misc/core.func

@@ -522,15 +522,9 @@ silent() {
     msg_custom "→" "${YWB}" "${cmd}"
 
     if [[ -s "$logfile" ]]; then
-      local log_lines=$(wc -l <"$logfile")
-      echo "--- Last 10 lines of silent log ---"
+      echo -e "\n${TAB}--- Last 10 lines of log ---"
       tail -n 10 "$logfile"
-      echo "-----------------------------------"
-
-      # Show how to view full log if there are more lines
-      if [[ $log_lines -gt 10 ]]; then
-        msg_custom "📋" "${YW}" "View full log (${log_lines} lines): ${logfile}"
-      fi
+      echo -e "${TAB}-----------------------------------\n"
     fi
 
     exit "$rc"

misc/error_handler.func

@@ -175,9 +175,9 @@ error_handler() {
   fi
 
   if [[ -n "$active_log" && -s "$active_log" ]]; then
-    echo "--- Last 20 lines of silent log ---"
+    echo -e "\n${TAB}--- Last 20 lines of log ---"
     tail -n 20 "$active_log"
-    echo "-----------------------------------"
+    echo -e "${TAB}-----------------------------------\n"
 
     # Detect context: Container (INSTALL_LOG set + /root exists) vs Host (BUILD_LOG)
     if [[ -n "${INSTALL_LOG:-}" && -d /root ]]; then
@@ -204,23 +204,50 @@ error_handler() {
         fi
 
         echo ""
-        echo -en "${YW}Remove broken container ${CTID}? (Y/n) [auto-remove in 60s]: ${CL}"
+        if declare -f msg_custom >/dev/null 2>&1; then
+          echo -en "${TAB}❓${TAB}${YW}Remove broken container ${CTID}? (Y/n) [auto-remove in 60s]: ${CL}"
+        else
+          echo -en "${YW}Remove broken container ${CTID}? (Y/n) [auto-remove in 60s]: ${CL}"
+        fi
 
         if read -t 60 -r response; then
           if [[ -z "$response" || "$response" =~ ^[Yy]$ ]]; then
-            echo -e "\n${YW}Removing container ${CTID}${CL}"
+            echo ""
+            if declare -f msg_info >/dev/null 2>&1; then
+              msg_info "Removing container ${CTID}"
+            else
+              echo -e "${YW}Removing container ${CTID}${CL}"
+            fi
             pct stop "$CTID" &>/dev/null || true
             pct destroy "$CTID" &>/dev/null || true
-            echo -e "${GN}✔${CL} Container ${CTID} removed"
+            if declare -f msg_ok >/dev/null 2>&1; then
+              msg_ok "Container ${CTID} removed"
+            else
+              echo -e "${GN}✔${CL} Container ${CTID} removed"
+            fi
           elif [[ "$response" =~ ^[Nn]$ ]]; then
-            echo -e "\n${YW}Container ${CTID} kept for debugging${CL}"
+            echo ""
+            if declare -f msg_warn >/dev/null 2>&1; then
+              msg_warn "Container ${CTID} kept for debugging"
+            else
+              echo -e "${YW}Container ${CTID} kept for debugging${CL}"
+            fi
           fi
         else
           # Timeout - auto-remove
-          echo -e "\n${YW}No response - auto-removing container${CL}"
+          echo ""
+          if declare -f msg_info >/dev/null 2>&1; then
+            msg_info "No response - removing container ${CTID}"
+          else
+            echo -e "${YW}No response - removing container ${CTID}${CL}"
+          fi
           pct stop "$CTID" &>/dev/null || true
           pct destroy "$CTID" &>/dev/null || true
-          echo -e "${GN}✔${CL} Container ${CTID} removed"
+          if declare -f msg_ok >/dev/null 2>&1; then
+            msg_ok "Container ${CTID} removed"
+          else
+            echo -e "${GN}✔${CL} Container ${CTID} removed"
+          fi
         fi
 
         # Force one final status update attempt after cleanup
@@ -254,6 +281,10 @@ on_exit() {
   # post_to_api was called ("installing" sent) but post_update_to_api was never called
   if [[ "${POST_TO_API_DONE:-}" == "true" && "${POST_UPDATE_DONE:-}" != "true" ]]; then
     if declare -f post_update_to_api >/dev/null 2>&1; then
+      # Ensure log is accessible on host before reporting
+      if declare -f ensure_log_on_host >/dev/null 2>&1; then
+        ensure_log_on_host
+      fi
       if [[ $exit_code -ne 0 ]]; then
         post_update_to_api "failed" "$exit_code"
       else
@@ -273,6 +304,10 @@ on_exit() {
 # - Exits with code 130 (128 + SIGINT=2)
 # ------------------------------------------------------------------------------
 on_interrupt() {
+  # Ensure log is accessible on host before reporting
+  if declare -f ensure_log_on_host >/dev/null 2>&1; then
+    ensure_log_on_host
+  fi
   # Report interruption to telemetry API (prevents stuck "installing" records)
   if declare -f post_update_to_api >/dev/null 2>&1; then
     post_update_to_api "failed" "130"
@@ -294,6 +329,10 @@ on_interrupt() {
 # - Triggered by external process termination
 # ------------------------------------------------------------------------------
 on_terminate() {
+  # Ensure log is accessible on host before reporting
+  if declare -f ensure_log_on_host >/dev/null 2>&1; then
+    ensure_log_on_host
+  fi
   # Report termination to telemetry API (prevents stuck "installing" records)
   if declare -f post_update_to_api >/dev/null 2>&1; then
     post_update_to_api "failed" "143"

misc/vm-core.func

@@ -207,15 +207,9 @@ silent() {
     msg_custom "→" "${YWB}" "${cmd}"
 
     if [[ -s "$logfile" ]]; then
-      local log_lines=$(wc -l <"$logfile")
-      echo "--- Last 10 lines of log ---"
+      echo -e "\n${TAB}--- Last 10 lines of log ---"
       tail -n 10 "$logfile"
-      echo "----------------------------"
-
-      # Show how to view full log if there are more lines
-      if [[ $log_lines -gt 10 ]]; then
-        msg_custom "📋" "${YW}" "View full log (${log_lines} lines): ${logfile}"
-      fi
+      echo -e "${TAB}----------------------------\n"
     fi
 
     exit "$rc"