Avoid problems with bundle writing while compressing the bundle

Author: alfonso-noriega

packages/cli-kit/src/public/node/archiver.integration.test.ts

@@ -2,7 +2,7 @@ import {zip, brotliCompress} from './archiver.js'
 import {fileExists, inTemporaryDirectory, mkdir, touchFile} from './fs.js'
 import {joinPath, dirname} from './path.js'
 import {exec} from './system.js'
-import {describe, expect, test} from 'vitest'
+import {describe, expect, test, vi} from 'vitest'
 import StreamZip from 'node-stream-zip'
 import brotli from 'brotli'
 import fs from 'fs'
@@ -57,6 +57,51 @@ describe('zip', () => {
       expect(expectedEntries.sort()).toEqual(archiveEntries.sort())
     })
   })
+
+  test('gracefully handles files deleted during archiving', async () => {
+    await inTemporaryDirectory(async (tmpDir) => {
+      // Given
+      const zipPath = joinPath(tmpDir, 'output.zip')
+      const outputDirectoryName = 'output'
+      const outputDirectoryPath = joinPath(tmpDir, outputDirectoryName)
+      const structure = ['file1.js', 'file2.js', 'file3.js']
+
+      await createFiles(structure, outputDirectoryPath)
+
+      const file2Path = joinPath(outputDirectoryPath, 'file2.js')
+
+      // Spy on readFile to simulate a file being deleted during archiving
+      // We'll make readFile throw ENOENT for file2.js specifically
+      const fsModule = await import('./fs.js')
+      const originalReadFile = fsModule.readFile
+      const readFileSpy = vi.spyOn(fsModule, 'readFile')
+      readFileSpy.mockImplementation((async (path: string, options?: unknown) => {
+        if (path === file2Path) {
+          const error: NodeJS.ErrnoException = new Error(`ENOENT: no such file or directory, open '${path}'`)
+          error.code = 'ENOENT'
+          throw error
+        }
+        return originalReadFile(path, options as never)
+      }) as typeof originalReadFile)
+
+      // When - should not throw even though file2.js throws ENOENT
+      await expect(
+        zip({
+          inputDirectory: outputDirectoryPath,
+          outputZipPath: zipPath,
+        }),
+      ).resolves.not.toThrow()
+
+      // Then - archive should contain remaining files only
+      const archiveEntries = await readArchiveFiles(zipPath)
+      expect(archiveEntries).toContain('file1.js')
+      expect(archiveEntries).toContain('file3.js')
+      // file2.js should not be in archive since readFile failed
+      expect(archiveEntries).not.toContain('file2.js')
+
+      readFileSpy.mockRestore()
+    })
+  })
 })
 
 describe('brotliCompress', () => {
@@ -66,7 +111,6 @@ describe('brotliCompress', () => {
       const brotliPath = joinPath(tmpDir, 'output.br')
       const outputDirectoryName = 'output'
       const outputDirectoryPath = joinPath(tmpDir, outputDirectoryName)
-      const extractPath = joinPath(tmpDir, 'extract')
       const testContent = 'test content'
 
       // Create test file
@@ -141,6 +185,48 @@ describe('brotliCompress', () => {
       expect(fs.existsSync(joinPath(extractPath, 'test.json'))).toBeFalsy()
     })
   })
+
+  test('gracefully handles files deleted during compression', async () => {
+    await inTemporaryDirectory(async (tmpDir) => {
+      // Given
+      const brotliPath = joinPath(tmpDir, 'output.br')
+      const outputDirectoryName = 'output'
+      const outputDirectoryPath = joinPath(tmpDir, outputDirectoryName)
+      const structure = ['file1.js', 'file2.js', 'file3.js']
+
+      await createFiles(structure, outputDirectoryPath)
+
+      const file2Path = joinPath(outputDirectoryPath, 'file2.js')
+
+      // Spy on readFile to simulate a file being deleted during compression
+      // We'll make readFile throw ENOENT for file2.js specifically
+      const fsModule = await import('./fs.js')
+      const originalReadFile = fsModule.readFile
+      const readFileSpy = vi.spyOn(fsModule, 'readFile')
+      readFileSpy.mockImplementation((async (path: string, options?: unknown) => {
+        if (path === file2Path) {
+          const error: NodeJS.ErrnoException = new Error(`ENOENT: no such file or directory, open '${path}'`)
+          error.code = 'ENOENT'
+          throw error
+        }
+        return originalReadFile(path, options as never)
+      }) as typeof originalReadFile)
+
+      // When - should not throw even though file2.js throws ENOENT
+      await expect(
+        brotliCompress({
+          inputDirectory: outputDirectoryPath,
+          outputPath: brotliPath,
+        }),
+      ).resolves.not.toThrow()
+
+      // Then - compressed file should exist and be valid
+      const exists = await fileExists(brotliPath)
+      expect(exists).toBeTruthy()
+
+      readFileSpy.mockRestore()
+    })
+  })
 })
 
 async function createFiles(structure: string[], directory: string) {

packages/cli-kit/src/public/node/archiver.ts

@@ -1,5 +1,5 @@
 import {relativePath, joinPath, dirname} from './path.js'
-import {glob, removeFile} from './fs.js'
+import {glob, removeFile, readFile} from './fs.js'
 import {outputDebug, outputContent, outputToken} from '../../public/node/output.js'
 import archiver from 'archiver'
 import {createWriteStream, readFileSync, writeFileSync} from 'fs'
@@ -64,13 +64,22 @@ export async function zip(options: ZipOptions): Promise<void> {
       archive.append(Buffer.alloc(0), {name: dirName})
     }
 
-    for (const filePath of pathsToZip) {
-      const fileRelativePath = relativePath(inputDirectory, filePath)
-      if (filePath && fileRelativePath) archive.file(filePath, {name: fileRelativePath})
-    }
+    // Read all files immediately before adding to archive to prevent ENOENT errors
+    // Using archive.file() causes lazy loading which fails if files are deleted before finalize()
+    const addFilesPromises = pathsToZip.map(async (filePath) => {
+      await archiveFile(inputDirectory, filePath, archive)
+    })
 
-    // eslint-disable-next-line @typescript-eslint/no-floating-promises
-    archive.finalize()
+    // Wait for all files to be read and added before finalizing
+    Promise.all(addFilesPromises)
+      .then(() => {
+        // eslint-disable-next-line @typescript-eslint/no-floating-promises
+        archive.finalize()
+      })
+      .catch((error) => {
+        // eslint-disable-next-line @typescript-eslint/prefer-promise-reject-errors
+        reject(error)
+      })
   })
 }
 
