From ea92f157ff58ec058ca6cb497a4b9bb9ce253e5e Mon Sep 17 00:00:00 2001 From: Robin Genz Date: Wed, 21 Oct 2026 07:56:53 +0200 Subject: [PATCH] fix(apps:liveupdates:upload): report files that vanish while zipping as user error Co-Authored-By: Claude Fable 5.1 --- src/utils/file.ts | 7 +++++-- src/utils/zip.test.ts | 37 +++++++++++++++++++++++++++++++++++++ src/utils/zip.ts | 20 +++++++++++++++++++- 3 files changed, 61 insertions(+), 3 deletions(-) create mode 100644 src/utils/zip.test.ts diff --git a/src/utils/file.ts b/src/utils/file.ts index 08ecd13..b718db9 100644 --- a/src/utils/file.ts +++ b/src/utils/file.ts @@ -4,7 +4,7 @@ import pathModule from 'path'; import { createBufferFromPath } from './buffer.js'; import { getCodeFromUnknownError, UserError } from './error.js'; -const concurrentModificationHint = +export const concurrentModificationHint = 'Make sure that no other process (e.g. a build or file sync client) modifies the folder while the command is running.'; const readPermissionHint = 'Make sure that you have permission to read the file.'; const writePermissionHint = 'Make sure that you have permission to write to the folder or choose a different path.'; @@ -65,12 +65,15 @@ export const readFileFromDirectory = async (path: string): Promise => { const code = getCodeFromUnknownError(error); const hint = code ? unreadableFileErrorHints[code] : undefined; if (hint) { - throw new UserError(`The file could not be read: ${path}. ${hint}`); + throw createUnreadableFileError(path, hint); } throw error; } }; +export const createUnreadableFileError = (path: string, hint: string): UserError => + new UserError(`The file could not be read: ${path}. ${hint}`); + export const directoryContainsSymlinks = async (path: string): Promise => { const dirEntries = await fs.promises.readdir(path, { withFileTypes: true, recursive: true }).catch(() => []); return dirEntries.some((dirEntry) => dirEntry.isSymbolicLink()); diff --git a/src/utils/zip.test.ts b/src/utils/zip.test.ts new file mode 100644 index 0000000..31653bb --- /dev/null +++ b/src/utils/zip.test.ts @@ -0,0 +1,37 @@ +import { describe, expect, it, vi } from 'vitest'; +import { UserError } from './error.js'; +import zip from './zip.js'; + +const { mockAddLocalFolder } = vi.hoisted(() => ({ mockAddLocalFolder: vi.fn() })); + +vi.mock('adm-zip', () => ({ + default: function () { + return { addLocalFolder: mockAddLocalFolder, getEntries: () => [], toBuffer: () => Buffer.alloc(0) }; + }, +})); + +describe('zipFolder', () => { + it('should throw a user error if a file vanished while zipping', async () => { + mockAddLocalFolder.mockImplementationOnce(() => { + throw new Error('ADM-ZIP: File not found: "www/index.html"'); + }); + + const promise = zip.zipFolder('www'); + + await expect(promise).rejects.toThrow(UserError); + await expect(promise).rejects.toThrow( + 'The file could not be read: www/index.html. Make sure that no other process', + ); + }); + + it('should rethrow other errors', async () => { + mockAddLocalFolder.mockImplementationOnce(() => { + throw new Error('ADM-ZIP: Invalid filename'); + }); + + const promise = zip.zipFolder('www'); + + await expect(promise).rejects.not.toThrow(UserError); + await expect(promise).rejects.toThrow('ADM-ZIP: Invalid filename'); + }); +}); diff --git a/src/utils/zip.ts b/src/utils/zip.ts index c5a7baa..6cb977c 100644 --- a/src/utils/zip.ts +++ b/src/utils/zip.ts @@ -1,4 +1,5 @@ import { UserError } from '@/utils/error.js'; +import { concurrentModificationHint, createUnreadableFileError } from '@/utils/file.js'; import AdmZip from 'adm-zip'; import { globby } from 'globby'; import path from 'path'; @@ -20,7 +21,15 @@ class ZipImpl implements Zip { async zipFolder(sourceFolder: string): Promise { const zip = new AdmZip(); - zip.addLocalFolder(sourceFolder); + try { + zip.addLocalFolder(sourceFolder); + } catch (error) { + const vanishedFilePath = getVanishedFilePath(error); + if (vanishedFilePath) { + throw createUnreadableFileError(vanishedFilePath, concurrentModificationHint); + } + throw error; + } // Strip external file attributes from every entry. `addLocalFolder` copies // the source's Unix mode bits onto each entry, so a directory with e.g. // `0500` on a CI checkout ends up in the bundle and makes `zip4j` on @@ -75,6 +84,15 @@ class ZipImpl implements Zip { } } +// adm-zip lists all files first and reads them afterwards. A file that was +// deleted in between is reported with this message and without an error code. +const getVanishedFilePath = (error: unknown): string | undefined => { + if (!(error instanceof Error)) { + return undefined; + } + return error.message.match(/^ADM-ZIP: File not found: "(.+)"$/)?.[1]; +}; + const zip: Zip = new ZipImpl(); export default zip;