From 2625473fdd2a9ee14149ff800d76f17dc3d59cbb Mon Sep 17 00:00:00 2001 From: doodlum <15017472+doodlum@users.noreply.github.com> Date: Mon, 5 Oct 2026 12:33:45 +0100 Subject: [PATCH] fix(mod-management): fail truncated archives as damaged, not as in use 7z reports a truncated or unreadable archive as "Cannot open the file as [7z] archive" with "Unexpected end of archive". isFileInUse matched the "cannot open", so extraction was retried three times, isCritical never got to "unexpected end of archive", and queryContinue's terminal check looked for the old "Can not open the file as archive" wording, so the "Archive damaged" dialog offered Continue, which installed nothing. Every way out of it ended as a user cancel. 7z names the archive type only once it has read the file, so that wording is now excluded from the file-in-use match, while the untyped "Cannot open the file as archive" that a locked file produces is still retried. The terminal check matches both wordings, case-insensitively. Fixes LAZ-1287 Co-Authored-By: Claude Opus 5.5 --- .../InstallManager.archiveErrors.test.ts | 200 ++++++++++++++++++ .../mod_management/InstallManager.ts | 12 +- 2 files changed, 210 insertions(+), 2 deletions(-) create mode 100644 src/renderer/src/extensions/mod_management/InstallManager.archiveErrors.test.ts diff --git a/src/renderer/src/extensions/mod_management/InstallManager.archiveErrors.test.ts b/src/renderer/src/extensions/mod_management/InstallManager.archiveErrors.test.ts new file mode 100644 index 0000000000..44cbb02d4a --- /dev/null +++ b/src/renderer/src/extensions/mod_management/InstallManager.archiveErrors.test.ts @@ -0,0 +1,200 @@ +/** + * LAZ-1287: how an extraction failure reported by 7z is classified. The first three error strings + * are 7-Zip 26.00's (the bundled 7z-bin) as node-7z collects them from real runs: a truncated .7z, + * a zero-filled .zip and an archive another process holds open with an exclusive lock. The + * localised one stands in for that lock on a non-English Windows, where 7z's own text stays English + * and only the system message is translated. + */ +import * as os from "node:os"; +import * as path from "node:path"; + +import { describe, expect, vi } from "vitest"; + +import type { IInstallManagerHarness } from "../../test-utils/harnessTypes"; +import { test as imTest } from "../../test-utils/installManagerTest"; +import { ArchiveBrokenError } from "../../util/CustomErrors"; + +vi.mock("../../util/log", () => { + const log = vi.fn(); + return { default: log, log }; +}); + +const ARCHIVE = "C:\\Users\\someone\\Downloads\\Skyland AIO-4.32.0.7z"; + +const TRUNCATED_7Z = + `ERROR: ${ARCHIVE}\r\n${ARCHIVE}\r\nOpen ERROR: Cannot open the file as [7z] archive\r\n` + + "\r\n\r\nERRORS:\r\nUnexpected end of archive\r\n"; +const NOT_AN_ARCHIVE = + `ERROR: ${ARCHIVE}\r\n${ARCHIVE}\r\nOpen ERROR: Cannot open the file as [zip] archive\r\n` + + "\r\n\r\nERRORS:\r\nIs not archive\r\n"; +const LOCKED = + `ERROR: ${ARCHIVE}\r\nCannot open the file as archive\r\n\r\n` + + "The process cannot access the file because it is being used by another process.\r\n"; +const LOCKED_LOCALISED = + `ERROR: ${ARCHIVE}\r\nCannot open the file as archive\r\n\r\n` + + "Der Prozess kann nicht auf die Datei zugreifen, da sie von einem anderen Prozess verwendet " + + "wird.\r\n"; + +interface IExtractResult { + code: number; + errors: string[]; +} + +/** The private members on the extraction path, reached through one cast. */ +interface IExtractionInternals { + installInner( + api: unknown, + archivePath: string, + tempPath: string, + destinationPath: string, + gameId: string, + installContext: undefined, + installationZip: unknown, + ): Promise; + extractWithRetry( + zip: unknown, + archivePath: string, + tempPath: string, + progress: () => void, + queryPassword: () => PromiseLike, + maxRetries?: number, + retryDelayMs?: number, + ): Promise; + queryContinue(api: unknown, errors: string[], archivePath: string): Promise; +} + +/** A 7z stand-in whose every run exits 2 with the given errors, as node-7z resolves it. */ +function failingZip(errors: string[]) { + return { extractFull: vi.fn(() => Promise.resolve({ code: 2, errors })) }; +} + +function tempDir(): string { + return path.join(os.tmpdir(), `vortex-laz1287-${process.pid}-${Math.random().toString(36)}`); +} + +/** Labels of the "Archive damaged" dialog's buttons, or undefined while none was shown. */ +function damagedDialogActions(h: IInstallManagerHarness): string[] | undefined { + const dialog = h.dispatched.find((action) => action.type === "SHOW_MODAL_DIALOG"); + return (dialog?.payload as { actions: string[] } | undefined)?.actions; +} + +/** Settles with the install's outcome, or with "dialog" once the dialog is up (it never settles). */ +async function outcomeOf(h: IInstallManagerHarness, install: Promise): Promise { + const dialogShown = vi + .waitFor( + () => { + if (damagedDialogActions(h) === undefined) { + throw new Error("no dialog yet"); + } + }, + { timeout: 8000, interval: 20 }, + ) + .then(() => "dialog"); + return Promise.race([ + install.then( + () => "installed", + (err: unknown) => err, + ), + dialogShown, + ]); +} + +describe("InstallManager extraction errors", () => { + imTest( + "a truncated .7z fails as a damaged archive at once, without retries or the dialog", + { timeout: 15000 }, + async ({ makeInstallManager }) => { + const h = makeInstallManager(); + const internals = h.manager as unknown as IExtractionInternals; + const zip = failingZip([TRUNCATED_7Z]); + + const install = internals.installInner( + h.api, + ARCHIVE, + tempDir(), + tempDir(), + "skyrimse", + undefined, + zip, + ); + + const outcome = await outcomeOf(h, install); + expect(outcome).toBeInstanceOf(ArchiveBrokenError); + expect(zip.extractFull).toHaveBeenCalledTimes(1); + expect(damagedDialogActions(h)).toBeUndefined(); + }, + ); + + imTest( + "a file 7z recognises but can't open is asked about once, without Continue", + { timeout: 15000 }, + async ({ makeInstallManager }) => { + const h = makeInstallManager(); + const internals = h.manager as unknown as IExtractionInternals; + const zip = failingZip([NOT_AN_ARCHIVE]); + + const install = internals.installInner( + h.api, + ARCHIVE, + tempDir(), + tempDir(), + "skyrimse", + undefined, + zip, + ); + + expect(await outcomeOf(h, install)).toBe("dialog"); + expect(zip.extractFull).toHaveBeenCalledTimes(1); + expect(damagedDialogActions(h)).toEqual(["Cancel", "Delete"]); + }, + ); + + imTest.for([ + ["in English", LOCKED], + ["on a non-English Windows", LOCKED_LOCALISED], + ])( + "an archive another process holds is retried %s", + async ([, error], { makeInstallManager }) => { + const h = makeInstallManager(); + const internals = h.manager as unknown as IExtractionInternals; + const zip = failingZip([error]); + + const result = await internals.extractWithRetry( + zip, + ARCHIVE, + tempDir(), + () => undefined, + () => Promise.resolve(""), + 3, + 0, + ); + + expect(zip.extractFull).toHaveBeenCalledTimes(4); + expect(result.code).toBe(2); + }, + ); + + imTest( + "an archive still held after the retries is not offered Continue", + async ({ makeInstallManager }) => { + const h = makeInstallManager(); + const internals = h.manager as unknown as IExtractionInternals; + + void internals.queryContinue(h.api, [LOCKED], ARCHIVE); + + expect(damagedDialogActions(h)).toEqual(["Cancel", "Delete"]); + }, + ); + + imTest( + "an archive that extracted with errors is still offered Continue", + async ({ makeInstallManager }) => { + const h = makeInstallManager(); + const internals = h.manager as unknown as IExtractionInternals; + + void internals.queryContinue(h.api, ["ERROR: CRC Failed : data.bin\r\n"], ARCHIVE); + + expect(damagedDialogActions(h)).toEqual(["Cancel", "Delete", "Continue"]); + }, + ); +}); diff --git a/src/renderer/src/extensions/mod_management/InstallManager.ts b/src/renderer/src/extensions/mod_management/InstallManager.ts index 90074e251b..36915958c8 100644 --- a/src/renderer/src/extensions/mod_management/InstallManager.ts +++ b/src/renderer/src/extensions/mod_management/InstallManager.ts @@ -280,6 +280,13 @@ export const VARIANT_ACTION = "Add Variant"; // exe is a self-extracting archive and we would be able to handle it const FILETYPES_AVOID = [".dll"]; +// 7z names the archive type ("Cannot open the file as [7z] archive") only after it has read the +// file and recognised its format, so that wording means the contents are unreadable, never that +// another process holds the file. Without a type ("Cannot open the file as archive") 7z could not +// identify the file at all, which is also how it reports a file it can't read. +const ARCHIVE_UNREADABLE_AS_TYPE = /can ?not open the file as \[[^\]]*\] archive/gi; +const ARCHIVE_UNREADABLE = /can ?not open the file as (\[[^\]]*\] )?archive/i; + function nop() { // nop } @@ -3850,7 +3857,7 @@ class InstallManager { if (errorCode && ["EBUSY", "EPERM", "EACCES"].includes(errorCode)) { return true; } - const lowered = errorMessage.toLowerCase(); + const lowered = errorMessage.replace(ARCHIVE_UNREADABLE_AS_TYPE, "").toLowerCase(); const patterns = [ "being used by another process", "locked by another process", @@ -4182,7 +4189,8 @@ class InstallManager { } private queryContinue(api: IExtensionApi, errors: string[], archivePath: string): Promise { - const terminal = errors.find((err) => err.indexOf("Can not open the file as archive") !== -1); + // nothing was extracted from an archive 7z couldn't open, so continuing would install nothing + const terminal = errors.some((err) => ARCHIVE_UNREADABLE.test(err)); return new Promise((resolve, reject) => { const actions = [