diff --git a/src/artifacts/extraction/ArtifactExtraction.ts b/src/artifacts/extraction/ArtifactExtraction.ts index c4a6fc8e12..165bf628c4 100644 --- a/src/artifacts/extraction/ArtifactExtraction.ts +++ b/src/artifacts/extraction/ArtifactExtraction.ts @@ -1,3 +1,4 @@ +import type { ArtifactCleanupAttempt } from "../ArtifactResourceScope.js"; import { lstat, realpath } from "node:fs/promises"; import { compareUnicodeCodePoints } from "../../domain/unicodeCodePointOrder.js"; @@ -245,13 +246,9 @@ const materializeSelection = async ({ localReaderOwner = undefined; const closeAttempt = await input.resourceScope.release(readerOwner); if (closeAttempt.kind === "failed") - throw ArtifactReaderFailure.withCleanup( - closeAttempt.cause, - ArtifactReaderFailure.cleanupObservation( - closeAttempt.cause, - readerOwner.resource, - ), - ); + throw input.resourceScope.cleanupFailure({ kind: "completed" }, [ + closeAttempt, + ]); extracted.sort((left, right) => compareUnicodeCodePoints(left.relative_path, right.relative_path), ); @@ -265,19 +262,11 @@ const materializeSelection = async ({ output, input.outputRoot, ); - if (cleanupFailures.length > 0) { - const observations = cleanupFailures.map( - ({ cause: cleanupCause, resource }) => - ArtifactReaderFailure.cleanupObservation(cleanupCause, resource), - ); - throw ArtifactReaderFailure.withCleanup( - cause, - { - reason: observations.map(({ reason }) => reason).join("; "), - resources: [ - ...new Set(observations.flatMap(({ resources }) => resources)), - ], - }, + const [firstCleanup, ...remainingCleanup] = cleanupFailures; + if (firstCleanup !== undefined) { + throw input.resourceScope.cleanupFailure( + { kind: "failed", cause }, + [firstCleanup, ...remainingCleanup], output?.published === true && completed !== undefined ? { kind: "artifact-extraction", extraction: completed } : undefined, @@ -346,12 +335,14 @@ const releaseExtractionOwners = async ( readerOwner: ArtifactResourceOwner | undefined, output: SafeOutputTree | undefined, outputRoot: string, -): Promise<{ readonly cause: unknown; readonly resource: string }[]> => { - const failures: { readonly cause: unknown; readonly resource: string }[] = []; +): Promise[]> => { + const failures: Extract< + ArtifactCleanupAttempt, + { readonly kind: "failed" } + >[] = []; if (readerOwner !== undefined) { const attempt = await resourceScope.release(readerOwner); - if (attempt.kind === "failed") - failures.push({ cause: attempt.cause, resource: readerOwner.resource }); + if (attempt.kind === "failed") failures.push(attempt); } if (output !== undefined) { const owner: ArtifactResourceOwner = { @@ -360,8 +351,7 @@ const releaseExtractionOwners = async ( resource: outputRoot, }; const attempt = await resourceScope.release(owner); - if (attempt.kind === "failed") - failures.push({ cause: attempt.cause, resource: owner.resource }); + if (attempt.kind === "failed") failures.push(attempt); } return failures; }; diff --git a/src/artifacts/inventory/classify.ts b/src/artifacts/inventory/classify.ts index 73cc6d760b..0df8784ebb 100644 --- a/src/artifacts/inventory/classify.ts +++ b/src/artifacts/inventory/classify.ts @@ -66,13 +66,9 @@ export const classifyAndHashRoot = async ( }; const cleanupAttempt = await options.resourceScope.release(owner); if (cleanupAttempt.kind === "failed") - throw ArtifactReaderFailure.withCleanup( - cleanupAttempt.cause, - ArtifactReaderFailure.cleanupObservation( - cleanupAttempt.cause, - owner.resource, - ), - ); + throw options.resourceScope.cleanupFailure({ kind: "completed" }, [ + cleanupAttempt, + ]); } return { format: result.format, digest: result.digest }; }); @@ -136,13 +132,7 @@ export const classifyAndHashRootForInventory = async ( if (outcome.kind === "failed" || outcome.value.rootSource === undefined) { const cleanupAttempt = await options.resourceScope.release(owner); if (cleanupAttempt.kind === "failed") - throw ArtifactReaderFailure.withCleanup( - outcome.kind === "failed" ? outcome.cause : cleanupAttempt.cause, - ArtifactReaderFailure.cleanupObservation( - cleanupAttempt.cause, - owner.resource, - ), - ); + throw options.resourceScope.cleanupFailure(outcome, [cleanupAttempt]); } if (outcome.kind === "failed") throw outcome.cause; return outcome.value; diff --git a/src/artifacts/inventory/hashStableRootArtifact.ts b/src/artifacts/inventory/hashStableRootArtifact.ts index cf57b804a2..5c5ef95334 100644 --- a/src/artifacts/inventory/hashStableRootArtifact.ts +++ b/src/artifacts/inventory/hashStableRootArtifact.ts @@ -55,10 +55,9 @@ export const openRootArtifact = async ( handle: cause.owner, }); if (cleanup.kind === "failed") - throw ArtifactReaderFailure.withCleanup( - failure, - ArtifactReaderFailure.cleanupObservation(cleanup.cause, resource), - ); + throw resourceScope.cleanupFailure({ kind: "failed", cause: failure }, [ + cleanup, + ]); } throw failure; } @@ -91,10 +90,7 @@ export const hashStableRootArtifact = async ( handle: owner, }); if (cleanup.kind === "failed") - throw ArtifactReaderFailure.withCleanup( - outcome.kind === "failed" ? outcome.cause : cleanup.cause, - ArtifactReaderFailure.cleanupObservation(cleanup.cause, resource), - ); + throw resourceScope.cleanupFailure(outcome, [cleanup]); if (outcome.kind === "failed") throw outcome.cause; return outcome.value; }; diff --git a/src/artifacts/inventory/scanCanonical.ts b/src/artifacts/inventory/scanCanonical.ts index 93d12b4fe2..e1521a2092 100644 --- a/src/artifacts/inventory/scanCanonical.ts +++ b/src/artifacts/inventory/scanCanonical.ts @@ -173,12 +173,11 @@ const cleanupInventoryOwners = async ({ for (const owner of owners) { const attempt = await options.resourceScope.release(owner); if (attempt.kind === "released") continue; - const primary = - cleanupFailure ?? - (outcome.kind === "failed" ? outcome.cause : attempt.cause); - cleanupFailure = ArtifactReaderFailure.withCleanup( - primary, - ArtifactReaderFailure.cleanupObservation(attempt.cause, owner.resource), + cleanupFailure = options.resourceScope.cleanupFailure( + cleanupFailure === undefined + ? outcome + : { kind: "failed", cause: cleanupFailure }, + [attempt], outcome.kind === "completed" ? { kind: "artifact-inventory", inventory: outcome.snapshot } : undefined, diff --git a/src/artifacts/javascript/JavaScriptArtifactFiles.ts b/src/artifacts/javascript/JavaScriptArtifactFiles.ts index d1149836b3..562235eed7 100644 --- a/src/artifacts/javascript/JavaScriptArtifactFiles.ts +++ b/src/artifacts/javascript/JavaScriptArtifactFiles.ts @@ -306,9 +306,11 @@ const visitReader = async ( }); if (cleanup.kind === "failed") { failure = { - cause: ArtifactReaderFailure.withCleanup( - failure === undefined ? cleanup.cause : failure.cause, - ArtifactReaderFailure.cleanupObservation(cleanup.cause, resource), + cause: context.resources.cleanupFailure( + failure === undefined + ? { kind: "completed" } + : { kind: "failed", cause: failure.cause }, + [cleanup], ), }; } diff --git a/tests/boundary/filesystem/artifactInventoryDescriptorCleanup.test.ts b/tests/boundary/filesystem/artifactInventoryDescriptorCleanup.test.ts index d89714ceb1..1817d7c3ea 100644 --- a/tests/boundary/filesystem/artifactInventoryDescriptorCleanup.test.ts +++ b/tests/boundary/filesystem/artifactInventoryDescriptorCleanup.test.ts @@ -87,6 +87,8 @@ it.each([false, true])( invalidated ? attemptsBeforeRecovery : attemptsBeforeRecovery + 1, ); expect(descriptor?.fd).toBe(-1); + if (!invalidated && !inventory.ok) + expect(inventory.error.cleanupIncomplete).toBe(false); if (invalidated) expect(closeAttempts).toBe(1); }, ); diff --git a/tests/boundary/providers/native/artifactProvider.cleanupRetry.test.ts b/tests/boundary/providers/native/artifactProvider.cleanupRetry.test.ts index ca20ee7ea6..d216d18cbc 100644 --- a/tests/boundary/providers/native/artifactProvider.cleanupRetry.test.ts +++ b/tests/boundary/providers/native/artifactProvider.cleanupRetry.test.ts @@ -97,6 +97,8 @@ it("retains a failed reader close, blocks new work, and retries the same reader" expect(await client.close()).toMatchObject({ ok: true }); expect(closeOwners).toHaveLength(5); expect(closeOwners[4]).toBe(closeOwners[1]); + if (!extraction.ok) expect(extraction.error.cleanupIncomplete).toBe(false); + expect(incompleteClose.error.cleanupIncomplete).toBe(false); const closed = await client.execute("inventory_artifact", {}); expect(closed).toMatchObject({ ok: false,