@@ -84,6 +93,16 @@ function collectParentDirectories(fileRelativePath: string, accumulator: Set<str
   }
 }
 
+async function archiveFile(inputDirectory: string, filePath: string, archive: archiver.Archiver): Promise<void> {
+  const fileRelativePath = relativePath(inputDirectory, filePath)
+  if (!filePath || !fileRelativePath) return
+
+  // Read file content immediately to avoid race conditions
+  const fileContent = await readFile(filePath)
+  // Use append with Buffer instead of file() to avoid lazy file reading
+  archive.append(fileContent, {name: fileRelativePath})
+}
+
 export interface BrotliOptions {
   /**
    * The directory to compress.
@@ -147,15 +166,19 @@ export async function brotliCompress(options: BrotliOptions): Promise<void> {
         dot: true,
         followSymbolicLinks: false,
       })
-        .then((pathsToZip) => {
-          for (const filePath of pathsToZip) {
-            const fileRelativePath = relativePath(options.inputDirectory, filePath)
-            archive.file(filePath, {name: fileRelativePath})
-          }
+        .then(async (pathsToZip) => {
+          // Read all files immediately to prevent ENOENT errors during race conditions
+          const addFilesPromises = pathsToZip.map(async (filePath) => {
+            await archiveFile(options.inputDirectory, filePath, archive)
+          })
+
+          await Promise.all(addFilesPromises)
           // eslint-disable-next-line @typescript-eslint/no-floating-promises
           archive.finalize()
         })
-        .catch((error) => reject(error instanceof Error ? error : new Error(String(error))))
+        .catch((error) => {
+          reject(error instanceof Error ? error : new Error(String(error)))
+        })
     })
 
     const tarContent = readFileSync(tempTarPath)