diff --git a/src/captures/TsharkCaptureProvider.ts b/src/captures/TsharkCaptureProvider.ts index a42413b2cd..8acada8e5a 100644 --- a/src/captures/TsharkCaptureProvider.ts +++ b/src/captures/TsharkCaptureProvider.ts @@ -447,8 +447,18 @@ export class TsharkCaptureProvider implements PacketCapturePort { if (cause instanceof AnalysisError) return cause; if (cause instanceof ArtifactReaderFailure && cause.cleanup !== undefined) return this.#resources.cleanupFailure( - cause.cleanup, cause, + (primary) => + this.#failure( + primary, + phase, + path, + workspacePath, + effectiveLimits, + observedOutput, + operation, + limits, + ), observedOutput, operation, ); diff --git a/src/process/ArtifactAnalysisResources.test.ts b/src/process/ArtifactAnalysisResources.test.ts index ede5260ab8..1536c0a4ae 100644 --- a/src/process/ArtifactAnalysisResources.test.ts +++ b/src/process/ArtifactAnalysisResources.test.ts @@ -1,3 +1,10 @@ +import { AnalysisCleanupLease } from "../artifacts/AnalysisCleanupLease.js"; +import { ArtifactResourceRetryFailure } from "../artifacts/ArtifactResourceScope.js"; +import { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js"; +import { withArtifactRegularFile } from "../artifacts/ArtifactFileOperation.js"; +import { writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import { createTestTempDirectory } from "../../tests/fixtures/temporaryDirectory.js"; import { OwnedCommandFailure } from "./OwnedCommand.js"; import { EventEmitter } from "node:events"; import { PassThrough } from "node:stream"; @@ -76,17 +83,27 @@ it("projects the specific reason and member evidence for retained cleanup", asyn "process-group:4242", ], diagnostics: { - reason: "process ownership token could not be read for 1 live process", - failures: [ - { - pid: 101, - reason: "environment-unreadable", - diagnostic: "sysctl_errno_22", + cleanup_error: { + details: { + diagnostics: { + reason: + "process ownership token could not be read for 1 live process", + failures: [ + { + pid: 101, + reason: "environment-unreadable", + diagnostic: "sysctl_errno_22", + }, + ], + unverified: [ + { + pid: 900, + diagnostic: "platform_binary_environment_withheld", + }, + ], + }, }, - ], - unverified: [ - { pid: 900, diagnostic: "platform_binary_environment_withheld" }, - ], + }, }, }, }); @@ -142,7 +159,7 @@ it("preserves the primary error and resolves cleanup status after retry", async expect(result.error.partialObservation).toEqual(primary.partialObservation); expect(result.error.cleanupIncomplete).toBe(true); expect(result.error.diagnostics).toMatchObject({ - previous_error: { code: "unreadable_output" }, + primary_error: { code: "unreadable_output" }, }); await resources.release(); @@ -220,20 +237,22 @@ it("resolves an artifact cleanup error when its scope owner later closes", async const owner = { kind: "lease", resource: "fixture descriptor", - lease: { - release: async () => { - closes++; - if (closes === 1) throw new Error("descriptor remains open"); - }, - }, + lease: new AnalysisCleanupLease(async () => { + closes++; + if (closes === 1) throw new Error("descriptor remains open"); + }, ["fixture descriptor"]), } as const; expect(await scope.release(owner)).toMatchObject({ kind: "failed" }); - throw new ArtifactReaderFailure("io", "Descriptor close failed", { - cleanup: { - reason: "descriptor remains open", - resources: ["fixture descriptor"], + throw new ArtifactResourceRetryFailure( + [{ owner, lease: owner.lease }], + "Descriptor close failed", + { + cleanup: { + reason: "descriptor remains open", + resources: ["fixture descriptor"], + }, }, - }); + ); } return { bytes: Buffer.alloc(0), sha256: "0".repeat(64) }; }; @@ -276,21 +295,23 @@ it("keeps a runtime root until its descriptor cleanup owner is released", async const owner = { kind: "lease", resource: "fixture descriptor", - lease: { - release: async () => { - descriptorCloses++; - if (descriptorCloses === 1) - throw new Error("descriptor remains open"); - }, - }, + lease: new AnalysisCleanupLease(async () => { + descriptorCloses++; + if (descriptorCloses === 1) + throw new Error("descriptor remains open"); + }, ["fixture descriptor"]), } as const; expect(await scope.release(owner)).toMatchObject({ kind: "failed" }); - throw new ArtifactReaderFailure("io", "Descriptor close failed", { - cleanup: { - reason: "descriptor remains open", - resources: ["fixture descriptor"], + throw new ArtifactResourceRetryFailure( + [{ owner, lease: owner.lease }], + "Descriptor close failed", + { + cleanup: { + reason: "descriptor remains open", + resources: ["fixture descriptor"], + }, }, - }); + ); } return { bytes: Buffer.alloc(0), sha256: "0".repeat(64) }; }; @@ -418,6 +439,9 @@ it.each([false, true])( "preserves artifact cancellation and independent failed cleanup (failed close: %s)", async (failedClose) => { const reason = { request: "artifact cancellation" }; + const root = await createTestTempDirectory("rea-resource-cancel-"); + const path = join(root, "descriptor"); + await writeFile(path, "authored bytes"); let mayClose = false; const readArtifact: typeof readStableArtifact = async ( _path, @@ -426,31 +450,18 @@ it.each([false, true])( _fs, scope, ) => { - if (failedClose) { - if (scope === undefined) - throw new Error("Expected retained file scope"); - const retained = { - kind: "lease", - resource: "descriptor", - lease: { - release: async () => { - if (!mayClose) - throw new Error("injected descriptor close failure"); - }, - }, - } as const; - expect(await scope.release(retained)).toMatchObject({ kind: "failed" }); - } - throw new ArtifactReaderFailure("cancelled", "read cancelled", { + const primary = new ArtifactReaderFailure("cancelled", "read cancelled", { cause: reason, - ...(failedClose - ? { - cleanup: { - reason: "injected descriptor close failure", - resources: ["descriptor"], - }, - } - : {}), + }); + if (!failedClose) throw primary; + if (scope === undefined) throw new Error("Expected retained file scope"); + return withArtifactRegularFile(path, scope, _signal, async (file) => { + const close = file.close.bind(file); + file.close = async () => { + if (!mayClose) throw new Error("injected descriptor close failure"); + await close(); + }; + throw primary; }); }; const resources = new ArtifactAnalysisResources( @@ -569,19 +580,24 @@ it("observes cancellation after pending descriptor cleanup before admitting repl const retained = { kind: "lease", resource: "descriptor", - lease: { - release: async () => { - closes++; - if (closes === 1) throw new Error("descriptor close denied"); - cleanupStarted(); - await held; - }, - }, + lease: new AnalysisCleanupLease(async () => { + closes++; + if (closes === 1) throw new Error("descriptor close denied"); + cleanupStarted(); + await held; + }, ["descriptor"]), } as const; await scope.release(retained); - throw new ArtifactReaderFailure("io", "descriptor cleanup failed", { - cleanup: { reason: "descriptor close denied", resources: ["descriptor"] }, - }); + throw new ArtifactResourceRetryFailure( + [{ owner: retained, lease: retained.lease }], + "descriptor cleanup failed", + { + cleanup: { + reason: "descriptor close denied", + resources: ["descriptor"], + }, + }, + ); }; const resources = new ArtifactAnalysisResources( "fixture", @@ -667,3 +683,218 @@ it.each([false, true])( } }, ); + +it("keeps cleanup with no reported capability unresolved instead of clearing it from an empty scope", async () => { + const resources = new ArtifactAnalysisResources( + "fixture", + "inspect_signal_decoders", + readStableArtifact, + readLinuxResourceLimits, + { message: "busy", details: {} }, + ); + const source = new ArtifactReaderFailure("io", "unverified cleanup", { + cleanup: { + reason: "no cleanup capability reported", + resources: ["unverified-resource"], + }, + }); + const result = await resources.run(async () => { + throw source; + }); + if (result.ok) throw new Error("Expected unresolved cleanup"); + expect(result.error.cleanupIncomplete).toBe(true); + expect(result.error.cleanupResources).toEqual(["unverified-resource"]); + let calls = 0; + const blocked = await resources.run(async () => { + calls++; + return ok("Must not run"); + }); + expect(blocked).toMatchObject({ + ok: false, + error: { cleanupIncomplete: true }, + }); + expect(calls).toBe(0); + await expect(resources.close()).rejects.toMatchObject({ + cleanupIncomplete: true, + }); + expect(result.error.cleanupIncomplete).toBe(true); + // No OS resource was supplied. An unresolved observation never grants cleanup authority. +}); + +it("joins mixed runtime cleanup and resolves each error only through its exact lease", async () => { + const resources = new ArtifactAnalysisResources( + "fixture", + "inspect_signal_decoders", + readStableArtifact, + readLinuxResourceLimits, + { message: "busy", details: {} }, + ); + let denied = true; + let healthyCloses = 0; + const bad = { + path: "bad-root", + close: async () => { + if (denied) throw new Error("bad root denied"); + }, + }; + const good = { + path: "good-root", + close: async () => { + healthyCloses++; + }, + }; + resources.retain(bad); + resources.retain(good); + try { + const failure: unknown = await resources + .release() + .catch((cause: unknown) => cause); + expect(failure).toMatchObject({ + cleanupIncomplete: true, + cleanupResources: ["bad-root"], + }); + expect(healthyCloses).toBe(1); + expect(resources.pendingPaths).toEqual(["bad-root"]); + denied = false; + await resources.close(); + expect(failure).toMatchObject({ + cleanupIncomplete: false, + cleanupResources: [], + }); + expect(healthyCloses).toBe(1); + } finally { + denied = false; + await resources.close(); + } +}); + +it("retains producer output when a completed read is cancelled during failed close", async () => { + const root = await createTestTempDirectory("rea-completed-read-cancel-"); + const path = join(root, "descriptor"); + await writeFile(path, "authored bytes"); + const caller = new AbortController(); + const reason = { request: "cancel during close" }; + const scope = new ArtifactResourceScope(); + const resources = new ArtifactAnalysisResources( + "fixture", + "inspect_signal_decoders", + readStableArtifact, + readLinuxResourceLimits, + { message: "busy", details: {} }, + ); + let mayClose = false; + const output = { + stdout: "completed producer output", + stderr: "", + truncated: false, + exit_code: 0, + }; + try { + const cause: unknown = await withArtifactRegularFile( + path, + scope, + caller.signal, + async (file) => { + const close = file.close.bind(file); + file.close = async () => { + caller.abort(reason); + if (!mayClose) throw new Error("controlled descriptor close failure"); + await close(); + }; + return await file.readFile(); + }, + ).catch((cause: unknown) => cause); + if (!(cause instanceof ArtifactReaderFailure)) + throw new Error("Expected retained artifact failure"); + const error = resources.cleanupFailure( + cause, + () => { + throw new Error("Completed read has no physical failure"); + }, + output, + ); + const primary = error.cleanupPrimary; + expect(primary).toMatchObject({ + _tag: "AnalysisCancelledError", + cause: reason, + capturedOutput: output, + }); + expect(error.capturedOutput).toEqual(output); + expect(error.cleanupIncomplete).toBe(true); + await expect(scope.close()).rejects.toMatchObject({ + reason: "unavailable", + }); + mayClose = true; + await scope.close(); + expect(error.cleanupIncomplete).toBe(false); + expect(error.cleanupPrimary).toBe(primary); + expect(error.capturedOutput).toEqual(output); + expect(projectAnalysisError(error)).toMatchObject( + projectAnalysisError(primary!), + ); + } finally { + mayClose = true; + await scope.close(); + await resources.close(); + } +}); + +it("keeps aggregate cleanup live through partial recovery without reclosing healthy roots", async () => { + const resources = new ArtifactAnalysisResources( + "fixture", + "inspect_signal_decoders", + readStableArtifact, + readLinuxResourceLimits, + { message: "busy", details: {} }, + ); + const denied = new Set(["first", "second"]); + const closes = new Map(); + for (const path of ["first", "healthy", "second"]) + resources.retain({ + path, + close: async () => { + closes.set(path, (closes.get(path) ?? 0) + 1); + if (denied.has(path)) throw new Error(`Close denied: ${path}`); + }, + }); + try { + const original: unknown = await resources + .release() + .catch((cause: unknown) => cause); + expect(original).toMatchObject({ + cleanupIncomplete: true, + cleanupResources: ["first", "second"], + }); + expect(closes.get("healthy")).toBe(1); + denied.delete("first"); + const retry: unknown = await resources + .release() + .catch((cause: unknown) => cause); + expect(original).toMatchObject({ + cleanupIncomplete: true, + cleanupResources: ["second"], + }); + expect(retry).toMatchObject({ + cleanupIncomplete: true, + cleanupResources: ["second"], + }); + expect(resources.pendingPaths).toEqual(["second"]); + denied.clear(); + await resources.close(); + for (const error of [original, retry]) + expect(error).toMatchObject({ + cleanupIncomplete: false, + cleanupResources: [], + }); + expect(closes).toEqual( + new Map([ + ["first", 2], + ["healthy", 1], + ["second", 3], + ]), + ); + } finally { + denied.clear(); + await resources.close(); + } +}); diff --git a/src/process/ArtifactAnalysisResources.ts b/src/process/ArtifactAnalysisResources.ts index ad1277fc53..c8674ebe83 100644 --- a/src/process/ArtifactAnalysisResources.ts +++ b/src/process/ArtifactAnalysisResources.ts @@ -1,6 +1,16 @@ -import { artifactFileCancellationCause } from "../artifacts/ArtifactFileOperation.js"; +import { + AnalysisCleanupLease, + AnalysisCleanupLeaseGroup, +} from "../artifacts/AnalysisCleanupLease.js"; +import { + ArtifactFileCleanupFailure, + artifactFileCancellationCause, +} from "../artifacts/ArtifactFileOperation.js"; import { ArtifactReaderFailure } from "../artifacts/ArtifactReader.js"; -import { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js"; +import { + ArtifactResourceScope, + ArtifactResourceRetryFailure, +} from "../artifacts/ArtifactResourceScope.js"; import { readStableArtifact } from "../artifacts/readStableArtifact.js"; import { AnalysisError, @@ -43,14 +53,7 @@ export class ArtifactAnalysisResources { { readonly owner: ProviderProcessSupervisor | undefined; readonly operation: string; - } - >(); - readonly #cleanupFailures = new Map< - AnalysisError, - { - readonly roots: Set; - readonly primaryFailure: AnalysisError | undefined; - readonly scopeOwner: boolean; + readonly lease: AnalysisCleanupLease; } >(); #tail: Promise = Promise.resolve(); @@ -101,7 +104,6 @@ export class ArtifactAnalysisResources { // Failed descriptor cleanup still belongs to the preceding request until // this scope has drained it and admits acquisition for the new request. this.#artifacts.operation = operation; - this.#resolveCleanupFailures(); if (signal?.aborted) return Promise.resolve( err( @@ -140,10 +142,18 @@ export class ArtifactAnalysisResources { root: ArtifactAnalysisRuntime, owner?: ProviderProcessSupervisor, ): void { + const retained = this.#pending.get(root); + const resources = () => this.#resourcesFor(root); this.#pending.set(root, { - owner, - operation: - this.#pending.get(root)?.operation ?? this.#artifacts.operation, + owner: owner ?? retained?.owner, + operation: retained?.operation ?? this.#artifacts.operation, + lease: + retained?.lease ?? + new AnalysisCleanupLease(() => this.#releaseRoot(root), { + get cleanupResources() { + return resources(); + }, + }), }); } @@ -152,7 +162,9 @@ export class ArtifactAnalysisResources { } #resourcesFor(root: ArtifactAnalysisRuntime): readonly string[] { - const ownership = this.#pending.get(root)?.owner?.launch.ownership; + const retained = this.#pending.get(root); + if (retained === undefined) return []; + const ownership = retained.owner?.launch.ownership; return ownership === undefined ? [root.path] : ownedCommandResources(ownership, root.path); @@ -165,55 +177,14 @@ export class ArtifactAnalysisResources { } async release(): Promise { + const failures: ProviderCleanupError[] = []; for (const [root, retained] of this.#pending) { try { - const { owner } = retained; - if (owner !== undefined) { - const stopped = await owner.stop(); - if (stopped.status === "incomplete") { - const cleanup = stopped.cleanupResult; - throw new ProviderCleanupError( - this.providerId, - this.#resourcesFor(root), - { - reason: stopped.reason, - ...(cleanup?.failures === undefined - ? {} - : { - failures: cleanup.failures.map((failure) => ({ - ...failure, - })), - }), - ...(cleanup?.unverified === undefined - ? {} - : { - unverified: cleanup.unverified.map((process) => ({ - ...process, - })), - }), - }, - { operation: retained.operation }, - ); - } - this.#pending.set(root, { ...retained, owner: undefined }); - } - if (this.#artifacts.scope.hasPending) - throw new ProviderCleanupError( - this.providerId, - [root.path], - { - reason: - "Artifact descriptors remain open; the private runtime root is retained until descriptor cleanup succeeds.", - }, - { operation: retained.operation }, - ); - await root.close(); - this.#pending.delete(root); - this.#resolveCleanupFailures(root); + await retained.lease.release(); } catch (cause: unknown) { const diagnostics = cause instanceof ProviderCleanupError ? cause.diagnostics : undefined; - throw new ProviderCleanupError( + const failure = new ProviderCleanupError( this.providerId, this.#resourcesFor(root), { @@ -225,16 +196,81 @@ export class ArtifactAnalysisResources { }, { operation: retained.operation, cause }, ); + new AnalysisCleanupLeaseGroup(failure, [retained.lease]); + failures.push(failure); } } + if (failures.length === 0) return; + if (failures.length === 1) throw failures[0]; + throw new ProviderCleanupError( + this.providerId, + failures.flatMap((failure) => failure.cleanupResources), + { + reason: "One or more analysis runtime resources remain owned.", + failures: failures.map(projectAnalysisError), + }, + { + operation: failures[0]?.operation ?? this.#artifacts.operation, + cause: new AggregateError( + failures, + "Analysis runtime cleanup remains incomplete", + ), + }, + ).bindCleanupDependencies(failures); + } + + async #releaseRoot(root: ArtifactAnalysisRuntime): Promise { + const retained = this.#pending.get(root); + if (retained === undefined) return; + if (retained.owner !== undefined) { + const stopped = await retained.owner.stop(); + if (stopped.status === "incomplete") { + const cleanup = stopped.cleanupResult; + throw new ProviderCleanupError( + this.providerId, + this.#resourcesFor(root), + { + reason: stopped.reason, + ...(cleanup?.failures === undefined + ? {} + : { + failures: cleanup.failures.map((failure) => ({ ...failure })), + }), + ...(cleanup?.unverified === undefined + ? {} + : { + unverified: cleanup.unverified.map((process) => ({ + ...process, + })), + }), + }, + { operation: retained.operation }, + ); + } + this.#pending.set(root, { ...retained, owner: undefined }); + } + if (this.#artifacts.scope.hasPending) + throw new ProviderCleanupError( + this.providerId, + [root.path], + { + reason: + "Artifact resources remain owned; the private runtime root is retained until their cleanup succeeds.", + }, + { operation: retained.operation }, + ); + await root.close(); + this.#pending.delete(root); } cleanupFailure( - observation: NonNullable, - cause: unknown, + cause: ArtifactReaderFailure, + classifyPrimary: (cause: unknown) => AnalysisError, capturedOutput?: AnalysisCapturedOutput, operation = this.operation, ): AnalysisError { + const observation = cause.cleanup; + if (observation === undefined) return classifyPrimary(cause); const failure = new ProviderCleanupError( this.providerId, observation.resources, @@ -246,23 +282,50 @@ export class ArtifactAnalysisResources { }, { operation, cause }, ); - this.#trackCleanupFailure(failure, [], failure, true); - if ( - cause instanceof ArtifactReaderFailure && - cause.reason === "cancelled" - ) { - const primary = new AnalysisCancelledError(operation, { + let leases = + cause instanceof ArtifactFileCleanupFailure || + cause instanceof ArtifactResourceRetryFailure + ? cause.cleanupLeases + : undefined; + if (leases === undefined) { + // An unreported capability cannot be recovered by observing an empty scope. + // Retain the unresolved source failure and refuse replacement admission. + const unresolved = new AnalysisCleanupLease(async () => { + throw cause; + }, observation.resources); + this.#artifacts.scope.retain({ + kind: "lease", + resource: observation.resources[0] ?? "Unresolved artifact cleanup", + lease: unresolved, + }); + leases = [unresolved]; + } + new AnalysisCleanupLeaseGroup(failure, leases); + let primary: AnalysisError | undefined; + if (cause instanceof ArtifactFileCleanupFailure) { + const physical = cause.fileCleanupFailure; + if (physical.outcome.kind === "failed") + primary = classifyPrimary(physical.outcome.cause); + else if (physical.completionCancellation !== undefined) + primary = new AnalysisCancelledError(operation, { + cause: physical.completionCancellation.reason, + ...(capturedOutput === undefined ? {} : { capturedOutput }), + partialObservation: cause.partialObservation, + }); + } else if (cause.reason === "cancelled") { + primary = new AnalysisCancelledError(operation, { cause: artifactFileCancellationCause(cause), ...(capturedOutput === undefined ? {} : { capturedOutput }), partialObservation: cause.partialObservation, }); - return analysisErrorWithCleanupFailure( - primary, - failure, - operation, - ).bindCleanupDependencies([primary, failure]); } - return failure; + return primary === undefined + ? failure + : analysisErrorWithCleanupFailure( + primary, + failure, + operation, + ).bindCleanupDependencies([primary, failure]); } commandFailure( @@ -343,7 +406,7 @@ export class ArtifactAnalysisResources { const partialObservation = result.ok ? observation(result.value) : result.error.partialObservation; - const primaryFailure = !result.ok + const primary = !result.ok ? result.error : signal?.aborted ? new AnalysisCancelledError(operation, { @@ -351,52 +414,26 @@ export class ArtifactAnalysisResources { partialObservation, }) : undefined; - const cleanupDiagnostics = - cause instanceof ProviderCleanupError ? cause.diagnostics : undefined; - const cleanupFailure = new ProviderCleanupError( - this.providerId, - [ - ...new Set([ - ...(!result.ok && result.error.cleanupIncomplete - ? result.error.cleanupResources - : []), - ...this.#pendingResources, - ]), - ], - { - ...cleanupDiagnostics, - reason: - cause instanceof ProviderCleanupError - ? String(cleanupDiagnostics?.reason ?? cause.message) - : String(cause), - previous_error: - primaryFailure === undefined - ? null - : projectAnalysisError(primaryFailure), - }, - { - operation, - ...(primaryFailure === undefined - ? {} - : { cleanupPrimary: primaryFailure }), - cause: - primaryFailure === undefined - ? cause - : new AggregateError( - [primaryFailure, cause], - "Analysis failed and artifact cleanup remains incomplete.", - { cause: primaryFailure }, - ), - ...(partialObservation === undefined ? {} : { partialObservation }), - }, - ); - this.#trackCleanupFailure( - cleanupFailure, - [...this.#pending.keys()], - primaryFailure?.cleanupIncomplete === true ? primaryFailure : undefined, - false, - ); - return err(cleanupFailure); + const cleanup = + cause instanceof AnalysisError + ? cause + : new ProviderCleanupError( + this.providerId, + this.#pendingResources, + { reason: String(cause) }, + { operation, cause }, + ); + const failure = + primary === undefined + ? cleanup + : analysisErrorWithCleanupFailure( + primary, + cleanup, + operation, + ).bindCleanupDependencies([primary, cleanup]); + if (partialObservation !== undefined) + failure.retainPartialObservation(partialObservation); + return err(failure); } } @@ -411,7 +448,6 @@ export class ArtifactAnalysisResources { let descriptorFailure: AnalysisError | undefined; try { await this.#artifacts.scope.close(); - this.#resolveCleanupFailures(); } catch (cause: unknown) { descriptorFailure = this.#artifactFailure(cause); } @@ -423,45 +459,24 @@ export class ArtifactAnalysisResources { descriptorFailure, cause, this.#artifacts.operation, - ); + ).bindCleanupDependencies([descriptorFailure, cause]); throw cause; } if (descriptorFailure !== undefined) throw descriptorFailure; } - #resolveCleanupFailures(releasedRoot?: ArtifactAnalysisRuntime): void { - for (const [failure, binding] of this.#cleanupFailures) { - if (releasedRoot !== undefined) binding.roots.delete(releasedRoot); - if (binding.roots.size > 0) continue; - if (binding.scopeOwner && this.#artifacts.scope.hasPending) continue; - if (binding.scopeOwner) binding.primaryFailure?.resolveCleanup(); - if (binding.primaryFailure?.cleanupIncomplete === true) continue; - failure.resolveCleanup(); - this.#cleanupFailures.delete(failure); - } - } - - #trackCleanupFailure( - failure: AnalysisError, - roots: readonly ArtifactAnalysisRuntime[], - primaryFailure: AnalysisError | undefined, - scopeOwner: boolean, - ): void { - this.#cleanupFailures.set(failure, { - roots: new Set(roots), - primaryFailure, - scopeOwner, - }); - this.#resolveCleanupFailures(); - } - #artifactFailure( cause: unknown, operation = this.#artifacts.operation, ): AnalysisError { if (cause instanceof AnalysisError) return cause; if (cause instanceof ArtifactReaderFailure && cause.cleanup !== undefined) - return this.cleanupFailure(cause.cleanup, cause, undefined, operation); + return this.cleanupFailure( + cause, + (primary) => this.#artifactFailure(primary, operation), + undefined, + operation, + ); if (cause instanceof ArtifactReaderFailure && cause.reason === "cancelled") return new AnalysisCancelledError(operation, { cause: artifactFileCancellationCause(cause), diff --git a/src/signals/SigrokRecordingProvider.ts b/src/signals/SigrokRecordingProvider.ts index c14b20d3d7..e623e02985 100644 --- a/src/signals/SigrokRecordingProvider.ts +++ b/src/signals/SigrokRecordingProvider.ts @@ -407,8 +407,17 @@ export class SigrokRecordingProvider implements SignalRecordingPort { if (cause instanceof AnalysisError) return cause; if (cause instanceof ArtifactReaderFailure && cause.cleanup !== undefined) return this.#resources.cleanupFailure( - cause.cleanup, cause, + (primary) => + this.#failure( + primary, + phase, + path, + workspacePath, + effectiveLimits, + observedOutput, + operation, + ), observedOutput, operation, ); diff --git a/tests/boundary/process/artifactFileOutcomes.test.ts b/tests/boundary/process/artifactFileOutcomes.test.ts new file mode 100644 index 0000000000..d8c68779f9 --- /dev/null +++ b/tests/boundary/process/artifactFileOutcomes.test.ts @@ -0,0 +1,179 @@ +import { writeFile } from "node:fs/promises"; +import type { FileHandle } from "node:fs/promises"; +import { join } from "node:path"; +import { expect, it } from "vitest"; +import { SigrokRecordingProvider } from "../../../src/signals/SigrokRecordingProvider.js"; +import { TsharkCaptureProvider } from "../../../src/captures/TsharkCaptureProvider.js"; +import { SignalRecordingService } from "../../../src/application/signals/SignalRecordingService.js"; +import { PacketCaptureService } from "../../../src/application/captures/PacketCaptureService.js"; +import { ArtifactReaderFailure } from "../../../src/artifacts/ArtifactReader.js"; +import { withArtifactRegularFile } from "../../../src/artifacts/ArtifactFileOperation.js"; +import { readStableArtifact } from "../../../src/artifacts/readStableArtifact.js"; +import { projectAnalysisError } from "../../../src/domain/analysisErrorProjection.js"; +import { PrivateRuntimeRoot } from "../../../src/process/PrivateRuntimeRoot.js"; +import { spawnOwnedProviderProcess } from "../../../src/process/ProviderProcess.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; + +// Real descriptors/owned Node commands; simulated Linux limits, no native tools. +it.each( + (["signal", "capture"] as const).flatMap((kind) => + [ + { + stage: "configuration", + failure: "denied", + tag: "AnalysisCapabilityUnavailableError", + }, + { + stage: "artifact", + failure: "denied", + tag: "AnalysisAccessDeniedError", + }, + { stage: "artifact", failure: "format", tag: "AnalysisInputError" }, + { + stage: "artifact", + failure: "integrity", + tag: "AnalysisArtifactChangedError", + }, + { + stage: "artifact", + failure: "limit", + tag: "AnalysisResourceConstraintError", + }, + { + stage: "integrity", + failure: "integrity", + tag: "AnalysisArtifactChangedError", + }, + ].map((control) => ({ kind, ...control })), + ), +)( + "retains the physical file outcome and exact descriptor through retry: %j", + async ({ kind, stage, failure, tag }) => { + const root = await createTestTempDirectory("rea-file-primary-"); + const inputPath = join(root, "input"); + const command = join(root, "producer.mjs"); + await writeFile(inputPath, "authored recording"); + await writeFile( + command, + kind === "signal" + ? "if(process.argv.includes('--version')) console.log('sigrok-cli 0.7.2\\nlibsigrokdecode 0.5.3'); else console.log('annotation');" + : "if(process.argv.includes('--version')) console.log('TShark (Wireshark) 4.6.9 (fixture)'); else console.log('[]');", + { mode: 0o700 }, + ); + const physical = + failure === "denied" + ? Object.assign(new Error("controlled read denial"), { code: "EACCES" }) + : new ArtifactReaderFailure( + failure === "format" + ? "format" + : failure === "limit" + ? "limit" + : "integrity", + "controlled physical file failure", + ); + const selectedPath = stage === "artifact" ? inputPath : command; + let allowed = false; + let commandReads = 0; + let launches = 0; + let handle: FileHandle | undefined; + const caller = new AbortController(); + const Provider = + kind === "signal" ? SigrokRecordingProvider : TsharkCaptureProvider; + const provider = new Provider( + { + ...process.env, + REA_SIGROK_COMMAND: command, + REA_SIGROK_PRLIMIT_COMMAND: command, + REA_TSHARK_COMMAND: command, + REA_TSHARK_PRLIMIT_COMMAND: command, + }, + async (options) => { + launches++; + return spawnOwnedProviderProcess({ + ...options, + command: process.execPath, + arguments: [command, ...options.arguments], + expectedCommand: process.execPath, + }); + }, + () => PrivateRuntimeRoot.create({ parent: root }), + "linux", + async (...args) => { + if (args[0] === command) commandReads++; + const selected = + args[0] === selectedPath && + (stage !== "integrity" || commandReads === 3); + if (!selected) return readStableArtifact(...args); + const scope = args[4]; + if (scope === undefined) + throw new Error("Expected retained artifact scope"); + return withArtifactRegularFile( + args[0], + scope, + args[2], + async (file) => { + handle = file; + const close = file.close.bind(file); + file.close = async () => { + // A later abort must not replace the already observed physical failure. + caller.abort({ + request: "late cancellation during failed close", + }); + if (!allowed) + throw new Error("controlled descriptor close failure"); + await close(); + }; + throw physical; + }, + ); + }, + async (limits) => ({ ...limits, file_bytes: 1048576 }), + ); + try { + const result = + provider instanceof SigrokRecordingProvider + ? await new SignalRecordingService(provider).decode( + { path: inputPath, decoders: "uart" }, + { signal: caller.signal }, + ) + : await new PacketCaptureService(provider).inspect( + { path: inputPath, fields: ["frame.number"] }, + { signal: caller.signal }, + ); + if (result.ok) + throw new Error("Expected physical failure and retained descriptor"); + let primary = result.error.cleanupPrimary; + while (primary?.cleanupPrimary !== undefined) + primary = primary.cleanupPrimary; + expect(primary).toMatchObject({ _tag: tag, cause: physical }); + expect(result.error.cleanupIncomplete).toBe(true); + expect(handle?.fd).toBeGreaterThanOrEqual(0); + const failed: unknown = await provider + .close() + .catch((cause: unknown) => cause); + expect(failed).toMatchObject({ cleanupIncomplete: true }); + const launchCount = launches; + allowed = true; + await provider.close(); + expect(handle?.fd).toBe(-1); + expect(result.error.cleanupIncomplete).toBe(false); + let restored = result.error.cleanupPrimary; + while (restored?.cleanupPrimary !== undefined) + restored = restored.cleanupPrimary; + expect(restored).toBe(primary); + expect(primary?.cause).toBe(physical); + expect(projectAnalysisError(result.error)).toMatchObject( + projectAnalysisError(primary!), + ); + expect(projectAnalysisError(result.error).details).toHaveProperty( + "cleanup_history", + ); + expect(failed).toMatchObject({ cleanupIncomplete: false }); + expect(launches).toBe(launchCount); + } finally { + allowed = true; + await provider.close(); + await handle?.close(); + } + }, +);