fix(install): clean up a broken managed Node and guard the termux probe
AI-review follow-up on #87467: - On probe failure, remove the extracted ~/.hermes/node tree and the node/npm/npx bin links so later installer steps and retry runs start clean instead of resolving node to a binary that cannot start. - The termux pkg branch had the same silent-success class: an empty version probe logged success and set HAS_NODE=true. Degrade with the binary's own error instead.
This commit is contained in:
+16
-3
@@ -980,8 +980,17 @@ install_node() {
|
||||
if pkg install -y nodejs >/dev/null; then
|
||||
local installed_ver
|
||||
installed_ver=$(node --version 2>/dev/null || true)
|
||||
log_success "Node.js $installed_ver installed via pkg"
|
||||
HAS_NODE=true
|
||||
if [ -n "$installed_ver" ]; then
|
||||
log_success "Node.js $installed_ver installed via pkg"
|
||||
HAS_NODE=true
|
||||
else
|
||||
# pkg succeeded but the binary cannot start — the same
|
||||
# silent-success class the managed-download probe guards
|
||||
# against (#87460). Degrade instead of claiming success.
|
||||
log_error "Node.js installed via pkg failed to start:"
|
||||
node --version >&2 || true
|
||||
HAS_NODE=false
|
||||
fi
|
||||
else
|
||||
log_warn "Failed to install Node.js via pkg"
|
||||
HAS_NODE=false
|
||||
@@ -1106,10 +1115,14 @@ install_node() {
|
||||
# whole installer at exit 127 with the loader's explanation
|
||||
# discarded by 2>/dev/null — installs died mid-sentence with no
|
||||
# output at all (#87460). Degrade instead, and surface the real
|
||||
# error the loader printed.
|
||||
# error the loader printed. Remove the broken tree and the bin
|
||||
# links so later steps and retry runs start clean instead of
|
||||
# resolving `node` to a binary that cannot start.
|
||||
log_error "Downloaded Node.js failed to start:"
|
||||
printf '%s\n' "$installed_ver" >&2
|
||||
log_info "On Debian/Ubuntu the usual fix is: sudo apt-get install -y libatomic1"
|
||||
rm -rf "$HERMES_HOME/node"
|
||||
rm -f "$node_link_dir/node" "$node_link_dir/npm" "$node_link_dir/npx"
|
||||
HAS_NODE=false
|
||||
return 0
|
||||
fi
|
||||
|
||||
@@ -153,6 +153,18 @@ def test_broken_node_degrades_with_clear_error(tmp_path: Path) -> None:
|
||||
assert "apt-get install -y libatomic1" in stdout + stderr
|
||||
assert "HAS_NODE=false" in stdout
|
||||
assert "HAS_NODE=true" not in stdout
|
||||
# AI-review follow-up: the broken tree and bin links must not linger
|
||||
# — retries and later installer steps resolve `node` cleanly.
|
||||
home = tmp_path / "home"
|
||||
link_dir = tmp_path / "links"
|
||||
assert not (home / "node").exists(), "broken managed Node tree left behind"
|
||||
assert not (link_dir / "node").exists(), "broken node symlink left behind"
|
||||
assert not (link_dir / "npm").exists(), "broken npm symlink left behind"
|
||||
# AI-review follow-up: the broken tree and bin links must not linger
|
||||
# — retries and later installer steps resolve `node` cleanly.
|
||||
assert not (home / "node").exists(), "broken managed Node tree left behind"
|
||||
assert not (link_dir / "node").exists(), "broken node symlink left behind"
|
||||
assert not (link_dir / "npm").exists(), "broken npm symlink left behind"
|
||||
|
||||
|
||||
def test_healthy_node_reports_success(tmp_path: Path) -> None:
|
||||
|
||||
Reference in New Issue
Block a user