Add more debug around resolving the actual arborist tree

Author: jdalton

src/commands/fix/agent-fix.mts

@@ -94,16 +94,16 @@ export type InstallPhaseHandler = (
   fixConfig: FixConfig,
 ) => Promise<void>
 
-export type Installer = (
-  pkgEnvDetails: EnvDetails,
-  options: InstallOptions,
-) => Promise<InstallerResult>
-
-export type InstallerResult = {
+export type ActualTreeResult = {
   actualTree?: NodeClass | undefined
   error?: unknown | undefined
 }
 
+export type Installer = (
+  pkgEnvDetails: EnvDetails,
+  options: InstallOptions,
+) => Promise<ActualTreeResult>
+
 const noopHandler = (() => {}) as unknown as InstallPhaseHandler
 
 export async function agentFix(
@@ -282,7 +282,13 @@ export async function agentFix(
         }
         if (fixEnv.isCi && existsSync(path.join(rootPath, 'node_modules'))) {
           // eslint-disable-next-line no-await-in-loop
-          actualTree = await getActualTree(cwd)
+          const treeResult = await getActualTree(cwd)
+          const maybeActualTree = treeResult.actualTree
+          if (!maybeActualTree) {
+            // Exit early if install fails.
+            return handleInstallFail(treeResult.error)
+          }
+          actualTree = maybeActualTree
         } else {
           // eslint-disable-next-line no-await-in-loop
           const installResult = await installer(pkgEnvDetails, { cwd, spinner })

src/commands/fix/get-actual-tree.mts

@@ -1,18 +1,22 @@
 import { Arborist } from '../../shadow/npm/arborist/index.mts'
 import { SAFE_NO_SAVE_ARBORIST_REIFY_OPTIONS_OVERRIDES } from '../../shadow/npm/arborist/lib/arborist/index.mts'
 
-import type { NodeClass } from '../../shadow/npm/arborist/types.mts'
+import type { ActualTreeResult } from './agent-fix.mts'
 
 export async function getActualTree(
   cwd: string = process.cwd(),
-): Promise<NodeClass> {
-  // @npmcli/arborist DOES have partial support for pnpm structured node_modules
-  // folders. However, support is iffy resulting in unhappy path errors and hangs.
-  // So, to avoid the unhappy path, we restrict our usage to --dry-run loading
-  // of the node_modules folder.
-  const arb = new Arborist({
-    path: cwd,
-    ...SAFE_NO_SAVE_ARBORIST_REIFY_OPTIONS_OVERRIDES,
-  })
-  return await arb.loadActual()
+): Promise<ActualTreeResult> {
+  try {
+    // @npmcli/arborist DOES have partial support for pnpm structured node_modules
+    // folders. However, support is iffy resulting in unhappy paths of errors and hangs.
+    // So, to avoid unhappy paths, we restrict our usage to --dry-run loading of the
+    // node_modules folder.
+    const arb = new Arborist({
+      path: cwd,
+      ...SAFE_NO_SAVE_ARBORIST_REIFY_OPTIONS_OVERRIDES,
+    })
+    return { actualTree: await arb.loadActual() }
+  } catch (e) {
+    return { error: e }
+  }
 }

src/commands/fix/npm-fix.mts

@@ -15,9 +15,9 @@ import { getAlertsMapFromPurls } from '../../utils/alerts-map.mts'
 import { getNpmConfig } from '../../utils/npm-config.mts'
 
 import type {
+  ActualTreeResult,
   FixConfig,
   InstallOptions,
-  InstallerResult,
 } from './agent-fix.mts'
 import type {
   ArboristInstance,
@@ -31,7 +31,7 @@ import type { PackageJson } from '@socketsecurity/registry/lib/packages'
 async function install(
   pkgEnvDetails: EnvDetails,
   options: InstallOptions,
-): Promise<InstallerResult> {
+): Promise<ActualTreeResult> {
   const {
     args: extraArgs,
     cwd,
@@ -70,45 +70,38 @@ async function install(
     ...(useDebug ? [] : ['--silent']),
     ...(extraArgs ?? []),
   ]
-  const quotedCmd = `\`${pkgEnvDetails.agent} install ${args.join(' ')}\``
-  debugFn('stdio', `spawn: ${quotedCmd}`)
 
   const isSpinning = spinner?.isSpinning
   spinner?.stop()
 
-  let error
-  let errored = false
+  const quotedCmd = `\`${pkgEnvDetails.agent} install ${args.join(' ')}\``
+  debugFn('stdio', `spawn: ${quotedCmd}`)
   try {
     await runAgentInstall(pkgEnvDetails, {
       args,
       spinner,
       stdio: useDebug ? 'inherit' : 'ignore',
     })
-  } catch (e) {
-    errored = true
-    error = e
+  } catch (error) {
+    const result = { error }
     debugFn('error', `caught: ${quotedCmd} failed`)
-    debugDir('inspect', { error })
+    debugDir('inspect', result)
+    return result
   }
 
-  let actualTree: NodeClass | undefined = undefined
-  if (!errored) {
-    try {
-      actualTree = await getActualTree(cwd)
-    } catch (e) {
-      errored = true
-      error = e
-      debugFn('error', 'caught: Arborist error')
-      debugDir('inspect', { error })
+  const treeResult = await getActualTree(cwd)
+  if (treeResult.actualTree) {
+    if (isSpinning) {
+      spinner.start()
     }
+    return treeResult
   }
+  debugFn('error', 'caught: await arb.loadActual() error')
+  debugDir('inspect', treeResult)
   if (isSpinning) {
     spinner.start()
   }
-  return {
-    ...(actualTree ? { actualTree } : undefined),
-    ...(errored ? { error } : undefined),
-  }
+  return treeResult
 }
 
 export async function npmFix(
@@ -145,8 +138,8 @@ export async function npmFix(
         debugDir('inspect', { error: e })
         return {
           ok: false,
-          message: 'Arborist error',
-          cause: (e as Error)?.message || 'Unknown Arborist error.',
+          message: 'npm error',
+          cause: (e as Error)?.message || 'Unknown npm error.',
         }
       }
       alertsMap = await getAlertsMapFromArborist(arb, getFixAlertsMapOptions())

src/commands/fix/pnpm-fix.mts

@@ -22,9 +22,9 @@ import { applyRange } from '../../utils/semver.mts'
 import { getOverridesDataPnpm } from '../optimize/get-overrides-by-agent.mts'
 
 import type {
+  ActualTreeResult,
   FixConfig,
   InstallOptions,
-  InstallerResult,
 } from './agent-fix.mts'
 import type { NodeClass } from '../../shadow/npm/arborist/types.mts'
 import type { CResult, StringKeyValueObject } from '../../types.mts'
@@ -36,7 +36,7 @@ const { OVERRIDES, PNPM } = constants
 async function install(
   pkgEnvDetails: EnvDetails,
   options: InstallOptions,
-): Promise<InstallerResult> {
+): Promise<ActualTreeResult> {
   const {
     args: extraArgs,
     cwd,
@@ -57,45 +57,38 @@ async function install(
     '--config.confirmModulesPurge=false',
     ...(extraArgs ?? []),
   ]
-  const quotedCmd = `\`${pkgEnvDetails.agent} install ${args.join(' ')}\``
-  debugFn('stdio', `spawn: ${quotedCmd}`)
 
   const isSpinning = spinner?.isSpinning
   spinner?.stop()
 
-  let error
-  let errored = false
+  const quotedCmd = `\`${pkgEnvDetails.agent} install ${args.join(' ')}\``
+  debugFn('stdio', `spawn: ${quotedCmd}`)
   try {
     await runAgentInstall(pkgEnvDetails, {
       args,
       spinner,
       stdio: isDebug('stdio') ? 'inherit' : 'ignore',
     })
-  } catch (e) {
-    errored = true
-    error = e
+  } catch (error) {
+    const result = { error }
     debugFn('error', `caught: ${quotedCmd} failed`)
-    debugDir('inspect', { error })
+    debugDir('inspect', result)
+    return result
   }
 
-  let actualTree: NodeClass | undefined = undefined
-  if (!errored) {
-    try {
-      actualTree = await getActualTree(cwd)
-    } catch (e) {
-      errored = true
-      error = e
-      debugFn('error', 'caught: Arborist error')
-      debugDir('inspect', { error })
+  const treeResult = await getActualTree(cwd)
+  if (treeResult.actualTree) {
+    if (isSpinning) {
+      spinner.start()
     }
+    return treeResult
   }
+  debugFn('error', 'caught: await arb.loadActual() error')
+  debugDir('inspect', treeResult)
   if (isSpinning) {
     spinner.start()
   }
-  return {
-    ...(actualTree ? { actualTree } : undefined),
-    ...(errored ? { error } : undefined),
-  }
+  return treeResult
 }
 
 export async function pnpmFix(