fix(artifacts): honor cancellation at the output publication boundary
This commit is contained in:
parent
9049dbaf0c
commit
e7882b34f4
|
|
@ -280,9 +280,11 @@ export class SafeOutputTree {
|
|||
}
|
||||
|
||||
/** Sync the owned output tree and prevent further writes through this instance. */
|
||||
async commit(): Promise<void> {
|
||||
async commit(signal?: AbortSignal): Promise<void> {
|
||||
this.#assertWritable();
|
||||
abortIfNeeded(signal);
|
||||
await this.#assertOwnedDirectories();
|
||||
abortIfNeeded(signal);
|
||||
// Windows has no directory fsync; file contents are already synced in write().
|
||||
if (this.platform === "win32") {
|
||||
this.#published = true;
|
||||
|
|
@ -315,6 +317,7 @@ export class SafeOutputTree {
|
|||
);
|
||||
await output.handle.sync();
|
||||
await parent.handle.sync();
|
||||
abortIfNeeded(signal);
|
||||
this.#published = true;
|
||||
} catch (cause: unknown) {
|
||||
sync = { kind: "failed", cause };
|
||||
|
|
@ -336,23 +339,26 @@ export class SafeOutputTree {
|
|||
closeFailure,
|
||||
this.#outputRoot,
|
||||
);
|
||||
if (sync.kind === "failed")
|
||||
throw ArtifactReaderFailure.withCleanup(
|
||||
new ArtifactReaderFailure(
|
||||
"path",
|
||||
"Could not durably sync extraction output",
|
||||
{ cause: sync.cause },
|
||||
),
|
||||
cleanup,
|
||||
);
|
||||
if (sync.kind === "failed") {
|
||||
const primary = isCancellationFailure(sync.cause)
|
||||
? sync.cause
|
||||
: new ArtifactReaderFailure(
|
||||
"path",
|
||||
"Could not durably sync extraction output",
|
||||
{ cause: sync.cause },
|
||||
);
|
||||
throw ArtifactReaderFailure.withCleanup(primary, cleanup);
|
||||
}
|
||||
throw ArtifactReaderFailure.withCleanup(closeFailure, cleanup);
|
||||
}
|
||||
if (sync.kind === "failed")
|
||||
if (sync.kind === "failed") {
|
||||
if (isCancellationFailure(sync.cause)) throw sync.cause;
|
||||
throw new ArtifactReaderFailure(
|
||||
"path",
|
||||
"Could not durably sync extraction output",
|
||||
{ cause: sync.cause },
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/** Remove only this operation's unsealed tree and verify absence. */
|
||||
|
|
@ -590,6 +596,11 @@ const abortIfNeeded = (signal?: AbortSignal): void => {
|
|||
);
|
||||
};
|
||||
|
||||
const isCancellationFailure = (
|
||||
cause: unknown,
|
||||
): cause is ArtifactReaderFailure =>
|
||||
cause instanceof ArtifactReaderFailure && cause.reason === "cancelled";
|
||||
|
||||
const isAlreadyExists = (cause: unknown): boolean =>
|
||||
cause instanceof Error && "code" in cause && cause.code === "EEXIST";
|
||||
|
||||
|
|
|
|||
|
|
@ -249,7 +249,7 @@ const materializeSelection = async ({
|
|||
compareUnicodeCodePoints(left.relative_path, right.relative_path),
|
||||
);
|
||||
completed = createExtractionResult(input, inventory, selected, extracted);
|
||||
await output.commit();
|
||||
await output.commit(signal);
|
||||
return completed;
|
||||
} catch (cause: unknown) {
|
||||
const cleanupFailures = await releaseExtractionOwners(
|
||||
|
|
|
|||
|
|
@ -113,7 +113,7 @@ export const publishWebScripts = async (
|
|||
options.signal,
|
||||
);
|
||||
options.signal?.throwIfAborted();
|
||||
await tree.commit();
|
||||
await tree.commit(options.signal);
|
||||
return result;
|
||||
} catch (cause: unknown) {
|
||||
if (cause instanceof SafeOutputTreeCreationFailure) tree = cause.tree;
|
||||
|
|
|
|||
|
|
@ -96,7 +96,7 @@ export const publishFirmwareExtraction = async (context: {
|
|||
published_bytes: files.reduce((sum, file) => sum + file.size, 0),
|
||||
});
|
||||
context.signal.throwIfAborted();
|
||||
await tree.commit();
|
||||
await tree.commit(context.signal);
|
||||
return result;
|
||||
} catch (cause: unknown) {
|
||||
if (cause instanceof SafeOutputTreeCreationFailure) tree = cause.tree;
|
||||
|
|
|
|||
|
|
@ -139,7 +139,7 @@ export class WakaruProvider implements JavaScriptRecoveryPort {
|
|||
await rm(root, { recursive: true, force: true });
|
||||
root = undefined;
|
||||
checkRecoveryDeadline(deadline, options?.signal);
|
||||
await tree.commit();
|
||||
await tree.commit(options?.signal);
|
||||
} catch (cause: unknown) {
|
||||
failed = { cause };
|
||||
}
|
||||
|
|
|
|||
|
|
@ -10,6 +10,8 @@ const injectedFilesystemFailures = vi.hoisted(() => ({
|
|||
closeNext: false,
|
||||
blockCloseRetries: false,
|
||||
chmodNext: false,
|
||||
syncAbortAfter: undefined as AbortController | undefined,
|
||||
syncCount: 0,
|
||||
rootIdentityPath: undefined as string | undefined,
|
||||
}));
|
||||
|
||||
|
|
@ -44,6 +46,15 @@ vi.mock("node:fs/promises", async (importOriginal) => {
|
|||
await close();
|
||||
};
|
||||
}
|
||||
if (injectedFilesystemFailures.syncAbortAfter !== undefined) {
|
||||
const sync = handle.sync.bind(handle);
|
||||
handle.sync = async () => {
|
||||
await sync();
|
||||
injectedFilesystemFailures.syncCount += 1;
|
||||
if (injectedFilesystemFailures.syncCount === 2)
|
||||
injectedFilesystemFailures.syncAbortAfter?.abort();
|
||||
};
|
||||
}
|
||||
return handle;
|
||||
},
|
||||
};
|
||||
|
|
@ -82,6 +93,34 @@ describe("safe artifact output close failures", () => {
|
|||
},
|
||||
);
|
||||
|
||||
it.skipIf(process.platform === "win32")(
|
||||
"keeps the tree unpublished when cancellation arrives during commit durability",
|
||||
async () => {
|
||||
const parent = await createTestTempDirectory("rea-safe-output-cancel-");
|
||||
const output = join(parent, "unpublished");
|
||||
const tree = await SafeOutputTree.create(output);
|
||||
const bytes = Buffer.from("pending output");
|
||||
await tree.write("file.txt", Readable.from(bytes), {
|
||||
sha256: createHash("sha256").update(bytes).digest("hex"),
|
||||
bytes: bytes.byteLength,
|
||||
});
|
||||
const controller = new AbortController();
|
||||
injectedFilesystemFailures.syncCount = 0;
|
||||
injectedFilesystemFailures.syncAbortAfter = controller;
|
||||
try {
|
||||
await expect(tree.commit(controller.signal)).rejects.toMatchObject({
|
||||
reason: "cancelled",
|
||||
});
|
||||
expect(tree.published).toBe(false);
|
||||
} finally {
|
||||
injectedFilesystemFailures.syncAbortAfter = undefined;
|
||||
injectedFilesystemFailures.syncCount = 0;
|
||||
}
|
||||
expect(await tree.rollback()).toMatchObject({ status: "complete" });
|
||||
await expect(access(output)).rejects.toMatchObject({ code: "ENOENT" });
|
||||
},
|
||||
);
|
||||
|
||||
it.skipIf(process.platform === "win32")(
|
||||
"preserves setup failure when closing the staging handle also fails",
|
||||
async () => {
|
||||
|
|
|
|||
|
|
@ -3,7 +3,7 @@ import { access, mkdir, readFile, rm, writeFile } from "node:fs/promises";
|
|||
import { join } from "node:path";
|
||||
|
||||
import { createPackage, createPackageWithOptions } from "@electron/asar";
|
||||
import { expect, it } from "vitest";
|
||||
import { expect, it, vi } from "vitest";
|
||||
|
||||
import { ArtifactProvider } from "../../../../src/artifacts/ArtifactProvider.js";
|
||||
import {
|
||||
|
|
@ -13,6 +13,7 @@ import {
|
|||
import { artifactExtractionResultSchema } from "../../../../src/domain/artifactGraph.js";
|
||||
import { projectAnalysisError } from "../../../../src/domain/analysisErrorProjection.js";
|
||||
import { createTestTempDirectory } from "../../../fixtures/temporaryDirectory.js";
|
||||
import { DirectoryArtifactReader } from "../../../../src/artifacts/DirectoryArtifactReader.js";
|
||||
|
||||
it.each(["file", "directory"] as const)(
|
||||
"rolls back when an inventoried %s disappears before extraction",
|
||||
|
|
@ -61,6 +62,42 @@ it("still refuses a regular file added after inventory and rolls back", async ()
|
|||
await expect(access(output)).rejects.toThrow();
|
||||
});
|
||||
|
||||
it("rolls back when cancellation arrives while closing the materialization reader", async () => {
|
||||
const root = await createTestTempDirectory("rea-extract-cancel-close-");
|
||||
const source = join(root, "source");
|
||||
await mkdir(source);
|
||||
await writeFile(join(source, "payload.txt"), "complete payload\n");
|
||||
const snapshot = await scanArtifactInventory(source);
|
||||
const output = join(root, "output");
|
||||
const controller = new AbortController();
|
||||
const close = DirectoryArtifactReader.prototype.close;
|
||||
const closeSpy = vi
|
||||
.spyOn(DirectoryArtifactReader.prototype, "close")
|
||||
.mockImplementation(async function (this: DirectoryArtifactReader) {
|
||||
controller.abort();
|
||||
await close.call(this);
|
||||
});
|
||||
try {
|
||||
await expect(
|
||||
materializeArtifactInventory(
|
||||
{
|
||||
inputPath: source,
|
||||
inputFormat: "asar",
|
||||
outputRoot: output,
|
||||
environment: process.env,
|
||||
integrityPolicy: "fail",
|
||||
},
|
||||
source,
|
||||
snapshot,
|
||||
controller.signal,
|
||||
),
|
||||
).rejects.toMatchObject({ reason: "cancelled" });
|
||||
await expect(access(output)).rejects.toThrow();
|
||||
} finally {
|
||||
closeSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it("preserves every equal-content occurrence and copies a nested ASAR as its containing file", async () => {
|
||||
const root = await createTestTempDirectory("rea-extract-nested-");
|
||||
const source = join(root, "source");
|
||||
|
|
|
|||
Loading…
Reference in New Issue