diff --git a/src/application/ClientConfigurationCleanup.ts b/src/application/ClientConfigurationCleanup.ts index bf0e310eab..596c32dcd9 100644 --- a/src/application/ClientConfigurationCleanup.ts +++ b/src/application/ClientConfigurationCleanup.ts @@ -74,7 +74,10 @@ export const retryClientConfigurationCleanup = async ( if (retried.cleanup !== undefined) observations.push(retried.cleanup); primary = retried.primary; const last = retried.failures.at(-1)!; - nested = last.outcome.kind === "failed" ? last.outcome.cause : undefined; + nested = + last.operationFailure === undefined && last.outcome.kind === "failed" + ? last.outcome.cause + : undefined; } else { const released = await resources.release({ kind: "lease", diff --git a/src/application/RegularFileRead.ts b/src/application/RegularFileRead.ts index 65ed2af261..31fc6bb432 100644 --- a/src/application/RegularFileRead.ts +++ b/src/application/RegularFileRead.ts @@ -12,6 +12,7 @@ import { RegularFileCancelled, regularFileCancellation, withOwnedRegularFile, + OwnedRegularFileOperationFailure, type RegularFileOperationOutcome, } from "../filesystem/OwnedRegularFile.js"; import { readBoundedFileBytes } from "../process/BoundedFileBytes.js"; @@ -29,12 +30,12 @@ export interface RegularFileReadOptions { readonly cancellationFailure?: (reason: unknown) => Error; } -/** A regular-file failure returned alongside an enabled cancellation factory. */ -export class RegularFileReadFailure extends Error { - readonly _tag = "RegularFileReadFailure"; +/** Preserves a physical descriptor-read cause until its owner has finalized. */ +export class RegularFileReadFailure extends OwnedRegularFileOperationFailure { + override readonly _tag = "RegularFileReadFailure"; - constructor(readonly operationCause: unknown) { - super("Regular file operation failed", { cause: operationCause }); + constructor(operationCause: unknown) { + super(operationCause); this.name = this._tag; } } @@ -65,6 +66,7 @@ export const retryRegularFileCleanup = async ( }> => { const outcome: RegularFileReadOutcome = failure.outcome.kind === "failed" && + failure.operationFailure === undefined && failure.outcome.cause instanceof RegularFileOperationCancelled && !(signal?.aborted === true && failure.outcome.cause === signal.reason) ? { kind: "failed", cause: failure.outcome.cause.reason } @@ -106,7 +108,10 @@ export const retryRegularFileCleanupFailures = async ( ? retried.outcome.cause : current.cleanupCause; const nested = - current.outcome.kind === "failed" ? current.outcome.cause : undefined; + current.operationFailure === undefined && + current.outcome.kind === "failed" + ? current.outcome.cause + : undefined; if (!(nested instanceof RegularFileCleanupFailure)) break; current = nested; } @@ -177,16 +182,27 @@ export const withRegularFile = async ( const retry = await retryRegularFileCleanup(cause, resources, signal); if (retry.cleanup !== undefined) throw cause; if (retry.outcome.kind === "failed") { - if (signal?.aborted === true && retry.outcome.cause === signal.reason) + if ( + cause.operationFailure === undefined && + signal?.aborted === true && + retry.outcome.cause === signal.reason + ) throw signal.reason; const cancellation = - regularFileCancellation(retry.outcome.cause) ?? - regularFileCancellation(cause); + cause.operationFailure === undefined + ? (regularFileCancellation(retry.outcome.cause) ?? + regularFileCancellation(cause)) + : undefined; if (cancellation !== undefined) { if (options.cancellationFailure !== undefined) throw options.cancellationFailure(cancellation.reason); throw cancellation.reason; } + if ( + cause.operationFailure !== undefined && + options.cancellationFailure !== undefined + ) + throw cause.operationFailure; throw retry.outcome.cause; } return retry.outcome.value; @@ -255,8 +271,8 @@ const readRegularFileContent = ( path: string, decode: (bytes: Buffer, state: Stats) => Value, options: RegularFileReadOptions, -): Promise => - withRegularFile( +): Promise => { + const operation = withRegularFile( path, async (handle, stats) => { try { @@ -282,3 +298,9 @@ const readRegularFileContent = ( }, options, ); + if (options.cancellationFailure !== undefined) return operation; + return operation.catch((cause: unknown) => { + if (cause instanceof RegularFileReadFailure) throw cause.operationCause; + throw cause; + }); +}; diff --git a/src/filesystem/OwnedRegularFile.ts b/src/filesystem/OwnedRegularFile.ts index aad2d1c1f9..25c0cac6f4 100644 --- a/src/filesystem/OwnedRegularFile.ts +++ b/src/filesystem/OwnedRegularFile.ts @@ -17,6 +17,16 @@ type CompletedRegularFileOutcome = Extract< { readonly kind: "completed" } >; +/** Physical operation origin kept separate from caller-provided error values. */ +export class OwnedRegularFileOperationFailure extends Error { + readonly _tag: string = "OwnedRegularFileOperationFailure"; + + constructor(readonly operationCause: unknown) { + super("Owned regular-file operation failed", { cause: operationCause }); + this.name = this._tag; + } +} + /** Cancellation after confirmed close, retaining the completed physical read. */ export class RegularFileCancelled extends Error { readonly _tag = "RegularFileCancelled"; @@ -54,6 +64,7 @@ export class RegularFileCleanupFailure extends Error { kind: "not_requested", }, readonly completionCancellation?: { readonly reason: unknown }, + readonly operationFailure?: OwnedRegularFileOperationFailure, ) { super(`Regular file cleanup failed for ${path}`, { cause: new AggregateError( @@ -78,6 +89,13 @@ export interface OwnedFileOperationOptions { export const regularFileCancellation = ( cause: unknown, ): { readonly reason: unknown } | undefined => { + if ( + cause instanceof RegularFileCleanupFailure && + cause.operationFailure !== undefined + ) + return cause.outcome.kind === "completed" + ? cause.completionCancellation + : undefined; const primary = cause instanceof RegularFileCleanupFailure && cause.outcome.kind === "failed" @@ -144,6 +162,18 @@ export const withOwnedFileHandle = async ( try { await owner.close(); } catch (cleanupCause: unknown) { + const operationFailure = + outcome.kind === "failed" && + outcome.cause instanceof OwnedRegularFileOperationFailure + ? outcome.cause + : undefined; + const retainedOutcome = + operationFailure === undefined + ? outcome + : { + kind: "failed" as const, + cause: operationFailure.operationCause, + }; const completionCancellation = outcome.kind === "completed" && options.signal?.aborted === true ? { reason: options.signal.reason } @@ -153,10 +183,11 @@ export const withOwnedFileHandle = async ( throw new RegularFileCleanupFailure( path, owner, - outcome, + retainedOutcome, cleanupCause, retainOwner(options.retainFailedClose, owner), completionCancellation, + operationFailure, ); } if (outcome.kind === "failed") throw outcome.cause; diff --git a/tests/boundary/filesystem/regularFileCancellation.test.ts b/tests/boundary/filesystem/regularFileCancellation.test.ts index 265d2f3f65..8e41becef8 100644 --- a/tests/boundary/filesystem/regularFileCancellation.test.ts +++ b/tests/boundary/filesystem/regularFileCancellation.test.ts @@ -5,7 +5,10 @@ import { expect, it, vi } from "vitest"; import { withRegularFile, + readRegularFile, + RegularFileReadFailure, retryRegularFileCleanup, + retryRegularFileCleanupFailures, } from "../../../src/application/RegularFileRead.js"; import { ArtifactResourceScope } from "../../../src/artifacts/ArtifactResourceScope.js"; import { RegularFileOperationCancelled } from "../../../src/filesystem/RegularFile.js"; @@ -88,6 +91,190 @@ it.each( }, ); +it("preserves a cleanup-shaped physical read rejection without borrowing its owner", async () => { + const root = await createTestTempDirectory( + "rea-file-physical-cleanup-shape-", + ); + const input = join(root, "input.txt"); + const foreignPath = join(root, "foreign.txt"); + await writeFile(input, "input"); + await writeFile(foreignPath, "foreign owner bytes"); + const foreignOwner = new OwnedFileHandle(await open(foreignPath, "r")); + const reason = new RegularFileCleanupFailure( + foreignPath, + foreignOwner, + { kind: "completed", value: "foreign value" }, + new Error("foreign cleanup failure"), + ); + const probe = await open(input, "r"); + const inputIdentity = await probe.stat(); + const prototype = probe.constructor.prototype as { + read: (...arguments_: never[]) => Promise; + }; + await probe.close(); + const originalRead = prototype.read; + prototype.read = async function ( + this: typeof probe, + ...arguments_: never[] + ): Promise { + const state = await this.stat(); + if (state.dev === inputIdentity.dev && state.ino === inputIdentity.ino) + throw reason; + return originalRead.apply(this, arguments_); + }; + try { + await expect(readRegularFile(input)).rejects.toBe(reason); + expect(foreignOwner.handle.fd).toBeGreaterThanOrEqual(0); + } finally { + prototype.read = originalRead; + await foreignOwner.close(); + } +}); + +it("retries only the owned close when a cleanup-shaped physical read remains pending", async () => { + const root = await createTestTempDirectory( + "rea-file-physical-cleanup-retry-", + ); + const input = join(root, "input.txt"); + const foreignPath = join(root, "foreign.txt"); + await writeFile(input, "input"); + await writeFile(foreignPath, "foreign owner bytes"); + const foreignOwner = new OwnedFileHandle(await open(foreignPath, "r")); + const reason = new RegularFileCleanupFailure( + foreignPath, + foreignOwner, + { kind: "completed", value: "foreign value" }, + new Error("foreign cleanup failure"), + ); + const controller = new AbortController(); + const probe = await open(input, "r"); + const inputIdentity = await probe.stat(); + const fileHandlePrototype = probe.constructor.prototype as { + read: (...arguments_: never[]) => Promise; + }; + await probe.close(); + const originalRead = fileHandlePrototype.read; + const ownerPrototype = OwnedFileHandle.prototype; + const originalClose = ownerPrototype.close; + let closeFailures = 0; + fileHandlePrototype.read = async function ( + this: typeof probe, + ...arguments_: never[] + ): Promise { + const state = await this.stat(); + if (state.dev === inputIdentity.dev && state.ino === inputIdentity.ino) { + controller.abort(reason); + throw reason; + } + return originalRead.apply(this, arguments_); + }; + ownerPrototype.close = async function (this: OwnedFileHandle): Promise { + const state = await this.handle.stat(); + if (state.dev === inputIdentity.dev && state.ino === inputIdentity.ino) { + closeFailures += 1; + if (closeFailures <= 2) + throw new Error(`owned close failure ${closeFailures}`); + } + return originalClose.call(this); + }; + const resources = new ArtifactResourceScope(); + try { + const failure = await readRegularFile(input, { + resources, + signal: controller.signal, + }).catch((cause: unknown) => cause); + expect(failure).toBeInstanceOf(RegularFileCleanupFailure); + if (!(failure instanceof RegularFileCleanupFailure)) + throw new Error("Expected the opened input owner to remain retained"); + expect(failure.outcome).toMatchObject({ kind: "failed", cause: reason }); + expect(failure.operationFailure).toBeDefined(); + + const retried = await retryRegularFileCleanupFailures(failure, resources); + expect(retried.primary).toBe(reason); + expect(retried.failures).toEqual([failure]); + expect(retried.cleanup).toBeUndefined(); + expect(closeFailures).toBe(3); + expect(foreignOwner.handle.fd).toBeGreaterThanOrEqual(0); + } finally { + fileHandlePrototype.read = originalRead; + ownerPrototype.close = originalClose; + await resources.retryCleanup(); + await foreignOwner.close(); + } +}); + +it("retains physical read provenance for an opted-in caller after close retry", async () => { + const root = await createTestTempDirectory( + "rea-file-physical-cleanup-recovered-", + ); + const input = join(root, "input.txt"); + const foreignPath = join(root, "foreign.txt"); + await writeFile(input, "input"); + await writeFile(foreignPath, "foreign owner bytes"); + const foreignOwner = new OwnedFileHandle(await open(foreignPath, "r")); + const reason = new RegularFileCleanupFailure( + foreignPath, + foreignOwner, + { kind: "completed", value: "foreign value" }, + new Error("foreign cleanup failure"), + ); + const controller = new AbortController(); + const probe = await open(input, "r"); + const inputIdentity = await probe.stat(); + const fileHandlePrototype = probe.constructor.prototype as { + read: (...arguments_: never[]) => Promise; + }; + await probe.close(); + const originalRead = fileHandlePrototype.read; + const ownerPrototype = OwnedFileHandle.prototype; + const originalClose = ownerPrototype.close; + let closeFailures = 0; + let cancellationFactoryCalls = 0; + fileHandlePrototype.read = async function ( + this: typeof probe, + ...arguments_: never[] + ): Promise { + const state = await this.stat(); + if (state.dev === inputIdentity.dev && state.ino === inputIdentity.ino) { + controller.abort(reason); + throw reason; + } + return originalRead.apply(this, arguments_); + }; + ownerPrototype.close = async function (this: OwnedFileHandle): Promise { + const state = await this.handle.stat(); + if (state.dev === inputIdentity.dev && state.ino === inputIdentity.ino) { + closeFailures += 1; + if (closeFailures === 1) throw new Error("transient owned close failure"); + } + return originalClose.call(this); + }; + const resources = new ArtifactResourceScope(); + try { + const failure = await readRegularFile(input, { + resources, + signal: controller.signal, + cancellationFailure: (abortReason) => { + cancellationFactoryCalls += 1; + return new Error("cancelled", { cause: abortReason }); + }, + }).catch((cause: unknown) => cause); + expect(failure).toBeInstanceOf(RegularFileReadFailure); + if (!(failure instanceof RegularFileReadFailure)) + throw new Error("Expected the physical read failure marker"); + expect(failure.operationCause).toBe(reason); + expect(cancellationFactoryCalls).toBe(0); + expect(closeFailures).toBe(2); + expect(resources.hasPending).toBe(false); + expect(foreignOwner.handle.fd).toBeGreaterThanOrEqual(0); + } finally { + fileHandlePrototype.read = originalRead; + ownerPrototype.close = originalClose; + await resources.retryCleanup(); + await foreignOwner.close(); + } +}); + it.each([false, true])( "preserves a nested read cancellation with the same signal=%s", async (sameSignal) => {