mirror of
https://github.com/morluto/rea
synced 2026-10-25 22:08:11 -04:00
fix(cancellation): bind retained errors to cleanup owners
Carry the capture handle cleanup lease through retry results and bind composed export failures to it. Bind aborted binary retirement errors to the failed close so successful later cleanup reveals the original cancellation. Validation: 46 focused tests passed; npm run check passed. Four regression assertions fail on the original source. Full suite and real-provider workflows were not run.
This commit is contained in:
@@ -16,6 +16,7 @@ import {
|
||||
type RegularFileOperationOutcome,
|
||||
} from "../filesystem/OwnedRegularFile.js";
|
||||
import { readBoundedFileBytes } from "../process/BoundedFileBytes.js";
|
||||
import type { AnalysisCleanupLease } from "../artifacts/AnalysisCleanupLease.js";
|
||||
import type { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js";
|
||||
import type { AnalysisCleanupObservation } from "../domain/analysisErrorBase.js";
|
||||
|
||||
@@ -62,6 +63,7 @@ export const retryRegularFileCleanup = async <Value>(
|
||||
signal?: AbortSignal,
|
||||
): Promise<{
|
||||
readonly outcome: RegularFileReadOutcome<Value>;
|
||||
readonly cleanupLease?: AnalysisCleanupLease;
|
||||
readonly cleanup?: AnalysisCleanupObservation;
|
||||
}> => {
|
||||
const outcome: RegularFileReadOutcome<Value> =
|
||||
@@ -79,6 +81,7 @@ export const retryRegularFileCleanup = async <Value>(
|
||||
if (released.kind === "released") return { outcome };
|
||||
return {
|
||||
outcome,
|
||||
cleanupLease: released.lease,
|
||||
cleanup: {
|
||||
reason: `${failureMessage(failure.cleanupCause)}; retry: ${failureMessage(released.cause)}`,
|
||||
resources: [failure.path],
|
||||
|
||||
@@ -4,6 +4,10 @@ import { isAbsolute } from "node:path";
|
||||
import { z } from "zod";
|
||||
|
||||
import { ArtifactReaderFailure } from "../artifacts/ArtifactReader.js";
|
||||
import {
|
||||
AnalysisCleanupLeaseGroup,
|
||||
type AnalysisCleanupLease,
|
||||
} from "../artifacts/AnalysisCleanupLease.js";
|
||||
import type { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js";
|
||||
import { publishWebScripts } from "../browser/assets/PublishWebScripts.js";
|
||||
import { selectScriptCapture } from "../browser/assets/ScriptCaptureAdapters.js";
|
||||
@@ -48,6 +52,7 @@ interface CaptureRead {
|
||||
readonly reason: string;
|
||||
readonly incomplete: boolean;
|
||||
readonly cause: unknown;
|
||||
readonly lease?: AnalysisCleanupLease;
|
||||
};
|
||||
}
|
||||
|
||||
@@ -259,6 +264,9 @@ const readCapture = async (
|
||||
reason: retry.cleanup?.reason ?? errorMessage(cause.cleanupCause),
|
||||
incomplete: retry.cleanup !== undefined,
|
||||
cause: cause.cleanupCause,
|
||||
...(retry.cleanupLease === undefined
|
||||
? {}
|
||||
: { lease: retry.cleanupLease }),
|
||||
};
|
||||
if (retry.outcome.kind === "completed") {
|
||||
const cancellation =
|
||||
@@ -353,8 +361,8 @@ const incompleteCaptureCleanup = (
|
||||
primary: AnalysisError,
|
||||
path: string,
|
||||
cleanup: NonNullable<CaptureRead["cleanup"]>,
|
||||
): AnalysisError =>
|
||||
analysisErrorWithCleanupFailure(
|
||||
): AnalysisError => {
|
||||
const error = analysisErrorWithCleanupFailure(
|
||||
primary,
|
||||
new ProviderCleanupError(
|
||||
PROVIDER.id,
|
||||
@@ -364,6 +372,10 @@ const incompleteCaptureCleanup = (
|
||||
),
|
||||
OPERATION,
|
||||
);
|
||||
if (cleanup.lease !== undefined)
|
||||
new AnalysisCleanupLeaseGroup(error, [cleanup.lease]);
|
||||
return error;
|
||||
};
|
||||
|
||||
const errorMessage = (cause: unknown): string =>
|
||||
cause instanceof Error ? cause.message : String(cause);
|
||||
|
||||
@@ -597,7 +597,7 @@ export class BinarySession
|
||||
}),
|
||||
closed.error,
|
||||
"open_binary",
|
||||
),
|
||||
).bindCleanupDependencies([closed.error]),
|
||||
);
|
||||
return closed;
|
||||
}
|
||||
|
||||
@@ -628,34 +628,52 @@ describe("captured script final commit cancellation", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("returns cancellation with the caller reason when a retried capture close recovers", async () => {
|
||||
const { input, resources } = await setup();
|
||||
const controller = new AbortController();
|
||||
const reason = { request: "stop export" };
|
||||
let closeCalls = 0;
|
||||
openMock.mockImplementation(async (...args) => {
|
||||
const handle = await actualFs.open(...args);
|
||||
if (String(args[0]) !== input.capture_path) return handle;
|
||||
const close = handle.close.bind(handle);
|
||||
vi.spyOn(handle, "close").mockImplementation(async () => {
|
||||
closeCalls += 1;
|
||||
if (closeCalls === 1) {
|
||||
controller.abort(reason);
|
||||
throw new Error("injected capture close failure");
|
||||
}
|
||||
await close();
|
||||
it.each([1, 2])(
|
||||
"returns cancellation after %i failed capture close attempts recover",
|
||||
async (failedCloses) => {
|
||||
const { input, resources } = await setup();
|
||||
const controller = new AbortController();
|
||||
const reason = { request: "stop export" };
|
||||
let closeCalls = 0;
|
||||
openMock.mockImplementation(async (...args) => {
|
||||
const handle = await actualFs.open(...args);
|
||||
if (String(args[0]) !== input.capture_path) return handle;
|
||||
const close = handle.close.bind(handle);
|
||||
vi.spyOn(handle, "close").mockImplementation(async () => {
|
||||
closeCalls += 1;
|
||||
if (closeCalls <= failedCloses) {
|
||||
controller.abort(reason);
|
||||
throw new Error("injected capture close failure");
|
||||
}
|
||||
await close();
|
||||
});
|
||||
return handle;
|
||||
});
|
||||
return handle;
|
||||
});
|
||||
const result = await exportWebScripts(input, resources, {
|
||||
signal: controller.signal,
|
||||
});
|
||||
if (result.ok) throw new Error("Expected cancellation");
|
||||
expect(closeCalls).toBe(2);
|
||||
expect(result.error._tag).toBe("AnalysisCancelledError");
|
||||
expect(result.error.cause).toBe(reason);
|
||||
expect(resources.hasPending).toBe(false);
|
||||
});
|
||||
const result = await exportWebScripts(input, resources, {
|
||||
signal: controller.signal,
|
||||
});
|
||||
if (result.ok) throw new Error("Expected cancellation");
|
||||
expect(closeCalls).toBe(2);
|
||||
expect(result.error.cleanupIncomplete).toBe(failedCloses === 2);
|
||||
expect(resources.hasPending).toBe(failedCloses === 2);
|
||||
if (failedCloses === 2) {
|
||||
expect(projectAnalysisError(result.error)).toMatchObject({
|
||||
code: "cleanup_incomplete",
|
||||
details: { resources: [input.capture_path] },
|
||||
});
|
||||
expect(result.error.cleanupPrimary?.cause).toBe(reason);
|
||||
} else {
|
||||
expect(result.error.cause).toBe(reason);
|
||||
}
|
||||
await resources.close();
|
||||
expect(closeCalls).toBe(failedCloses + 1);
|
||||
expect(resources.hasPending).toBe(false);
|
||||
expect(result.error.cleanupIncomplete).toBe(false);
|
||||
expect(projectAnalysisError(result.error)).toMatchObject({
|
||||
code: "cancelled",
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
it("identifies a missing capture file and rejected host path syntax", async () => {
|
||||
const { input, resources } = await setup();
|
||||
@@ -925,6 +943,11 @@ describe("capture read and close failure", () => {
|
||||
await resources.close();
|
||||
expect(closeCalls).toBe(4);
|
||||
expect(openedHandle.fd).toBe(-1);
|
||||
expect(result.error.cleanupIncomplete).toBe(false);
|
||||
expect(projectAnalysisError(result.error)).toMatchObject({
|
||||
code: "artifact_operation_failed",
|
||||
details: { reason: "io" },
|
||||
});
|
||||
} finally {
|
||||
allowClose = true;
|
||||
await resources.close().catch(() => undefined);
|
||||
@@ -984,6 +1007,10 @@ describe("capture handle ownership", () => {
|
||||
allowClose = true;
|
||||
await resources.close();
|
||||
expect([...closeCalls.values()]).toEqual([4]);
|
||||
expect(result.error.cleanupIncomplete).toBe(false);
|
||||
expect(projectAnalysisError(result.error)).toMatchObject({
|
||||
code: "invalid_request",
|
||||
});
|
||||
} finally {
|
||||
allowClose = true;
|
||||
await resources.close().catch(() => undefined);
|
||||
|
||||
@@ -2,6 +2,10 @@ import { access, mkdir, readFile, rm, writeFile } from "node:fs/promises";
|
||||
import { join } from "node:path";
|
||||
import { expect, it, onTestFinished } from "vitest";
|
||||
|
||||
import {
|
||||
AnalysisCleanupLease,
|
||||
AnalysisCleanupLeaseGroup,
|
||||
} from "../../../src/artifacts/AnalysisCleanupLease.js";
|
||||
import type { AnalysisClient } from "../../../src/application/AnalysisProvider.js";
|
||||
import { HopperStartError } from "../../../src/domain/hopperErrors.js";
|
||||
import { projectAnalysisError } from "../../../src/domain/analysisErrorProjection.js";
|
||||
@@ -33,6 +37,11 @@ const retryFixture = async (
|
||||
reason: "owned runtime removal denied",
|
||||
leftover_paths: [runtime],
|
||||
});
|
||||
const cleanupLease = new AnalysisCleanupLease(
|
||||
() => rm(runtime, { recursive: true, force: true }),
|
||||
[runtime],
|
||||
);
|
||||
new AnalysisCleanupLeaseGroup(cleanupError, [cleanupLease]);
|
||||
const client: AnalysisClient = {
|
||||
execute,
|
||||
operationHealthSnapshot: () => ({
|
||||
@@ -45,7 +54,7 @@ const retryFixture = async (
|
||||
close: async (options) => {
|
||||
onClose?.();
|
||||
if (!allowed) return err(cleanupError);
|
||||
await rm(runtime, { recursive: true, force: true });
|
||||
await cleanupLease.release();
|
||||
if (options?.retainDocument !== true) await rm(document, { force: true });
|
||||
return resultOk(null);
|
||||
},
|
||||
@@ -243,6 +252,12 @@ it("keeps cancellation as the primary when previous-client retirement fails afte
|
||||
});
|
||||
fixture.allowCleanup();
|
||||
expect((await fixture.session.close()).ok).toBe(true);
|
||||
expect(fixture.cleanupError.cleanupIncomplete).toBe(false);
|
||||
expect(result.error.cleanupIncomplete).toBe(false);
|
||||
expect(projectAnalysisError(result.error)).toMatchObject({
|
||||
code: "cancelled",
|
||||
});
|
||||
expect(result.error.cleanupPrimary?.cause).toBe(reason);
|
||||
});
|
||||
|
||||
it("preserves caller-selected document retention when retrying cleanup", async () => {
|
||||
|
||||
Reference in New Issue
Block a user