fix(providers): deduplicate nested cleanup notifications

This commit is contained in:
morluto committed 2026-10-27 10:45:54 +08:00
1 parent b989e3c3a3
commit 0e17aab451
3 files changed
+159 -14

No files matched your search

@@ -88,3 +88,56 @@ it("preserves the original explanation and collected output alongside cleanup un
errors: [primary, cleanup],
});
});
it.each(["return", "reject"] as const)(
"reports a nested owner's failure once when close also %ss it",
async (outcome) => {
const failure = new ProviderCleanupError("fixture", ["owned-process"], {});
const notifications: unknown[] = [];
const child: AnalysisClient = {
execute: clientClosingWith(failure).execute,
close: (options) => {
options?.onCleanupFailure?.(failure);
return outcome === "return"
? Promise.resolve(err(failure))
: Promise.reject(failure);
},
};
const parent: AnalysisClient = {
execute: child.execute,
close: (options) => closeAnalysisClient(child, "fixture", options),
};
const closed = await closeAnalysisClient(parent, "composite", {
onCleanupFailure: (error) => notifications.push(error),
});
expect(closed).toEqual(err(failure));
expect(notifications).toEqual([failure]);
},
);
it("reports distinct failures during final cleanup and reports again on retry", async () => {
const first = new ProviderCleanupError("first", ["first-resource"], {});
const final = new ProviderCleanupError("second", ["second-resource"], {});
const notifications: unknown[] = [];
const client: AnalysisClient = {
execute: clientClosingWith(first).execute,
close: (options) => {
options?.onCleanupFailure?.(first);
return Promise.resolve(err(final));
},
};
const options = {
onCleanupFailure: (error: unknown) => notifications.push(error),
};
expect(await closeAnalysisClient(client, "composite", options)).toEqual(
err(final),
);
expect(notifications).toEqual([first, final]);
expect(await closeAnalysisClient(client, "composite", options)).toEqual(
err(final),
);
expect(notifications).toEqual([first, final, first, final]);
});
@@ -12,6 +12,8 @@ import { ProviderCleanupError } from "../../domain/providerCleanupError.js";
import { projectAnalysisError } from "../../domain/analysisErrorProjection.js";
import { err, ok } from "../../domain/result.js";
import { CompositeProvider } from "./CompositeProvider.js";
import { closeAnalysisClient } from "../../providers/AnalysisClientCleanup.js";
import type { AnalysisError } from "../../domain/analysisErrorBase.js";
const identity = (id: string): ProviderIdentity => ({
id,
@@ -54,7 +56,7 @@ const failingProvider = (
},
{ remediationAction: `Resolve ${resource} before retrying ${id}.` },
);
const close = vi.fn(async () => err(cleanupError));
const close = vi.fn<AnalysisClient["close"]>(async () => err(cleanupError));
const provider: AnalysisProvider = {
identity: () => providerIdentity,
capabilities: () => [operationCapability(providerIdentity, operation)],
@@ -254,3 +256,85 @@ it.each(["busy", "unknown", "idle", undefined] as const)(
expect((await client.close()).ok).toBe(true);
},
);
it.each([1, 2])(
"reports early cleanup failures once across a deferred %s-child composite and retries",
async (childCount) => {
let release!: () => void;
const finalization = new Promise<void>((resolve) => {
release = resolve;
});
const children = [
failingProvider("provider-left", "address_name", "left-db"),
failingProvider("provider-right", "procedure_info", "right-db"),
].slice(0, childCount);
for (const child of children) {
let attempts = 0;
child.close.mockImplementation(async (options) => {
attempts += 1;
if (attempts > 2) return ok(null);
// The resource owner discovers its failure while final cleanup is
// still pending, before the composite can return its result.
options?.onCleanupFailure?.(child.cleanupError);
await finalization;
return err(child.cleanupError);
});
}
const client = new CompositeProvider(
children.map((child) => child.provider),
).createClient(executableTarget);
await client.execute("address_name", {});
if (childCount === 2) await client.execute("procedure_info", {});
const notifications: AnalysisError[] = [];
const options = {
onCleanupFailure: (failure: AnalysisError) => notifications.push(failure),
};
let settled = false;
const closing = closeAnalysisClient(client, "composite", options).then(
(result) => {
settled = true;
return result;
},
);
try {
await Promise.resolve();
expect(settled).toBe(false);
expect(notifications).toEqual(
children.map((child) => child.cleanupError),
);
} finally {
release();
}
const first = await closing;
expect(first.ok).toBe(false);
if (first.ok) throw new Error("Expected retained child cleanup");
expect(notifications).toEqual([
...children.map((child) => child.cleanupError),
...(childCount === 2 ? [first.error] : []),
]);
expect(first.error.cleanupResources).toEqual(
children.flatMap((child) => child.cleanupError.cleanupResources),
);
notifications.length = 0;
const retried = await closeAnalysisClient(client, "composite", options);
expect(retried.ok).toBe(false);
if (retried.ok) throw new Error("Expected failed cleanup retry");
expect(notifications).toEqual([
...children.map((child) => child.cleanupError),
...(childCount === 2 ? [retried.error] : []),
]);
if (childCount === 2) expect(retried.error).not.toBe(first.error);
notifications.length = 0;
expect(await closeAnalysisClient(client, "composite", options)).toEqual(
ok(null),
);
expect(notifications).toEqual([]);
for (const child of children) expect(child.close).toHaveBeenCalledTimes(3);
expect(await closeAnalysisClient(client, "composite", options)).toEqual(
ok(null),
);
for (const child of children) expect(child.close).toHaveBeenCalledTimes(3);
},
);
+21 -13
View File
@@ -1,24 +1,31 @@
import { ProviderCleanupError } from "../domain/providerCleanupError.js";
import { err, type Result } from "../domain/result.js";
import { AnalysisError } from "../domain/analysisErrorBase.js";
import type {
AnalysisClient,
ExecutionOptions,
} from "../application/AnalysisProvider.js";
import type { AnalysisClient } from "../application/AnalysisProvider.js";
/** Close one provider client and normalize unexpected adapter rejection. */
export const closeAnalysisClient = async (
client: AnalysisClient,
providerId: string,
options: Pick<ExecutionOptions, "progress"> & {
readonly retainDocument?: boolean;
readonly onCleanupFailure?: (failure: AnalysisError) => void;
} = {},
options: Parameters<AnalysisClient["close"]>[0] = {},
): Promise<Result<null, AnalysisError>> => {
// Nested/composite clients report failures before their final result settles.
// Own notification identity for this close attempt, including the fallback
// for clients that only return or reject with their cleanup failure.
const reported = new Set<AnalysisError>();
const report = (failure: AnalysisError): void => {
if (reported.has(failure)) return;
reported.add(failure);
options.onCleanupFailure?.(failure);
};
let result: Result<null, AnalysisError>;
try {
const result = await client.close(options);
if (!result.ok) options.onCleanupFailure?.(result.error);
return result;
result = await client.close({
...options,
...(options.onCleanupFailure === undefined
? {}
: { onCleanupFailure: report }),
});
} catch (cause: unknown) {
// A typed cleanup failure already names its leftover resources.
const failure =
@@ -30,7 +37,8 @@ export const closeAnalysisClient = async (
{ reason: "provider client close rejected unexpectedly" },
{ cause },
);
options.onCleanupFailure?.(failure);
return err(failure);
result = err(failure);
}
if (!result.ok) report(result.error);
return result;
};