diff --git a/src/application/binary/AnalysisClientCleanup.test.ts b/src/application/binary/AnalysisClientCleanup.test.ts index 23944836c3..4dff023c8d 100644 --- a/src/application/binary/AnalysisClientCleanup.test.ts +++ b/src/application/binary/AnalysisClientCleanup.test.ts @@ -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]); +}); diff --git a/src/application/binary/CompositeProvider.test.ts b/src/application/binary/CompositeProvider.test.ts index b0e7a37a6b..b35aec68af 100644 --- a/src/application/binary/CompositeProvider.test.ts +++ b/src/application/binary/CompositeProvider.test.ts @@ -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(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((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); + }, +); diff --git a/src/providers/AnalysisClientCleanup.ts b/src/providers/AnalysisClientCleanup.ts index 6bb3057687..c80e5cd168 100644 --- a/src/providers/AnalysisClientCleanup.ts +++ b/src/providers/AnalysisClientCleanup.ts @@ -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 & { - readonly retainDocument?: boolean; - readonly onCleanupFailure?: (failure: AnalysisError) => void; - } = {}, + options: Parameters[0] = {}, ): Promise> => { + // 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(); + const report = (failure: AnalysisError): void => { + if (reported.has(failure)) return; + reported.add(failure); + options.onCleanupFailure?.(failure); + }; + let result: Result; 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; };