diff --git a/docs/tool-design.md b/docs/tool-design.md index 303137375d..ea54ba4e3b 100644 --- a/docs/tool-design.md +++ b/docs/tool-design.md @@ -161,6 +161,9 @@ providers need not expose identical state machines. Shared command owners must retain the observed cancellation reason separately from physical failures and cleanup ownership. Carry that fact through failure wrapping so adapters can preserve the exact reason without interpreting it. + ZIP member streams retain the exact signal reason; hdiutil wrappers consume + the command owner's typed cancellation record and keep physical command + failures separate, even if a host error is named `AbortError`. Client cancellation alone does not prove native work stopped. For atomic publication, document the cancellation cutoff and preserve any committed output's publication receipt. diff --git a/src/artifacts/NativeDmgArtifactReader.ts b/src/artifacts/NativeDmgArtifactReader.ts index d11ceae997..a1ff070381 100644 --- a/src/artifacts/NativeDmgArtifactReader.ts +++ b/src/artifacts/NativeDmgArtifactReader.ts @@ -19,6 +19,7 @@ import { } from "../filesystem/RegularFile.js"; import { OwnedFileHandle } from "../filesystem/OwnedFileHandle.js"; import { + ExecFileOutputCancelledError, execFileOutput, execFileOutputFailure, } from "../process/ExecFileOutput.js"; @@ -73,6 +74,7 @@ class NativeDmgCommandFailure extends ArtifactReaderFailure { readonly stdout: string, options?: { readonly cause?: unknown; + readonly cancellation?: { readonly reason: unknown }; readonly processNotStarted?: boolean; readonly exitCode?: number; }, @@ -80,7 +82,14 @@ class NativeDmgCommandFailure extends ArtifactReaderFailure { super( failure.reason, message, - options?.cause === undefined ? undefined : { cause: options.cause }, + options === undefined + ? undefined + : { + ...(options.cause === undefined ? {} : { cause: options.cause }), + ...(options.cancellation === undefined + ? {} + : { cancellation: options.cancellation }), + }, ); this.processNotStarted = options?.processNotStarted ?? false; this.verifyDiagnostic = failure.verifyDiagnostic; @@ -108,12 +117,12 @@ const createSystemHost = ( return { stdout, stderr, exitCode: 0 }; } catch (cause: unknown) { const output = execFileOutputFailure(cause); - if (cause instanceof Error && cause.name === "AbortError") + if (cause instanceof ExecFileOutputCancelledError) throw new NativeDmgCommandFailure( { reason: "cancelled" }, "DMG operation cancelled", output?.stdout ?? "", - { cause }, + { cause, cancellation: { reason: cause.cause } }, ); const exitCode = typeof output?.code === "number" ? output.code : processExitCode(cause); @@ -665,12 +674,12 @@ const runChecked = async ( result = await host.run(arguments_, signal, options); } catch (cause: unknown) { if (cause instanceof ArtifactReaderFailure) throw cause; - if (cause instanceof Error && cause.name === "AbortError") + if (cause instanceof ExecFileOutputCancelledError) throw new NativeDmgCommandFailure( { reason: "cancelled" }, "DMG operation cancelled", execFileOutputFailure(cause)?.stdout ?? "", - { cause }, + { cause, cancellation: { reason: cause.cause } }, ); const output = execFileOutputFailure(cause); throw new NativeDmgCommandFailure( diff --git a/src/artifacts/NativeDmgProtocol.test.ts b/src/artifacts/NativeDmgProtocol.test.ts index 8bc29399ea..b1e5e3772a 100644 --- a/src/artifacts/NativeDmgProtocol.test.ts +++ b/src/artifacts/NativeDmgProtocol.test.ts @@ -1,3 +1,5 @@ +import { ExecFileOutputCancelledError } from "../process/ExecFileOutput.js"; +import { ArtifactReaderFailure } from "./ArtifactReader.js"; import { build } from "plist"; import { expect, it } from "vitest"; import { @@ -44,3 +46,41 @@ it("retains malformed successful hdiutil inventory output as a typed producer pa }, }); }); + +it("uses the typed command cancellation owner for injected hosts and preserves physical AbortError names", async () => { + for (const cancelled of [true, false]) { + const reason = { requested: "stop injected verify" }; + const physical = Object.assign(new Error("physical host failure"), { + name: "AbortError", + code: "EIO", + }); + const failureCause = cancelled + ? new ExecFileOutputCancelledError(reason) + : physical; + const reader = new NativeDmgArtifactReader( + "/fixture/image.dmg", + {}, + { + run: async () => { + throw failureCause; + }, + }, + ); + try { + const result = (async () => { + for await (const _entry of reader.entries()) { + /* consume */ + } + })(); + const failure: unknown = await result.catch((cause: unknown) => cause); + expect(failure).toBeInstanceOf(ArtifactReaderFailure); + if (!(failure instanceof ArtifactReaderFailure)) + throw new Error("Expected typed failure"); + expect(failure.reason).toBe(cancelled ? "cancelled" : "io"); + expect(failure.cause).toBe(failureCause); + expect(failure.cancellation?.reason).toBe(cancelled ? reason : undefined); + } finally { + await reader.close(); + } + } +}); diff --git a/src/artifacts/ZipArtifactReader.test.ts b/src/artifacts/ZipArtifactReader.test.ts index eb55c6b08f..fd8b687280 100644 --- a/src/artifacts/ZipArtifactReader.test.ts +++ b/src/artifacts/ZipArtifactReader.test.ts @@ -1,3 +1,4 @@ +import { translateArtifactFailure } from "./ArtifactOperationFailure.js"; import { writeOrderedZip } from "../../tests/fixtures/artifactEntryOrder.js"; import { zipSymlinkMetadataFixture } from "../../tests/fixtures/zipSymlinkMetadata.js"; import { ArtifactReaderFailure } from "./ArtifactReader.js"; @@ -388,3 +389,46 @@ it.each([null, { caller: "opaque ZIP interruption" }])( } }, ); + +it("retains exact caller cancellation after a real ZIP member opens and leaves an uncancelled read intact", async () => { + const directory = await createTestTempDirectory( + "rea-zip-stream-cancellation-", + ); + const path = join(directory, "fixture.zip"); + const bytes = new Uint8Array(512 * 1024).fill(0x71); + const writer = new ZipWriter(new Uint8ArrayWriter()); + await writer.add("member.bin", new Uint8ArrayReader(bytes), { level: 0 }); + await writeFile(path, await writer.close()); + const reader = new ZipArtifactReader(path, "zip"); + const reason = { requested: "stop member" }; + const controller = new AbortController(); + try { + const entries = []; + for await (const entry of reader.entries()) entries.push(entry); + const entry = entries[0]; + if (entry === undefined) throw new Error("Expected member"); + const stream = await reader.open(entry, controller.signal); + const consumption = (async () => { + for await (const _chunk of stream) controller.abort(reason); + })(); + const failure: unknown = await consumption.catch((cause: unknown) => cause); + expect(failure).toBeInstanceOf(ArtifactReaderFailure); + if (!(failure instanceof ArtifactReaderFailure)) + throw new Error("Expected cancellation"); + expect(failure.reason).toBe("cancelled"); + expect(failure.cancellation?.reason).toBe(reason); + expect(translateArtifactFailure("extract_artifact", failure).cause).toBe( + reason, + ); + await expect(reader.open(entry, controller.signal)).rejects.toMatchObject({ + reason: "cancelled", + cancellation: { reason }, + }); + const chunks: Buffer[] = []; + for await (const chunk of await reader.open(entry)) + chunks.push(Buffer.from(chunk)); + expect(Buffer.concat(chunks)).toEqual(Buffer.from(bytes)); + } finally { + await reader.close(); + } +}); diff --git a/src/artifacts/ZipArtifactReader.ts b/src/artifacts/ZipArtifactReader.ts index 39b814189f..3ddd544a6a 100644 --- a/src/artifacts/ZipArtifactReader.ts +++ b/src/artifacts/ZipArtifactReader.ts @@ -387,7 +387,9 @@ const extractStream = (entry: FileEntry, signal?: AbortSignal): Readable => { // against `getData` rejecting with its own AbortError. That ordering is what // keeps the caller-visible failure typed as `cancelled`. output.destroy( - new ArtifactReaderFailure("cancelled", "ZIP operation cancelled"), + new ArtifactReaderFailure("cancelled", "ZIP operation cancelled", { + cause: signal?.reason, + }), ); }; signal?.addEventListener("abort", onAbort, { once: true }); diff --git a/tests/process-global/dmgCancellationReason.test.ts b/tests/process-global/dmgCancellationReason.test.ts new file mode 100644 index 0000000000..f31caea071 --- /dev/null +++ b/tests/process-global/dmgCancellationReason.test.ts @@ -0,0 +1,95 @@ +import { access, readFile, rm } from "node:fs/promises"; +import { join } from "node:path"; +import { setTimeout as delay } from "node:timers/promises"; +import { expect, it, vi } from "vitest"; +import { createTestTempDirectory } from "../fixtures/temporaryDirectory.js"; + +const state = vi.hoisted(() => ({ marker: "", calls: 0 })); +vi.mock("../../src/process/ExecFileOutput.js", async (importOriginal) => { + const actual = + await importOriginal< + typeof import("../../src/process/ExecFileOutput.js") + >(); + return { + ...actual, + execFileOutput: ( + command: string, + args: string[], + options: Parameters[2], + ) => { + if (command !== "/usr/bin/hdiutil" || args[0] !== "verify") + throw new Error("Unexpected native command"); + state.calls += 1; + return actual.execFileOutput( + process.execPath, + [ + "-e", + 'require("node:fs").writeFileSync(process.argv[1], String(process.pid)); setInterval(() => {}, 1000)', + state.marker, + ], + options, + ); + }, + }; +}); +import { NativeDmgArtifactReader } from "../../src/artifacts/NativeDmgArtifactReader.js"; +import { ArtifactReaderFailure } from "../../src/artifacts/ArtifactReader.js"; +import { ExecFileOutputCancelledError } from "../../src/process/ExecFileOutput.js"; +import { translateArtifactFailure } from "../../src/artifacts/ArtifactOperationFailure.js"; + +it.skipIf(process.platform !== "darwin").each([false, true])( + "preserves exact DMG caller reason and settles the command before return (pre-aborted: %s)", + async (preAborted) => { + const root = await createTestTempDirectory("rea-dmg-cancellation-reason-"); + state.marker = join(root, "ready"); + state.calls = 0; + const reason = { requested: "stop verify" }; + const controller = new AbortController(); + if (preAborted) controller.abort(reason); + const reader = new NativeDmgArtifactReader("fixture.dmg", {}); + const result = (async () => { + for await (const _entry of reader.entries(controller.signal)) { + /* consume */ + } + })().catch((cause: unknown) => cause); + try { + if (!preAborted) { + const deadline = Date.now() + 5_000; + while ( + !(await access(state.marker).then( + () => true, + () => false, + )) + ) { + if (Date.now() > deadline) + throw new Error("Child readiness marker absent"); + await delay(10); + } + controller.abort(reason); + } + const failure = await result; + expect(failure).toBeInstanceOf(ArtifactReaderFailure); + if (!(failure instanceof ArtifactReaderFailure)) + throw new Error("Expected cancellation"); + expect(failure.reason).toBe("cancelled"); + expect(failure.cancellation?.reason).toBe(reason); + expect(translateArtifactFailure("inspect_artifact", failure).cause).toBe( + reason, + ); + expect(state.calls).toBe(preAborted ? 0 : 1); + if (!preAborted) { + expect(failure.cause).toBeInstanceOf(ExecFileOutputCancelledError); + expect((failure.cause as ExecFileOutputCancelledError).cause).toBe( + reason, + ); + const pid = Number(await readFile(state.marker, "utf8")); + expect(() => process.kill(pid, 0)).toThrow(); + } + } finally { + controller.abort(reason); + await result; + await reader.close(); + await rm(root, { recursive: true, force: true }); + } + }, +);