Skip to content

Commit 490af62

Browse files
alfonso-noriegaalfonso-noriega
committed
Avoid problems with bundle writing while compressing the bundle
1 parent 0810f84 commit 490af62

File tree

2 files changed

+149
-12
lines changed

2 files changed

+149
-12
lines changed

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

Lines changed: 90 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
import {zip, brotliCompress} from './archiver.js'
2-
import {fileExists, inTemporaryDirectory, mkdir, touchFile} from './fs.js'
2+
import {fileExists, inTemporaryDirectory, mkdir, touchFile, removeFile, readFile} from './fs.js'
33
import {joinPath, dirname} from './path.js'
44
import {exec} from './system.js'
5-
import {describe, expect, test} from 'vitest'
5+
import {describe, expect, test, vi} from 'vitest'
66
import StreamZip from 'node-stream-zip'
77
import brotli from 'brotli'
88
import fs from 'fs'
9+
import {glob} from './fs.js'
910

1011
describe('zip', () => {
1112
test('zips a directory', async () => {
@@ -57,6 +58,51 @@ describe('zip', () => {
5758
expect(expectedEntries.sort()).toEqual(archiveEntries.sort())
5859
})
5960
})
61+
62+
test('gracefully handles files deleted during archiving', async () => {
63+
await inTemporaryDirectory(async (tmpDir) => {
64+
// Given
65+
const zipPath = joinPath(tmpDir, 'output.zip')
66+
const outputDirectoryName = 'output'
67+
const outputDirectoryPath = joinPath(tmpDir, outputDirectoryName)
68+
const structure = ['file1.js', 'file2.js', 'file3.js']
69+
70+
await createFiles(structure, outputDirectoryPath)
71+
72+
const file2Path = joinPath(outputDirectoryPath, 'file2.js')
73+
74+
// Spy on readFile to simulate a file being deleted during archiving
75+
// We'll make readFile throw ENOENT for file2.js specifically
76+
const fsModule = await import('./fs.js')
77+
const originalReadFile = fsModule.readFile
78+
const readFileSpy = vi.spyOn(fsModule, 'readFile')
79+
readFileSpy.mockImplementation(async (path: string) => {
80+
if (path === file2Path) {
81+
const error: NodeJS.ErrnoException = new Error(`ENOENT: no such file or directory, open '${path}'`)
82+
error.code = 'ENOENT'
83+
throw error
84+
}
85+
return originalReadFile(path)
86+
})
87+
88+
// When - should not throw even though file2.js throws ENOENT
89+
await expect(
90+
zip({
91+
inputDirectory: outputDirectoryPath,
92+
outputZipPath: zipPath,
93+
}),
94+
).resolves.not.toThrow()
95+
96+
// Then - archive should contain remaining files only
97+
const archiveEntries = await readArchiveFiles(zipPath)
98+
expect(archiveEntries).toContain('file1.js')
99+
expect(archiveEntries).toContain('file3.js')
100+
// file2.js should not be in archive since readFile failed
101+
expect(archiveEntries).not.toContain('file2.js')
102+
103+
readFileSpy.mockRestore()
104+
})
105+
})
60106
})
61107

62108
describe('brotliCompress', () => {
@@ -141,6 +187,48 @@ describe('brotliCompress', () => {
141187
expect(fs.existsSync(joinPath(extractPath, 'test.json'))).toBeFalsy()
142188
})
143189
})
190+
191+
test('gracefully handles files deleted during compression', async () => {
192+
await inTemporaryDirectory(async (tmpDir) => {
193+
// Given
194+
const brotliPath = joinPath(tmpDir, 'output.br')
195+
const outputDirectoryName = 'output'
196+
const outputDirectoryPath = joinPath(tmpDir, outputDirectoryName)
197+
const structure = ['file1.js', 'file2.js', 'file3.js']
198+
199+
await createFiles(structure, outputDirectoryPath)
200+
201+
const file2Path = joinPath(outputDirectoryPath, 'file2.js')
202+
203+
// Spy on readFile to simulate a file being deleted during compression
204+
// We'll make readFile throw ENOENT for file2.js specifically
205+
const fsModule = await import('./fs.js')
206+
const originalReadFile = fsModule.readFile
207+
const readFileSpy = vi.spyOn(fsModule, 'readFile')
208+
readFileSpy.mockImplementation(async (path: string) => {
209+
if (path === file2Path) {
210+
const error: NodeJS.ErrnoException = new Error(`ENOENT: no such file or directory, open '${path}'`)
211+
error.code = 'ENOENT'
212+
throw error
213+
}
214+
return originalReadFile(path)
215+
})
216+
217+
// When - should not throw even though file2.js throws ENOENT
218+
await expect(
219+
brotliCompress({
220+
inputDirectory: outputDirectoryPath,
221+
outputPath: brotliPath,
222+
}),
223+
).resolves.not.toThrow()
224+
225+
// Then - compressed file should exist and be valid
226+
const exists = await fileExists(brotliPath)
227+
expect(exists).toBeTruthy()
228+
229+
readFileSpy.mockRestore()
230+
})
231+
})
144232
})
145233

