fix: harden installed package lifecycle

Author: pastelsky

src/getPackageExportSizes.ts

@@ -22,10 +22,9 @@ async function analyzeAllPackageExports(
   signal?: AbortSignal,
 ) {
   throwIfAborted(signal)
-  const packagePath = installation.sourcePath || installation.packagePath
   return getAllExports(
     installation.packageString,
-    packagePath,
+    installation.packagePath,
     installation.packageName,
     installation.installPath,
     signal,
@@ -34,9 +33,9 @@ async function analyzeAllPackageExports(
 
 async function getExportsForInstalledPackage(
   installedPackage: InstalledPackage,
-  options: InstallPackageOptions = {},
+  options: InstallPackageOptions,
+  startTime: number,
 ) {
-  const startTime = performance.now()
   const packageString = installedPackage.packageString
 
   try {
@@ -57,13 +56,14 @@ export async function getPackageExports(
   packageInput: string | InstalledPackage,
   options: InstallPackageOptions = {},
 ) {
+  const startTime = performance.now()
   if (typeof packageInput === 'string') {
     return withInstalledPackage(packageInput, options, installedPackage =>
-      getExportsForInstalledPackage(installedPackage, options),
+      getExportsForInstalledPackage(installedPackage, options, startTime),
     )
   }
 
-  return getExportsForInstalledPackage(packageInput, options)
+  return getExportsForInstalledPackage(packageInput, options, startTime)
 }
 
 async function analyzePackageExportSizes(
@@ -166,9 +166,9 @@ async function analyzePackageExportSizes(
 
 async function getExportSizesForInstalledPackage(
   installedPackage: InstalledPackage,
-  options: GetPackageStatsOptions = {},
+  options: GetPackageStatsOptions,
+  startTime: number,
 ) {
-  const startTime = performance.now()
   const packageString = installedPackage.packageString
 
   try {
@@ -192,11 +192,12 @@ export async function getPackageExportSizes(
   packageInput: string | InstalledPackage,
   options: GetPackageStatsOptions = {},
 ) {
+  const startTime = performance.now()
   if (typeof packageInput === 'string') {
     return withInstalledPackage(packageInput, options, installedPackage =>
-      getExportSizesForInstalledPackage(installedPackage, options),
+      getExportSizesForInstalledPackage(installedPackage, options, startTime),
     )
   }
 
-  return getExportSizesForInstalledPackage(packageInput, options)
+  return getExportSizesForInstalledPackage(packageInput, options, startTime)
 }

src/getPackageStats.ts

@@ -110,9 +110,9 @@ async function analyzePackageStats(
 
 async function getStatsForInstalledPackage(
   installedPackage: InstalledPackage,
-  options: GetPackageStatsOptions = {},
+  options: GetPackageStatsOptions,
+  startTime: number,
 ) {
-  const startTime = performance.now()
   const packageString = installedPackage.packageString
 
   try {
@@ -141,11 +141,12 @@ export default async function getPackageStats(
   packageInput: string | InstalledPackage,
   options: GetPackageStatsOptions = {},
 ) {
+  const startTime = performance.now()
   if (typeof packageInput === 'string') {
     return withInstalledPackage(packageInput, options, installedPackage =>
-      getStatsForInstalledPackage(installedPackage, options),
+      getStatsForInstalledPackage(installedPackage, options, startTime),
     )
   }
 
-  return getStatsForInstalledPackage(packageInput, options)
+  return getStatsForInstalledPackage(packageInput, options, startTime)
 }

src/installedPackage.ts

@@ -1,4 +1,5 @@
 import path from 'node:path'
+import fs from 'node:fs/promises'
 import type { InstallPackageOptions } from './common.types.js'
 import { parsePackageString, throwIfAborted } from './utils/common.utils.js'
 import InstallationUtils from './utils/installation.utils.js'
@@ -11,19 +12,14 @@ export type InstalledPackage = Readonly<{
   packageName: string
   installPath: string
   packagePath: string
-  sourcePath?: string
 }>
 
 /** Installs a package once and returns a handle that analyses can reuse. */
 export async function installPackage(
   packageString: string,
   options: InstallPackageOptions = {},
 ): Promise<InstalledPackage> {
-  const {
-    name: packageName,
-    isLocal,
-    normalPath: sourcePath,
-  } = parsePackageString(packageString)
+  const { name: packageName, isLocal } = parsePackageString(packageString)
   const installPath = await InstallationUtils.preparePath(
     packageName,
     options.client,
@@ -44,19 +40,23 @@ export async function installPackage(
       packageName,
       installPath,
       packagePath: path.join(installPath, 'node_modules', packageName),
-      sourcePath,
     })
   } catch (error) {
-    if (!options.debug) {
-      await InstallationUtils.cleanupPath(installPath)
-    }
+    await InstallationUtils.cleanupPath(installPath)
     throw error
   }
 }
 
 /** Removes the workspace owned by a package installation handle. */
 export async function disposePackage(installedPackage: InstalledPackage) {
   await InstallationUtils.cleanupPath(installedPackage.installPath)
+  try {
+    await fs.access(installedPackage.installPath)
+  } catch (error) {
+    if ((error as NodeJS.ErrnoException).code === 'ENOENT') return
+    throw error
+  }
+  throw new Error(`Failed to remove ${installedPackage.installPath}`)
 }
 
 /** Runs one analysis with a temporary package installation. */
@@ -70,8 +70,6 @@ export async function withInstalledPackage<TResult>(
   try {
     return await analyze(installedPackage)
   } finally {
-    if (!options.debug) {
-      await disposePackage(installedPackage)
-    }
+    await disposePackage(installedPackage)
   }
 }