Avoid problems with bundle writing while compressing the bundle

alfonso-noriega · alfonso-noriega · commit a51564011fe8 · 2025-10-31T13:53:02.000+01:00
diff --git a/packages/cli-kit/src/public/node/archiver.integration.test.ts b/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) {
diff --git a/packages/cli-kit/src/public/node/archiver.ts b/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,43 @@ export async function zip(options: ZipOptions): Promise<void> {
       archive.append(Buffer.alloc(0), {name: dirName})
     }
 
-    for (const filePath of pathsToZip) {
+    // 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) => {
       const fileRelativePath = relativePath(inputDirectory, filePath)
-      if (filePath && fileRelativePath) archive.file(filePath, {name: fileRelativePath})
-    }
+      if (!filePath || !fileRelativePath) return
+
+      try {
+        // 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})
+        // eslint-disable-next-line @typescript-eslint/no-explicit-any, no-catch-all/no-catch-all
+      } catch (error: any) {
+        // If file was deleted during bundling, skip it gracefully
+        const errorMessage = error instanceof Error ? error.message : String(error)
+        if (errorMessage.includes('ENOENT')) {
+          outputDebug(outputContent`File was deleted during bundling, skipping: ${outputToken.path(fileRelativePath)}`)
+        } else {
+          outputDebug(
+            outputContent`Failed to read file for archive, skipping: ${outputToken.path(
+              fileRelativePath,
+            )} - ${outputToken.raw(errorMessage)}`,
+          )
+        }
+      }
+    })
 
-    // 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)
+      })
   })
 }
 
@@ -147,11 +177,33 @@ export async function brotliCompress(options: BrotliOptions): Promise<void> {
         dot: true,
         followSymbolicLinks: false,
       })
-        .then((pathsToZip) => {
-          for (const filePath of pathsToZip) {
+        .then(async (pathsToZip) => {
+          // Read all files immediately to prevent ENOENT errors during race conditions
+          const addFilesPromises = pathsToZip.map(async (filePath) => {
             const fileRelativePath = relativePath(options.inputDirectory, filePath)
-            archive.file(filePath, {name: fileRelativePath})
-          }
+            try {
+              // 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})
+              // eslint-disable-next-line @typescript-eslint/no-explicit-any, no-catch-all/no-catch-all
+            } catch (error: any) {
+              const errorMessage = error instanceof Error ? error.message : String(error)
+              if (errorMessage.includes('ENOENT')) {
+                outputDebug(
+                  outputContent`File was deleted during bundling, skipping: ${outputToken.path(fileRelativePath)}`,
+                )
+              } else {
+                outputDebug(
+                  outputContent`Failed to read file for tar archive, skipping: ${outputToken.path(
+                    fileRelativePath,
+                  )} - ${outputToken.raw(errorMessage)}`,
+                )
+              }
+            }
+          })
+
+          await Promise.all(addFilesPromises)
           // eslint-disable-next-line @typescript-eslint/no-floating-promises
           archive.finalize()
         })
