Classify Node.js resolve failures via the failure framework (#1687)

* feat: classify Node resolve failures via the failure framework

* docs: changelog for Node.js resolve failure classification

* fix: return 1 on Node download/checksum failures and correct invalid-node assertion

Wrapping _install in `if ! { ... | tee }` disables errexit inside the
function body, so the bare `false` at the download and checksum failure
sites no longer aborted the install — _install fell through to tar/chmod
and could exit 0 when a stale /tmp/node.tar.gz was present. Use explicit
`return 1` so the failure propagates. Also fix the testInvalidNode stderr
assertion, which still expected the reworded "Error:" prefix after the
message was restored to its original verbatim text.

Author: colincasey

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## [Unreleased]
 
+- Migrated Node.js version-resolution build errors (unresolvable version, invalid semver requirement) onto the call-site failure-classification framework. The error messages are unchanged. ([#1687](https://github.com/heroku/heroku-buildpack-nodejs/pull/1687))
 
 ## [v356] - 2026-06-25
 

bin/compile

@@ -109,9 +109,7 @@ handle_failure() {
     fail_using_yarn2_with_yarn_production_environment_variable_set "$LOG_FILE"
     fail_yarn_outdated "$LOG_FILE"
     fail_yarn_lockfile_outdated "$LOG_FILE"
-    fail_node_install "$LOG_FILE" "$BUILD_DIR"
     fail_yarn_install "$LOG_FILE" "$BUILD_DIR"
-    fail_invalid_semver "$LOG_FILE"
     log_other_failures "$LOG_FILE"
     warn_aws_proxy "$BUILD_DIR"
     warn_untracked_dependencies "$LOG_FILE"
@@ -315,12 +313,7 @@ install_bins() {
   local install_start
 
   build_data::set_string "build_step" "install-nodejs"
-  install_start=$(build_data::current_unix_realtime)
   runtimes::nodejs::install "$node_engine" "$BUILD_DIR/.heroku/node"
-  build_data::set_duration "install_node_binary_time" "$install_start"
-  build_data::set_string "node_version" "$(node --version)"
-  build_data::set_raw "node_version_major" "$(get_node_major_version)"
-  build_data::set_string "bundled_npm_version" "$(npm --version)"
 
   build_data::set_string "build_step" "install-npm"
   install_npm "$npm_engine" "$BUILD_DIR/.heroku/node" "$NPM_LOCK"

lib/_failures.sh

@@ -254,64 +254,6 @@ fail_yarn_lockfile_outdated() {
   fi
 }
 
-fail_bin_install() {
-  local error
-  local version="$1"
-
-  # Allow the subcommand to fail without trapping the error so we can
-  # get the failing message output
-  set +e
-
-  # re-request the result, saving off the reason for the failure this time
-  error=$($RESOLVE "$BP_DIR/inventory/node.toml" "$version" 2>&1)
-
-  # re-enable trapping
-  set -e
-
-  if [[ $error = "No result" ]]; then
-    echo "Could not find Node version corresponding to version requirement: $version"
-  elif [[ $error == "Could not parse"* ]] || [[ $error == "Could not get"* ]]; then
-    echo "Error: Invalid semantic version \"$version\""
-  else
-    echo "Error: Unknown error installing \"$version\" of node"
-  fi
-
-  return 1
-}
-
-fail_node_install() {
-  local node_engine
-  local log_file="$1"
-  local build_dir="$2"
-
-  if grep -qi 'Could not find Node version corresponding to version requirement' "$log_file"; then
-    node_engine=$(read_json "$build_dir/package.json" ".engines.node")
-    build_data::set_string "failure" "invalid-node-version"
-    echo ""
-    warn "No matching version found for Node: $node_engine
-
-       Heroku supports the latest Stable version of Node.js as well as all
-       active LTS (Long-Term-Support) versions, however you have specified
-       a version in package.json ($node_engine) that does not correspond to
-       any published version of Node.js.
-
-       You should always specify a Node.js version that matches the runtime
-       you’re developing and testing with. To find your version locally:
-
-       $ node --version
-       v6.11.1
-
-       Use the engines section of your package.json to specify the version of
-       Node.js to use on Heroku. Drop the ‘v’ to save only the version number:
-
-       \"engines\": {
-         \"node\": \"6.11.1\"
-       }
-    " https://help.heroku.com/6235QYN4/
-    fail
-  fi
-}
-
 fail_yarn_install() {
   local yarn_engine
   local log_file="$1"
@@ -347,25 +289,6 @@ fail_yarn_install() {
   fi
 }
 
-fail_invalid_semver() {
-  local log_file="$1"
-  if grep -qi 'Error: Invalid semantic version' "$log_file"; then
-    build_data::set_string "failure" "invalid-semver-requirement"
-    echo ""
-    warn "Invalid semver requirement
-
-       Node, Yarn, and npm adhere to semver, the semantic versioning convention
-       popularized by GitHub.
-
-       http://semver.org/
-
-       However you have specified a version requirement that is not a valid
-       semantic version.
-    " https://help.heroku.com/0ZIOF3ST
-    fail
-  fi
-}
-
 # Yarn 2 failures
 
 fail_using_yarn2_with_yarn_production_environment_variable_set() {

lib/runtimes/nodejs.sh

@@ -14,6 +14,49 @@ RESOLVE="${BP_DIR}/lib/vendor/resolve-version-$(get_os)"
 function runtimes::nodejs::install() {
 	local requested_version="${1:-}"
 	local dir="${2:?}"
+
+	local log_file
+	log_file=$(mktemp)
+
+	local start
+	start=$(build_data::current_unix_realtime)
+
+	# Run inside `if !` so errexit is suppressed and we can inspect the failure ourselves.
+	# Capture only stdout into the log: `_install` echoes its failure discriminators to stdout
+	# for the classifier, while user-facing warnings/errors go to stderr (via output::*) and must
+	# pass straight through to the real stderr — so we deliberately do NOT merge `2>&1` here.
+	# `tee` passes stdout through, so normal install output still reaches the caller's pipe.
+	# shellcheck disable=SC2310 # invoked in a condition so set -e is disabled inside
+	if ! { runtimes::nodejs::_install "${requested_version}" "${dir}" | tee "${log_file}"; }; then
+		# Capture the full pipe status before any other command clobbers PIPESTATUS.
+		local install_exit="${PIPESTATUS[0]}"
+		build_data::set_duration "install_node_binary_time" "${start}"
+
+		local -A failure
+		# shellcheck disable=SC2310 # the classifier fills `failure` by nameref; invoked directly so its writes survive
+		if runtimes::nodejs::_handle_install_failure "${log_file}" "${requested_version}" failure; then
+			failure::emit failure
+		fi
+
+		# No known failure mode recognised. Bubble up so the legacy ERR trap classifies it
+		# (generic fallback) while matchers not yet migrated here still get handled.
+		return "${install_exit}"
+	fi
+
+	build_data::set_duration "install_node_binary_time" "${start}"
+
+	local node_version node_version_major bundled_npm_version
+	node_version=$(node --version)
+	node_version_major=$(get_node_major_version)
+	bundled_npm_version=$(npm --version)
+	build_data::set_string "node_version" "${node_version}"
+	build_data::set_raw "node_version_major" "${node_version_major}"
+	build_data::set_string "bundled_npm_version" "${bundled_npm_version}"
+}
+
+function runtimes::nodejs::_install() {
+	local requested_version="${1:-}"
+	local dir="${2:?}"
 	local resolve_result
 
 	if [[ -n "${NODE_BINARY_URL}" ]]; then
@@ -26,11 +69,22 @@ function runtimes::nodejs::install() {
 			echo "Resolving node version ${requested_version}..."
 		fi
 
-		# shellcheck disable=SC2310 # || disables errexit, but that's intentional for this pattern
-		resolve_result=$(runtimes::nodejs::_resolve "${requested_version}" || echo "failed")
+		# The resolver prints "No result" on stdout (exit 0) for an unknown version, or
+		# "Could not parse"/"Could not get …" for a malformed requirement; capture stderr too so
+		# either surfaces in a single invocation.
+		local resolve_result resolve_exit_code
+		resolve_result=$("${RESOLVE}" "${BP_DIR}/inventory/node.toml" "${requested_version}" 2>&1) && resolve_exit_code=0 || resolve_exit_code=$?
 
-		if [[ "${resolve_result}" == "failed" ]]; then
-			fail_bin_install "${requested_version}"
+		if [[ "${resolve_exit_code}" -ne 0 || "${resolve_result}" == "No result" ]]; then
+			# Print the canonical discriminating line so _handle_install_failure can classify it.
+			if [[ "${resolve_result}" == "No result" ]]; then
+				echo "Could not find Node version corresponding to version requirement: ${requested_version}"
+			elif [[ "${resolve_result}" == "Could not parse"* ]] || [[ "${resolve_result}" == "Could not get"* ]]; then
+				echo "Error: Invalid semantic version \"${requested_version}\""
+			else
+				echo "Error: Unknown error installing \"${requested_version}\" of node"
+			fi
+			return 1
 		fi
 
 		version=$(echo "${resolve_result}" | jq -r .version)
@@ -71,7 +125,7 @@ function runtimes::nodejs::install() {
 			If that doesn't help, check the Node.js status page:
 			https://status.nodejs.org/
 		EOF
-		false
+		return 1
 	fi
 
 	if [[ -z "${NODE_BINARY_URL}" ]]; then
@@ -83,15 +137,15 @@ function runtimes::nodejs::install() {
 				output::error <<-EOF
 					Checksum validation failed for Node.js ${version} - ${checksum_type}:${checksum_value}
 				EOF
-				false
+				return 1
 			fi
 			;;
 		*)
 			build_data::set_string "failure" "unsupported-checksum"
 			output::error <<-EOF
 				Unsupported checksum for Node.js ${version} - ${checksum_type}:${checksum_value}
 			EOF
-			false
+			return 1
 			;;
 		esac
 	fi
@@ -101,21 +155,6 @@ function runtimes::nodejs::install() {
 	chmod +x "${dir}"/bin/*
 }
 
-function runtimes::nodejs::_resolve() {
-	local node_version="$1"
-	local output
-
-	if output=$("${RESOLVE}" "${BP_DIR}/inventory/node.toml" "${node_version}"); then
-		if [[ ${output} = "No result" ]]; then
-			return 1
-		else
-			echo "${output}"
-			return 0
-		fi
-	fi
-	return 1
-}
-
 function runtimes::nodejs::_warn_wide_range() {
 	local requested_version="$1"
 	local lts_version="$2"
@@ -164,13 +203,70 @@ function runtimes::nodejs::_warn_known_bad_release() {
 
 # Pure classifier for Node.js install failures.
 #
-# Input:  $1 path to the captured Node install log; $2 name of an associative array to fill.
+# Input:  $1 path to the captured Node install log; $2 the requested version requirement;
+#         $3 name of an associative array to fill.
 # Returns 0 + fills the array on a known failure; returns 1 untouched otherwise. No side effects.
 function runtimes::nodejs::_handle_install_failure() {
 	local log_file="${1}"
+	local requested_version="${2}"
 	# shellcheck disable=SC2178 # nameref alias to the caller's associative array, not a string
-	local -n __failure="${2}"
+	local -n __failure="${3}"
+
+	if grep -qi 'Could not find Node version corresponding to version requirement' "${log_file}"; then
+		__failure["id"]="invalid-node-version"
+		__failure["classification"]="user"
+		__failure["detail"]="${requested_version}"
+		__failure["message"]=$(
+			cat <<-EOF
+				No matching version found for Node: ${requested_version}
+
+				Heroku supports the latest Stable version of Node.js as well as all
+				active LTS (Long-Term-Support) versions, however you have specified
+				a version in package.json (${requested_version}) that does not correspond to
+				any published version of Node.js.
+
+				You should always specify a Node.js version that matches the runtime
+				you're developing and testing with. To find your version locally:
+
+				$ node --version
+				v6.11.1
+
+				Use the engines section of your package.json to specify the version of
+				Node.js to use on Heroku. Drop the 'v' to save only the version number:
+
+				"engines": {
+				  "node": "6.11.1"
+				}
+
+				https://help.heroku.com/6235QYN4/
+			EOF
+		)
+		return 0
+	fi
+
+	if grep -qi 'Error: Invalid semantic version' "${log_file}"; then
+		__failure["id"]="invalid-semver-requirement"
+		__failure["classification"]="user"
+		__failure["detail"]="${requested_version}"
+		__failure["message"]=$(
+			cat <<-EOF
+				Invalid semver requirement
+
+				Node, Yarn, and npm adhere to semver, the semantic versioning convention
+				popularized by GitHub.
+
+				http://semver.org/
+
+				However you have specified a version requirement that is not a valid
+				semantic version.
+
+				https://help.heroku.com/0ZIOF3ST
+			EOF
+		)
+		return 0
+	fi
 
+	# No known failure mode recognised — let the caller fall through.
 	return 1
 }
 

test/run-general

@@ -389,21 +389,25 @@ testConcurrencyTooHigh() {
 
 
 testInvalidNode() {
-  compile "invalid-node"
+  cache_dir=$(mktmpdir)
+  compile "invalid-node" "${cache_dir}"
   assertCaptured "Resolving node version 0.11.333"
   assertCaptured "Could not find Node version corresponding to version requirement: 0.11.333"
-  assertCaptured "No matching version found for Node: 0.11.333"
-  assertCaptured "https://help.heroku.com/6235QYN4"
+  assertCapturedStderr "No matching version found for Node: 0.11.333"
+  assertCapturedStderr "https://help.heroku.com/6235QYN4"
   assertCapturedError
+  assertMetricEqualsString "${cache_dir}" "failure" "invalid-node-version"
 }
 
 
 testInvalidNodeSemver() {
-  compile "invalid-node-semver"
+  cache_dir=$(mktmpdir)
+  compile "invalid-node-semver" "${cache_dir}"
   assertCaptured "Resolving node version stable"
   assertCaptured "Error: Invalid semantic version \"stable\""
-  assertCaptured "Invalid semver requirement"
+  assertCapturedStderr "Invalid semver requirement"
   assertCapturedError
+  assertMetricEqualsString "${cache_dir}" "failure" "invalid-semver-requirement"
 }
 
 

test/unit

@@ -703,12 +703,31 @@ testHandleNpmInstallNoMatchReturnsNonZero() {
 testHandleNodeInstallScaffoldNoMatch() {
   local -A result
   # Scaffold classifier recognises nothing yet; it must return non-zero and leave the array empty.
-  if runtimes::nodejs::_handle_install_failure "$(_unit_fixture node-failures/no-result.log)" result; then
+  if runtimes::nodejs::_handle_install_failure "$(_unit_fixture node-failures/no-result.log)" "16.0.0" result; then
     fail "expected scaffold classifier to return non-zero"
   fi
   assertEquals "array should be left empty on no match" "" "${result[id]:-}"
 }
 
+testHandleNodeInstallInvalidVersion() {
+  local -A result
+  runtimes::nodejs::_handle_install_failure "$(_unit_fixture node-failures/invalid-version.log)" "0.11.333" result
+  assertEquals "classifier should return 0 on a match" "0" "$?"
+  assertEquals "invalid-node-version" "${result[id]}"
+  assertEquals "user" "${result[classification]}"
+  assertContains "${result[message]}" "No matching version found for Node: 0.11.333"
+  assertContains "${result[detail]}" "0.11.333"
+}
+
+testHandleNodeInstallInvalidSemver() {
+  local -A result
+  runtimes::nodejs::_handle_install_failure "$(_unit_fixture node-failures/invalid-semver.log)" "stable" result
+  assertEquals "invalid-semver-requirement" "${result[id]}"
+  assertEquals "user" "${result[classification]}"
+  assertContains "${result[message]}" "Invalid semver requirement"
+  assertEquals "stable" "${result[detail]}"
+}
+
 testExtractErrorDetailReturnsFirstDescriptiveLine() {
   # Skips the bare `code <CODE>` line and the "complete log" noise, strips the
   # `npm error `/`npm ERR! ` prefix (but keeps any keyword like `notsup` that follows).

test/unit-fixtures/node-failures/invalid-semver.log

@@ -0,0 +1,2 @@
+Resolving node version stable...
+Error: Invalid semantic version "stable"

test/unit-fixtures/node-failures/invalid-version.log

@@ -0,0 +1,2 @@
+Resolving node version 0.11.333...
+Could not find Node version corresponding to version requirement: 0.11.333