mirror of
https://github.com/morluto/rea.git
synced 2026-11-09 16:26:36 +00:00
fix(reference): preserve import cancellation provenance (#9597)
* fix(reference): preserve import cancellation reason * test(reference): cover null cancellation during failed cleanup * test(reference): always release fixture directory
This commit is contained in:
1 parent
e66e92904e
commit
e8d1bbc98b
9 files changed
+149
-31
No files matched your search
@@ -39,9 +39,11 @@ const failure = (
|
||||
});
|
||||
|
||||
const cancelled = (
|
||||
signal: AbortSignal | undefined,
|
||||
partial?: ReferenceSourceRead,
|
||||
): ReferenceSourceImportError => ({
|
||||
...failure("cancelled", "Reference source import cancelled"),
|
||||
...(signal?.aborted === true ? { cause: signal.reason } : {}),
|
||||
...(partial === undefined ? {} : { partial }),
|
||||
});
|
||||
|
||||
@@ -168,11 +170,11 @@ export const importReferenceSource = async (
|
||||
options: ReferenceSourceImportOptions,
|
||||
resources: ArtifactResourceScope,
|
||||
): Promise<Result<HistoricalSourceGraph, ReferenceSourceImportError>> => {
|
||||
if (isAborted(options.signal)) return err(cancelled());
|
||||
if (isAborted(options.signal)) return err(cancelled(options.signal));
|
||||
const prepared = await prepareReferenceSourceImport(options, resources);
|
||||
if (!prepared.ok) return prepared;
|
||||
const { ignored, root, secrets } = prepared.value;
|
||||
if (isAborted(options.signal)) return err(cancelled());
|
||||
if (isAborted(options.signal)) return err(cancelled(options.signal));
|
||||
|
||||
const exclusions: HistoricalSourceGraphInput["exclusions"] = [];
|
||||
const shouldExclude = createShouldExclude(exclusions, secrets, ignored);
|
||||
@@ -201,13 +203,14 @@ export const importReferenceSource = async (
|
||||
|
||||
const vcs = vcsResult.value;
|
||||
|
||||
if (isAborted(options.signal)) return err(cancelled(readResult.value));
|
||||
if (isAborted(options.signal))
|
||||
return err(cancelled(options.signal, readResult.value));
|
||||
|
||||
const read = readResult.value;
|
||||
const { entries, relationships, parseFailures, limitations } =
|
||||
parseReferenceSourceEntries(read, exclusions, options.signal);
|
||||
|
||||
if (isAborted(options.signal)) return err(cancelled(read));
|
||||
if (isAborted(options.signal)) return err(cancelled(options.signal, read));
|
||||
|
||||
const uniqueRelationships = deduplicateRelationships(relationships);
|
||||
const uniqueFailures = normalizeHistoricalSourceParseFailures(parseFailures);
|
||||
|
||||
@@ -80,11 +80,17 @@ const resolveRoot = async (
|
||||
signal?: AbortSignal,
|
||||
): Promise<Result<string, ReferenceSourceImportError>> => {
|
||||
if (isAborted(signal))
|
||||
return err(failure("cancelled", "Reference source import cancelled"));
|
||||
return err({
|
||||
...failure("cancelled", "Reference source import cancelled"),
|
||||
cause: signal?.reason,
|
||||
});
|
||||
try {
|
||||
const metadata = await stat(requestedRoot);
|
||||
if (isAborted(signal))
|
||||
return err(failure("cancelled", "Reference source import cancelled"));
|
||||
return err({
|
||||
...failure("cancelled", "Reference source import cancelled"),
|
||||
cause: signal?.reason,
|
||||
});
|
||||
if (!metadata.isDirectory())
|
||||
return err(
|
||||
failure(
|
||||
@@ -94,11 +100,17 @@ const resolveRoot = async (
|
||||
);
|
||||
const canonicalRoot = await realpath(requestedRoot);
|
||||
if (isAborted(signal))
|
||||
return err(failure("cancelled", "Reference source import cancelled"));
|
||||
return err({
|
||||
...failure("cancelled", "Reference source import cancelled"),
|
||||
cause: signal?.reason,
|
||||
});
|
||||
return ok(canonicalRoot);
|
||||
} catch (cause: unknown) {
|
||||
if (isAborted(signal))
|
||||
return err(failure("cancelled", "Reference source import cancelled"));
|
||||
return err({
|
||||
...failure("cancelled", "Reference source import cancelled"),
|
||||
cause: signal?.reason,
|
||||
});
|
||||
const rootFailure = rootFilesystemFailure(
|
||||
cause,
|
||||
"Reference source root could not be resolved",
|
||||
@@ -242,7 +254,11 @@ const readIgnoreCase = async (
|
||||
} catch (cause: unknown) {
|
||||
if (readFailure !== undefined) return err(readFailure);
|
||||
if (isAborted(signal) || isIgnorePolicyCancellation(cause))
|
||||
return err(failure("cancelled", "Reference source import cancelled"));
|
||||
return err({
|
||||
...failure("cancelled", "Reference source import cancelled"),
|
||||
cause:
|
||||
signal?.aborted === true ? signal.reason : ignorePolicyCause(cause),
|
||||
});
|
||||
return err({
|
||||
...failure(
|
||||
"io",
|
||||
|
||||
@@ -19,7 +19,10 @@ import {
|
||||
noFollowOpenSupported,
|
||||
prepareRoot,
|
||||
} from "./ReferenceSourceReaderValidate.js";
|
||||
import { referenceSourceResourceFailure } from "./ReferenceSourceReaderErrors.js";
|
||||
import {
|
||||
cancelled,
|
||||
referenceSourceResourceFailure,
|
||||
} from "./ReferenceSourceReaderErrors.js";
|
||||
|
||||
const PATH_RACE_LIMITATION =
|
||||
"Path identity is revalidated around operations; Node lacks portable descriptor-relative openat traversal, so a syscall-boundary pathname race remains.";
|
||||
@@ -199,12 +202,7 @@ const traverse = async (
|
||||
state: TraversalState,
|
||||
): Promise<ReferenceSourceResult<undefined>> => {
|
||||
while (state.pending.length > 0) {
|
||||
if (isAborted(state.signal))
|
||||
return err({
|
||||
tag: "reference-source-reader",
|
||||
code: "cancelled",
|
||||
message: "Reference source traversal cancelled",
|
||||
});
|
||||
if (isAborted(state.signal)) return err(cancelled(state.signal));
|
||||
const current = state.pending.pop();
|
||||
if (current === undefined) break;
|
||||
const result = await traverseDirectory(state, current);
|
||||
|
||||
@@ -69,7 +69,8 @@ export const traverseDirectory = async (
|
||||
current.path,
|
||||
state.signal,
|
||||
);
|
||||
if (isAborted(state.signal)) return { ok: false, error: cancelled() };
|
||||
if (isAborted(state.signal))
|
||||
return { ok: false, error: cancelled(state.signal) };
|
||||
if (!before.ok) {
|
||||
recordDirectoryFailure(
|
||||
state,
|
||||
@@ -103,11 +104,12 @@ export const traverseDirectory = async (
|
||||
// A directory that cannot be opened retains its ordinary partial-entry
|
||||
// failure; only an opened directory needs policy before enumeration.
|
||||
result = isAborted(state.signal)
|
||||
? err(cancelled())
|
||||
? err(cancelled(state.signal))
|
||||
: ((await state.beforeDirectory?.(
|
||||
pathFromRoot(state.root, current.path),
|
||||
)) ?? ok(undefined));
|
||||
if (result.ok && isAborted(state.signal)) result = err(cancelled());
|
||||
if (result.ok && isAborted(state.signal))
|
||||
result = err(cancelled(state.signal));
|
||||
while (result.ok) {
|
||||
let child: Dirent | null;
|
||||
try {
|
||||
@@ -154,7 +156,7 @@ export const traverseDirectory = async (
|
||||
const previous = result.ok ? undefined : result.error.cleanup;
|
||||
const primary = result.ok
|
||||
? isAborted(state.signal)
|
||||
? cancelled()
|
||||
? cancelled(state.signal)
|
||||
: failure(
|
||||
"io",
|
||||
readFailure?.localDiagnostic ??
|
||||
@@ -173,12 +175,13 @@ export const traverseDirectory = async (
|
||||
...new Set([...previous.resources, ...observation.resources]),
|
||||
],
|
||||
},
|
||||
cause: primary.cause ?? cleanup.cause,
|
||||
cause: Object.hasOwn(primary, "cause") ? primary.cause : cleanup.cause,
|
||||
});
|
||||
}
|
||||
if (!result.ok) return result;
|
||||
}
|
||||
if (isAborted(state.signal)) return { ok: false, error: cancelled() };
|
||||
if (isAborted(state.signal))
|
||||
return { ok: false, error: cancelled(state.signal) };
|
||||
if (readFailure !== undefined) {
|
||||
recordDirectoryFailure(
|
||||
state,
|
||||
@@ -196,7 +199,8 @@ export const traverseDirectory = async (
|
||||
current.path,
|
||||
state.signal,
|
||||
);
|
||||
if (isAborted(state.signal)) return { ok: false, error: cancelled() };
|
||||
if (isAborted(state.signal))
|
||||
return { ok: false, error: cancelled(state.signal) };
|
||||
if (!after.ok || !sameFile(before.stats, after.stats)) {
|
||||
recordDirectoryFailure(
|
||||
state,
|
||||
@@ -263,11 +267,13 @@ const processEntry = async (
|
||||
name: string,
|
||||
directories: PendingDirectory[],
|
||||
): Promise<ReferenceSourceResult<undefined>> => {
|
||||
if (isAborted(state.signal)) return { ok: false, error: cancelled() };
|
||||
if (isAborted(state.signal))
|
||||
return { ok: false, error: cancelled(state.signal) };
|
||||
const absolute = join(current.path, name);
|
||||
const path = pathFromRoot(state.root, absolute);
|
||||
const metadata = await readMetadata(absolute);
|
||||
if (isAborted(state.signal)) return { ok: false, error: cancelled() };
|
||||
if (isAborted(state.signal))
|
||||
return { ok: false, error: cancelled(state.signal) };
|
||||
if (!metadata.ok) {
|
||||
const excluded = applyExclusion(state.shouldExclude, path, "unknown");
|
||||
if (!excluded.ok)
|
||||
@@ -312,7 +318,8 @@ const processEntry = async (
|
||||
state.exclusions.push({ path, kind });
|
||||
return { ok: true, value: undefined };
|
||||
}
|
||||
if (isAborted(state.signal)) return { ok: false, error: cancelled() };
|
||||
if (isAborted(state.signal))
|
||||
return { ok: false, error: cancelled(state.signal) };
|
||||
if (kind === "symlink")
|
||||
state.entries.push(
|
||||
await readStableSymlinkTarget(
|
||||
@@ -370,6 +377,11 @@ const processFileEntry = async (
|
||||
? entry.message
|
||||
: `Reference file cleanup failed: ${path}`,
|
||||
cleanup: result.cleanup,
|
||||
...(entry.status === "failed" &&
|
||||
entry.code === "cancelled" &&
|
||||
state.signal?.aborted === true
|
||||
? { cause: state.signal.reason }
|
||||
: {}),
|
||||
});
|
||||
return ok(undefined);
|
||||
};
|
||||
|
||||
@@ -14,8 +14,12 @@ export const failure = (
|
||||
message,
|
||||
});
|
||||
|
||||
export const cancelled = (): ReferenceSourceReaderError =>
|
||||
failure("cancelled", "Reference source traversal cancelled");
|
||||
export const cancelled = (
|
||||
signal?: AbortSignal,
|
||||
): ReferenceSourceReaderError => ({
|
||||
...failure("cancelled", "Reference source traversal cancelled"),
|
||||
...(signal?.aborted === true ? { cause: signal.reason } : {}),
|
||||
});
|
||||
|
||||
/** Project resource failures into the reference-source error contract. */
|
||||
export const referenceSourceResourceFailure = (
|
||||
|
||||
@@ -99,7 +99,7 @@ export const prepareRoot = async (
|
||||
readonly rootIdentity: BigIntStats;
|
||||
}>
|
||||
> => {
|
||||
if (isAborted(signal)) return err(cancelled());
|
||||
if (isAborted(signal)) return err(cancelled(signal));
|
||||
try {
|
||||
const metadata = await bigLstat(root);
|
||||
if (metadata.isSymbolicLink() || !metadata.isDirectory())
|
||||
@@ -110,7 +110,7 @@ export const prepareRoot = async (
|
||||
),
|
||||
);
|
||||
const canonicalRoot = await realpath(root);
|
||||
if (isAborted(signal)) return err(cancelled());
|
||||
if (isAborted(signal)) return err(cancelled(signal));
|
||||
const canonicalMetadata = await bigLstat(canonicalRoot);
|
||||
if (!sameFile(metadata, canonicalMetadata))
|
||||
return err(
|
||||
@@ -121,7 +121,7 @@ export const prepareRoot = async (
|
||||
);
|
||||
return ok({ canonicalRoot, rootIdentity: canonicalMetadata });
|
||||
} catch (cause: unknown) {
|
||||
if (isAborted(signal)) return err(cancelled());
|
||||
if (isAborted(signal)) return err(cancelled(signal));
|
||||
const rootFailure = rootFilesystemFailure(
|
||||
cause,
|
||||
"Reference source root could not be resolved",
|
||||
|
||||
@@ -411,3 +411,53 @@ it("keeps cancelled traversal typed when native directory close rejects after in
|
||||
});
|
||||
expect(closeCalls).toBe(1);
|
||||
});
|
||||
|
||||
it("preserves null cancellation and retained directory cleanup across retry", async () => {
|
||||
const root = await createTestTempDirectory("rea-reference-null-close-");
|
||||
const resources = new ArtifactResourceScope();
|
||||
onTestFinished(() => resources.close().catch(() => undefined));
|
||||
const controller = new AbortController();
|
||||
const closeFailure = new Error("directory close temporarily denied");
|
||||
let closeCalls = 0;
|
||||
let releaseFixture: (() => Promise<void>) | undefined;
|
||||
onTestFinished(() => releaseFixture?.());
|
||||
opendirMock.mockImplementation(async (...args) => {
|
||||
const directory = await actual.opendir(...args);
|
||||
const operations: DirectoryOperations = directory;
|
||||
const close = operations.close.bind(directory);
|
||||
releaseFixture = close;
|
||||
vi.spyOn(operations, "close").mockImplementation(async () => {
|
||||
closeCalls += 1;
|
||||
if (closeCalls === 1) throw closeFailure;
|
||||
await close();
|
||||
});
|
||||
return directory;
|
||||
});
|
||||
const result = await readReferenceSource(root, resources, {
|
||||
signal: controller.signal,
|
||||
beforeDirectory: async () => {
|
||||
controller.abort(null);
|
||||
return ok(undefined);
|
||||
},
|
||||
});
|
||||
if (result.ok) throw new Error("Expected cancellation and cleanup failure");
|
||||
expect(result.error.code).toBe("cancelled");
|
||||
expect(result.error.cause).toBeNull();
|
||||
expect(result.error.cleanup).toEqual({
|
||||
resources: [root],
|
||||
reason: expect.stringContaining(closeFailure.message),
|
||||
});
|
||||
expect(result.error.partial).toMatchObject({ entries: [], bytesRead: 0 });
|
||||
expect(resources.hasPending).toBe(true);
|
||||
expect(resources.cleanupResources).toEqual([root]);
|
||||
expect(closeCalls).toBe(1);
|
||||
await expect(resources.close()).rejects.toMatchObject({
|
||||
cleanup: {
|
||||
resources: [root],
|
||||
reason: expect.stringContaining(closeFailure.message),
|
||||
},
|
||||
});
|
||||
expect(resources.hasPending).toBe(true);
|
||||
expect(resources.cleanupResources).toEqual([root]);
|
||||
expect(closeCalls).toBe(1);
|
||||
});
|
||||
@@ -801,6 +801,18 @@ describe("reference source import behavior", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it.each([null, "stop import", { requested: "stop import" }])(
|
||||
"preserves the caller's pre-admission cancellation reason %j",
|
||||
async (reason) => {
|
||||
const controller = new AbortController();
|
||||
controller.abort(reason);
|
||||
const result = await importTree("unopened-source", controller.signal);
|
||||
if (result.ok) throw new Error("Expected cancellation");
|
||||
expect(result.error.code).toBe("cancelled");
|
||||
expect(result.error.cause).toBe(reason);
|
||||
},
|
||||
);
|
||||
|
||||
it("records bounded local Git state without invoking Git", async () => {
|
||||
const parent = await createTestTempDirectory("rea-reference-git-");
|
||||
try {
|
||||
|
||||
@@ -7,6 +7,28 @@ import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
|
||||
|
||||
import { readStableSymlinkTarget } from "../../../src/reference/ReferenceSourceReaderEntries.js";
|
||||
import { readReferenceSource } from "../../support/referenceSourceResourceScope.js";
|
||||
import { ok } from "../../../src/domain/result.js";
|
||||
|
||||
it("retains the caller reason and observed entries when traversal is cancelled", async () => {
|
||||
const root = await createTestTempDirectory("rea-reference-cancel-");
|
||||
await writeFile(join(root, "observed.js"), "observed");
|
||||
await mkdir(join(root, "nested"));
|
||||
const controller = new AbortController();
|
||||
const reason = { requested: "stop traversal" };
|
||||
const result = await readReferenceSource(root, {
|
||||
signal: controller.signal,
|
||||
beforeDirectory: async (path) => {
|
||||
if (path === "nested") controller.abort(reason);
|
||||
return ok(undefined);
|
||||
},
|
||||
});
|
||||
if (result.ok) throw new Error("Expected cancellation");
|
||||
expect(result.error.code).toBe("cancelled");
|
||||
expect(result.error.cause).toBe(reason);
|
||||
expect(result.error.partial?.entries).toContainEqual(
|
||||
expect.objectContaining({ path: "observed.js", status: "read" }),
|
||||
);
|
||||
});
|
||||
|
||||
describe("readReferenceSource entries", () => {
|
||||
it("returns explicit entries in canonical code-point path order", async () => {
|
||||
@@ -231,6 +253,7 @@ describe("readReferenceSource failures and exclusions", () => {
|
||||
tag: "reference-source-reader",
|
||||
code: "cancelled",
|
||||
message: "Reference source traversal cancelled",
|
||||
cause: controller.signal.reason,
|
||||
},
|
||||
});
|
||||
|
||||
|
||||
Reference in new issue
Block a user