fix: handle npm install failures gracefully instead of crashing installer (#24)

The installer crashed with exit status 1 when npm install failed during
Phase 3, because unguarded run_command calls allowed set -e to abort the
entire post-install action.

Changes:
- Wrap all npm/yarn install calls in error handlers so failures produce
  helpful retry messages instead of crashing the installer
- Replace mapfile with a bash 3.2-compatible while-read loop (macOS
  ships bash 3.2 which lacks mapfile, causing an unbound variable crash)
- Use test -s instead of test -f when waiting for .cspell.json in the
  container, so the wait detects empty files from partial filesystem sync
- Only emit "Node toolchain installed" when installation actually succeeds

Fixes #23

Author: UltraBob

dcq-install.sh

@@ -435,7 +435,11 @@ maybe_install_missing_root_deps() {
     return 1
   fi
 
-  mapfile -t missing_node_deps_array <<< "$missing_node_deps"
+  # Avoid mapfile (requires bash 4+; macOS ships bash 3.2).
+  missing_node_deps_array=()
+  while IFS= read -r _line; do
+    [ -n "$_line" ] && missing_node_deps_array+=("$_line")
+  done <<< "$missing_node_deps"
   if [ "$suppress_list" -ne 1 ]; then
     emit 'Detected missing Drupal JS tooling dependencies in package.json (%d):\n' "${#missing_node_deps_array[@]}"
     for dep in "${missing_node_deps_array[@]}"; do
@@ -467,13 +471,22 @@ maybe_install_missing_root_deps() {
     done
     if [ "$package_manager" = "npm" ]; then
       cmd=( "$ddev_cmd" "exec" "bash" "-lc" "cd /var/www/html && npm install --save-dev --package-lock${deps_cmd}" )
-      run_command "${cmd[@]}"
+      if ! run_command "${cmd[@]}"; then
+        emit 'npm install --save-dev failed. You can retry manually.\n'
+        return 1
+      fi
       emit 'Node dependencies added (project root).\n'
       cmd=( "$ddev_cmd" "exec" "bash" "-lc" "cd /var/www/html && npm install --package-lock" )
-      run_command "${cmd[@]}"
+      if ! run_command "${cmd[@]}"; then
+        emit 'npm install failed. You can retry manually.\n'
+        return 1
+      fi
     else
       cmd=( "$ddev_cmd" "exec" "bash" "-lc" "cd /var/www/html && yarn add -D${deps_cmd}" )
-      run_command "${cmd[@]}"
+      if ! run_command "${cmd[@]}"; then
+        emit 'yarn add failed. You can retry manually.\n'
+        return 1
+      fi
       emit 'Node dependencies added (project root).\n'
     fi
     return 0
@@ -996,8 +1009,8 @@ expand_cspell_config() {
 
   emit 'Expanding .cspell.json with project-specific settings...\n'
 
-  if ! wait_for_container_file "$ddev_cmd" "$container_cspell" 20 1; then
-    emit 'Skipping CSpell expansion (.cspell.json did not become visible in the container after waiting).\n'
+  if ! wait_for_container_file "$ddev_cmd" "$container_cspell" 20 1 "readable"; then
+    emit 'Skipping CSpell expansion (.cspell.json not readable in the container after waiting).\n'
     return 0
   fi
 
@@ -1079,14 +1092,23 @@ command_available() {
 }
 
 wait_for_container_file() {
+  # Wait for a file to appear in the container.  When require_readable is set
+  # to a non-empty value the check additionally verifies that the file has
+  # non-zero size (test -s), which guards against filesystem-sync race
+  # conditions where the inode is visible but the content has not landed yet.
   local ddev_cmd="$1"
   local path="$2"
   local max_attempts="${3:-20}"
   local delay_seconds="${4:-1}"
+  local require_readable="${5:-}"
   local attempts=0
+  local test_flag="-f"
+  if [ -n "$require_readable" ]; then
+    test_flag="-s"
+  fi
 
   while [ "$attempts" -lt "$max_attempts" ]; do
-    if "$ddev_cmd" exec test -f "$path" >/dev/null 2>&1; then
+    if "$ddev_cmd" exec test "$test_flag" "$path" >/dev/null 2>&1; then
       return 0
     fi
     attempts=$((attempts + 1))
@@ -2215,9 +2237,16 @@ if [ "$core_package_json_present" -eq 1 ]; then
           else
             cmd=( "$ddev_cmd" "exec" "bash" "-lc" "cd /var/www/html && yarn install" )
           fi
-          run_command "${cmd[@]}"
+          if run_command "${cmd[@]}"; then
+            node_install_done=1
+          else
+            emit 'JS dependency install failed. You can retry manually:\n'
+            emit '  ddev exec bash -lc "cd /var/www/html && %s install"\n' "${root_pm:-npm}"
+          fi
+        fi
+        if [ "$node_install_done" -eq 1 ]; then
+          emit 'Node toolchain installed (project root).\n'
         fi
-        emit 'Node toolchain installed (project root).\n'
       fi
     fi
   fi

tests/test.bats

@@ -1360,6 +1360,20 @@ SH
   assert_output --partial "Skipping CSpell expansion (Drupal core dictionary files are not available at web/core/misc/cspell)."
 }
 
+@test "container test -s rejects empty files and accepts non-empty files" {
+  # Validates the mechanism behind wait_for_container_file's readability check.
+  # An empty file (simulating a partially-synced inode) must fail test -s so
+  # that the installer waits for content to arrive rather than handing a zero-
+  # byte file to prepare-cspell.php.
+  set -u -o pipefail
+
+  run ddev exec bash -lc 'truncate -s 0 /tmp/dcq-empty-test && test -s /tmp/dcq-empty-test'
+  assert_failure
+
+  run ddev exec bash -lc 'printf "{}" > /tmp/dcq-nonempty-test && test -s /tmp/dcq-nonempty-test'
+  assert_success
+}
+
 @test "remove cleans ddev assets and shims" {
   set -u -o pipefail
   run ddev add-on get "${DIR}"
@@ -1401,7 +1415,7 @@ SH
 
   run ddev add-on get "${DIR}"
   assert_success
-  assert_output --partial "Node dependencies added (project root)."
+  assert_output --partial "Node toolchain installed (project root)."
   assert_container_file_exist "/var/www/html/package-lock.json"
   assert_container_file_exist "/var/www/html/node_modules/eslint-plugin-no-jquery/package.json"
   assert_container_file_exist "/var/www/html/node_modules/stylelint-prettier/package.json"
@@ -1424,7 +1438,7 @@ SH
 
   run ddev add-on get "${DIR}"
   assert_success
-  assert_output --partial "Node dependencies added (project root)."
+  assert_output --partial "Node toolchain installed (project root)."
   assert_container_file_exist "/var/www/html/yarn.lock"
   assert_container_file_exist "/var/www/html/node_modules/eslint-plugin-no-jquery/package.json"
   assert_container_file_exist "/var/www/html/node_modules/stylelint-prettier/package.json"
@@ -1441,12 +1455,43 @@ SH
 
   run ddev add-on get "${DIR}"
   assert_success
-  assert_output --partial "Node dependencies added (project root)."
+  assert_output --partial "Node toolchain installed (project root)."
   assert_container_file_exist "/var/www/html/package-lock.json"
   assert_container_file_exist "/var/www/html/node_modules/eslint-plugin-no-jquery/package.json"
   assert_container_file_exist "/var/www/html/node_modules/stylelint-prettier/package.json"
 }
 
+# bats test_tags=node
+@test "node install failure does not crash the installer" {
+  set -u -o pipefail
+  export DCQ_INSTALL_DEPS=skip
+  export DCQ_INSTALL_NODE_DEPS=root
+  mkdir -p web/core
+  write_stub_package_json "web/core/package.json"
+
+  # Pre-create a root package.json that includes the same deps as the core
+  # stub (so find_missing_node_deps returns nothing and we reach the fallback
+  # npm-install path) PLUS an unresolvable dependency so npm install fails.
+  # The installer should handle this gracefully instead of crashing the
+  # entire post-install action via set -e.
+  cat > package.json <<'JSON'
+{
+  "name": "dcq-fail-test",
+  "private": true,
+  "devDependencies": {
+    "eslint-plugin-no-jquery": "^3.1.1",
+    "stylelint-prettier": "^5.0.3",
+    "this-package-should-not-exist-dcq-test": "99999.0.0"
+  }
+}
+JSON
+
+  run bash -lc "ddev add-on get \"${DIR}\" 2>&1"
+  assert_success
+  assert_output --partial "JS dependency install failed."
+  assert_output --partial "You can retry manually"
+}
+
 # bats test_tags=full
 @test "fresh install (full)" {
   set -eu -o pipefail