146234
async function createFiles(structure: string[], directory: string) {

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

Lines changed: 59 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import {relativePath, joinPath, dirname} from './path.js'
2-
import {glob, removeFile} from './fs.js'
2+
import {glob, removeFile, readFile} from './fs.js'
33
import {outputDebug, outputContent, outputToken} from '../../public/node/output.js'
44
import archiver from 'archiver'
55
import {createWriteStream, readFileSync, writeFileSync} from 'fs'
@@ -64,13 +64,42 @@ export async function zip(options: ZipOptions): Promise<void> {
6464
archive.append(Buffer.alloc(0), {name: dirName})
6565
}
6666

67-
for (const filePath of pathsToZip) {
67+
// Read all files immediately before adding to archive to prevent ENOENT errors
68+
// Using archive.file() causes lazy loading which fails if files are deleted before finalize()
69+
const addFilesPromises = pathsToZip.map(async (filePath) => {
6870
const fileRelativePath = relativePath(inputDirectory, filePath)
69-
if (filePath && fileRelativePath) archive.file(filePath, {name: fileRelativePath})
70-
}
71+
if (!filePath || !fileRelativePath) return
72+
73+
try {
74+
// Read file content immediately to avoid race conditions
75+
const fileContent = await readFile(filePath)
76+
// Use append with Buffer instead of file() to avoid lazy file reading
77+
archive.append(fileContent, {name: fileRelativePath})
78+
// eslint-disable-next-line @typescript-eslint/no-explicit-any, no-catch-all/no-catch-all
79+
} catch (error: any) {
80+
// If file was deleted during bundling, skip it gracefully
81+
const errorMessage = error instanceof Error ? error.message : String(error)
82+
if (errorMessage.includes('ENOENT')) {
83+
outputDebug(
84+
outputContent`File was deleted during bundling, skipping: ${outputToken.path(fileRelativePath)}`,
85+
)
86+
} else {
87+
outputDebug(
88+
outputContent`Failed to read file for archive, skipping: ${outputToken.path(fileRelativePath)} - ${outputToken.raw(errorMessage)}`,
89+
)
90+
}
91+
}
92+
})
7193

72-
// eslint-disable-next-line @typescript-eslint/no-floating-promises
73-
archive.finalize()
94+
// Wait for all files to be read and added before finalizing
95+
Promise.all(addFilesPromises)
96+
.then(() => {
97+
// eslint-disable-next-line @typescript-eslint/no-floating-promises
98+
archive.finalize()
99+
})
100+
.catch((error) => {
101+
reject(error)
102+
})
74103
})
75104
}
76105

@@ -147,11 +176,31 @@ export async function brotliCompress(options: BrotliOptions): Promise<void> {
147176
dot: true,
148177
followSymbolicLinks: false,
149178
})
150-
.then((pathsToZip) => {
151-
for (const filePath of pathsToZip) {
179+
.then(async (pathsToZip) => {
180+
// Read all files immediately to prevent ENOENT errors during race conditions
181+
const addFilesPromises = pathsToZip.map(async (filePath) => {
152182
const fileRelativePath = relativePath(options.inputDirectory, filePath)
153-
archive.file(filePath, {name: fileRelativePath})
154-
}
183+
try {
184+
// Read file content immediately to avoid race conditions
185+
const fileContent = await readFile(filePath)
186+
// Use append with Buffer instead of file() to avoid lazy file reading
187+
archive.append(fileContent, {name: fileRelativePath})
188+
// eslint-disable-next-line @typescript-eslint/no-explicit-any, no-catch-all/no-catch-all
189+
} catch (error: any) {
190+
const errorMessage = error instanceof Error ? error.message : String(error)
191+
if (errorMessage.includes('ENOENT')) {
192+
outputDebug(
193+
outputContent`File was deleted during bundling, skipping: ${outputToken.path(fileRelativePath)}`,
194+
)
195+
} else {
196+
outputDebug(
197+
outputContent`Failed to read file for tar archive, skipping: ${outputToken.path(fileRelativePath)} - ${outputToken.raw(errorMessage)}`,
198+
)
199+
}
200+
}
201+
})
202+
203+
await Promise.all(addFilesPromises)
155204
// eslint-disable-next-line @typescript-eslint/no-floating-promises
156205
archive.finalize()
157206
})

0 commit comments

Comments
 (0)