mirror of
https://github.com/morluto/rea.git
synced 2026-10-11 21:39:55 +00:00
fix(browser): retain module resolver cleanup ownership
Serialize resolution with shutdown, retain the owned browser after close failures, and retry cleanup before another launch. Wire cleanup through the shared service and CLI/MCP lifecycles, preserving primary failures and completed native reports alongside cleanup diagnostics. Separate the browser resolution workflow from lifecycle ownership. Validation: seven resolver source tests, eleven application tests, MCP shutdown retry, public CLI parity, and full static/architecture checks passed. Real Chromium behavior remains unverified.
This commit is contained in:
@@ -38,6 +38,7 @@ export interface WebModuleResolutionBatch {
|
||||
|
||||
/** Replaceable native resolution engine, with no provider protocol in callers. */
|
||||
export interface WebModuleResolutionPort {
|
||||
close?(): Promise<void>;
|
||||
resolve(
|
||||
input: {
|
||||
readonly importerUrl: string;
|
||||
|
||||
@@ -69,6 +69,10 @@ export class WebModuleTraceService {
|
||||
readonly resolver: WebModuleResolutionPort,
|
||||
) {}
|
||||
|
||||
close(): Promise<void> {
|
||||
return this.resolver.close?.() ?? Promise.resolve();
|
||||
}
|
||||
|
||||
async trace(
|
||||
rawInput: unknown,
|
||||
options?: ExecutionOptions,
|
||||
|
||||
@@ -7,8 +7,10 @@ const input = {
|
||||
importMap: null,
|
||||
specifiers: ["./dep.js"],
|
||||
};
|
||||
describe("optional native resolution acquisition", () => {
|
||||
it("awaits owned cleanup when cancellation arrives during acquisition", async () => {
|
||||
describe("native module resolver cleanup ownership", () => {
|
||||
it.each([false, true])(
|
||||
"awaits owned cleanup when acquisition is cancelled (cleanup fails: %s)",
|
||||
async (cleanupFails) => {
|
||||
const controller = new AbortController();
|
||||
let closed = false;
|
||||
const provider = new NativeModuleResolver(
|
||||
@@ -22,33 +24,131 @@ describe("optional native resolution acquisition", () => {
|
||||
},
|
||||
close: () => {
|
||||
closed = true;
|
||||
if (cleanupFails)
|
||||
return Promise.reject(new Error("fixture cleanup refusal"));
|
||||
return Promise.resolve();
|
||||
},
|
||||
});
|
||||
},
|
||||
);
|
||||
const result = await provider.resolve(input, { signal: controller.signal });
|
||||
const result = await provider.resolve(input, {
|
||||
signal: controller.signal,
|
||||
});
|
||||
expect(closed).toBe(true);
|
||||
if (result.ok) throw new Error("expected cancelled result");
|
||||
expect(result.error._tag).toBe("AnalysisCancelledError");
|
||||
const projected = projectAnalysisError(result.error);
|
||||
expect(projected.code).toBe(
|
||||
cleanupFails ? "cleanup_incomplete" : "cancelled",
|
||||
);
|
||||
if (cleanupFails) {
|
||||
expect(projected.details).toMatchObject({
|
||||
diagnostics: { primary_error: { code: "cancelled" } },
|
||||
});
|
||||
}
|
||||
},
|
||||
);
|
||||
it("returns cleanup_incomplete when owned cleanup fails after an engine error", async () => {
|
||||
let launches = 0;
|
||||
let closes = 0;
|
||||
let cleanupAllowed = false;
|
||||
const provider = new NativeModuleResolver(
|
||||
{ REA_BROWSER_EXECUTABLE: "/browser" },
|
||||
() =>
|
||||
Promise.resolve({
|
||||
() => {
|
||||
launches += 1;
|
||||
return Promise.resolve({
|
||||
version: () => "fixture",
|
||||
newContext: () =>
|
||||
Promise.reject(new Error("fixture context failure")),
|
||||
close: () =>
|
||||
Promise.reject(new Error("fixture owned cleanup refusal")),
|
||||
}),
|
||||
close: async () => {
|
||||
closes += 1;
|
||||
if (!cleanupAllowed)
|
||||
throw new Error("fixture owned cleanup refusal");
|
||||
},
|
||||
});
|
||||
},
|
||||
);
|
||||
const result = await provider.resolve(input);
|
||||
if (result.ok) throw new Error("expected cleanup failure");
|
||||
expect(projectAnalysisError(result.error).code).toBe("cleanup_incomplete");
|
||||
expect(result.error.cleanupIncomplete).toBe(true);
|
||||
expect(projectAnalysisError(result.error).details).toMatchObject({
|
||||
diagnostics: {
|
||||
primary_error: {
|
||||
details: {
|
||||
diagnostics: { error_message: "fixture context failure" },
|
||||
},
|
||||
},
|
||||
cleanup_error: {
|
||||
details: { diagnostics: { reason: "fixture owned cleanup refusal" } },
|
||||
},
|
||||
},
|
||||
});
|
||||
const blocked = await provider.resolve(input);
|
||||
expect(blocked.ok).toBe(false);
|
||||
if (!blocked.ok) expect(blocked.error.cleanupIncomplete).toBe(true);
|
||||
expect(launches).toBe(1);
|
||||
await expect(provider.close()).rejects.toMatchObject({
|
||||
cleanupIncomplete: true,
|
||||
});
|
||||
cleanupAllowed = true;
|
||||
await provider.close();
|
||||
await provider.close();
|
||||
expect(closes).toBe(4);
|
||||
expect(launches).toBe(1);
|
||||
});
|
||||
it("awaits acquisition and cleanup while refusing queued work during shutdown", async () => {
|
||||
let acquired: () => void = () => {
|
||||
throw new Error("acquisition gate missing");
|
||||
};
|
||||
const acquiring = new Promise<void>((resolve) => {
|
||||
acquired = resolve;
|
||||
});
|
||||
let release: () => void = () => {
|
||||
throw new Error("release gate missing");
|
||||
};
|
||||
const released = new Promise<void>((resolve) => {
|
||||
release = resolve;
|
||||
});
|
||||
let launches = 0;
|
||||
let closes = 0;
|
||||
const provider = new NativeModuleResolver(
|
||||
{ REA_BROWSER_EXECUTABLE: "/browser" },
|
||||
async () => {
|
||||
launches += 1;
|
||||
acquired();
|
||||
await released;
|
||||
return {
|
||||
version: () => "fixture",
|
||||
newContext: () =>
|
||||
Promise.reject(new Error("fixture context failure")),
|
||||
close: async () => {
|
||||
closes += 1;
|
||||
},
|
||||
};
|
||||
},
|
||||
);
|
||||
const first = provider.resolve(input);
|
||||
await acquiring;
|
||||
const queued = provider.resolve(input);
|
||||
const closing = provider.close();
|
||||
let closed = false;
|
||||
void closing.then(() => {
|
||||
closed = true;
|
||||
});
|
||||
await Promise.resolve();
|
||||
expect(closed).toBe(false);
|
||||
release();
|
||||
await first;
|
||||
const blocked = await queued;
|
||||
expect(blocked.ok).toBe(false);
|
||||
await closing;
|
||||
expect(closed).toBe(true);
|
||||
expect([launches, closes]).toEqual([1, 1]);
|
||||
expect((await provider.resolve(input)).ok).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("optional native resolution acquisition", () => {
|
||||
it("does not load the browser engine when the selected operation lacks its configuration", async () => {
|
||||
const provider = new NativeModuleResolver({}, () => {
|
||||
throw new Error("must not launch");
|
||||
|
||||
@@ -7,6 +7,7 @@ import type {
|
||||
WebModuleResolutionBatch,
|
||||
} from "../../application/WebModulePorts.js";
|
||||
import type { AnalysisError } from "../../domain/analysisErrorBase.js";
|
||||
import { analysisErrorWithCleanupFailure } from "../../domain/analysisErrorCleanup.js";
|
||||
import {
|
||||
AnalysisCancelledError,
|
||||
AnalysisCapabilityUnavailableError,
|
||||
@@ -16,6 +17,7 @@ import {
|
||||
import { BrowserObservationError } from "../../domain/browserObservationError.js";
|
||||
import { jsonObjectSchema } from "../../domain/jsonValue.js";
|
||||
import { ProviderAdapterError } from "../../domain/providerAdapterError.js";
|
||||
import { ProviderCleanupError } from "../../domain/providerCleanupError.js";
|
||||
import { err, ok, type Result } from "../../domain/result.js";
|
||||
import { webModuleResolutionSchema } from "../../domain/webModuleTrace.js";
|
||||
import { ProviderStartupDeadline } from "../../process/ProviderDeadline.js";
|
||||
@@ -32,26 +34,49 @@ const reportSchema = z.strictObject({
|
||||
),
|
||||
});
|
||||
|
||||
type ModuleResolverBrowser = Pick<Browser, "newContext" | "version" | "close">;
|
||||
/** Production launch seam: only the adapter knows the browser API. */
|
||||
export type ModuleResolverLauncher = (
|
||||
options: LaunchOptions,
|
||||
) => Promise<Pick<Browser, "newContext" | "version" | "close">>;
|
||||
) => Promise<ModuleResolverBrowser>;
|
||||
const launch: ModuleResolverLauncher = async (options) =>
|
||||
(await import("playwright-core")).chromium.launch(options);
|
||||
|
||||
/** Use native import.meta.resolve in an owned context with locally fulfilled requests. */
|
||||
export class NativeModuleResolver implements WebModuleResolutionPort {
|
||||
#tail: Promise<void> = Promise.resolve();
|
||||
#closing = false;
|
||||
#closePromise: Promise<void> | undefined;
|
||||
#pendingBrowser: ModuleResolverBrowser | undefined;
|
||||
|
||||
constructor(
|
||||
readonly environment: Readonly<Record<string, string | undefined>>,
|
||||
readonly launcher: ModuleResolverLauncher = launch,
|
||||
) {}
|
||||
|
||||
async resolve(
|
||||
resolve(
|
||||
input: Parameters<WebModuleResolutionPort["resolve"]>[0],
|
||||
options?: ExecutionOptions,
|
||||
): ReturnType<WebModuleResolutionPort["resolve"]> {
|
||||
if (this.#closing) return Promise.resolve(err(this.#closedError()));
|
||||
const operation = this.#tail.then(() =>
|
||||
this.#closing ? err(this.#closedError()) : this.#resolve(input, options),
|
||||
);
|
||||
this.#tail = operation.then(
|
||||
() => undefined,
|
||||
() => undefined,
|
||||
);
|
||||
return operation;
|
||||
}
|
||||
|
||||
async #resolve(
|
||||
input: Parameters<WebModuleResolutionPort["resolve"]>[0],
|
||||
options?: ExecutionOptions,
|
||||
): ReturnType<WebModuleResolutionPort["resolve"]> {
|
||||
if (options?.signal?.aborted === true)
|
||||
return err(new AnalysisCancelledError(OPERATION));
|
||||
const pending = await this.#retryCleanup();
|
||||
if (pending !== undefined) return err(pending);
|
||||
const executable = this.environment.REA_BROWSER_EXECUTABLE;
|
||||
if (executable === undefined || !isAbsolute(executable))
|
||||
return err(
|
||||
@@ -66,11 +91,16 @@ export class NativeModuleResolver implements WebModuleResolutionPort {
|
||||
),
|
||||
);
|
||||
const deadline = new ProviderStartupDeadline(TIMEOUT_MS, options?.signal);
|
||||
let browser: Pick<Browser, "newContext" | "version" | "close"> | undefined;
|
||||
let browser: ModuleResolverBrowser | undefined;
|
||||
let cleanup: Promise<void> | undefined;
|
||||
const close = (): Promise<void> => {
|
||||
if (cleanup === undefined && browser !== undefined)
|
||||
cleanup = browser.close();
|
||||
if (cleanup === undefined && browser !== undefined) {
|
||||
const owned = browser;
|
||||
cleanup = Promise.resolve().then(async () => {
|
||||
await owned.close();
|
||||
this.#pendingBrowser = undefined;
|
||||
});
|
||||
}
|
||||
return cleanup ?? Promise.resolve();
|
||||
};
|
||||
const onAbort = (): void => {
|
||||
@@ -82,6 +112,7 @@ export class NativeModuleResolver implements WebModuleResolutionPort {
|
||||
deadline.signal.addEventListener("abort", onAbort, { once: true });
|
||||
let result: Result<WebModuleResolutionBatch, AnalysisError>;
|
||||
try {
|
||||
deadline.signal.throwIfAborted();
|
||||
browser = await this.launcher({
|
||||
executablePath: executable,
|
||||
headless: true,
|
||||
@@ -99,7 +130,89 @@ export class NativeModuleResolver implements WebModuleResolutionPort {
|
||||
: []),
|
||||
],
|
||||
});
|
||||
this.#pendingBrowser = browser;
|
||||
deadline.signal.throwIfAborted();
|
||||
result = ok(await resolveInBrowser(browser, input, deadline, executable));
|
||||
} catch (cause: unknown) {
|
||||
result = err(
|
||||
deadline.interruption === "cancelled"
|
||||
? new AnalysisCancelledError(OPERATION)
|
||||
: deadline.interruption === "timeout"
|
||||
? new AnalysisTimeoutError(OPERATION, TIMEOUT_MS)
|
||||
: cause instanceof AnalysisOutputError ||
|
||||
cause instanceof AnalysisCapabilityUnavailableError ||
|
||||
cause instanceof BrowserObservationError
|
||||
? cause
|
||||
: new ProviderAdapterError("native-module-resolver", OPERATION, {
|
||||
cause,
|
||||
diagnostics: {
|
||||
executable,
|
||||
error_message:
|
||||
cause instanceof Error ? cause.message : String(cause),
|
||||
},
|
||||
}),
|
||||
);
|
||||
} finally {
|
||||
deadline.signal.removeEventListener("abort", onAbort);
|
||||
deadline.dispose();
|
||||
}
|
||||
try {
|
||||
await close();
|
||||
} catch (cause: unknown) {
|
||||
const failure = cleanupFailure(
|
||||
cause,
|
||||
result.ok ? result.value.rawResult : undefined,
|
||||
);
|
||||
return err(
|
||||
result.ok
|
||||
? failure
|
||||
: analysisErrorWithCleanupFailure(result.error, failure, OPERATION),
|
||||
);
|
||||
}
|
||||
if (result.ok && options?.signal?.aborted)
|
||||
return err(new AnalysisCancelledError(OPERATION));
|
||||
return result;
|
||||
}
|
||||
|
||||
async close(): Promise<void> {
|
||||
this.#closing = true;
|
||||
this.#closePromise ??= this.#tail
|
||||
.then(async () => {
|
||||
const failure = await this.#retryCleanup();
|
||||
if (failure !== undefined) throw failure;
|
||||
})
|
||||
.catch((cause: unknown) => {
|
||||
this.#closePromise = undefined;
|
||||
throw cause;
|
||||
});
|
||||
return this.#closePromise;
|
||||
}
|
||||
|
||||
#closedError(): AnalysisError {
|
||||
return new BrowserObservationError(OPERATION, "cleanup_failed", {
|
||||
detail: "Native module resolver is closing.",
|
||||
});
|
||||
}
|
||||
|
||||
async #retryCleanup(): Promise<AnalysisError | undefined> {
|
||||
const browser = this.#pendingBrowser;
|
||||
if (browser === undefined) return undefined;
|
||||
try {
|
||||
await browser.close();
|
||||
this.#pendingBrowser = undefined;
|
||||
return undefined;
|
||||
} catch (cause: unknown) {
|
||||
return cleanupFailure(cause);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const resolveInBrowser = async (
|
||||
browser: ModuleResolverBrowser,
|
||||
input: Parameters<WebModuleResolutionPort["resolve"]>[0],
|
||||
deadline: ProviderStartupDeadline,
|
||||
executable: string,
|
||||
): Promise<WebModuleResolutionBatch> => {
|
||||
const context = await browser.newContext({
|
||||
serviceWorkers: "block",
|
||||
acceptDownloads: false,
|
||||
@@ -223,7 +336,7 @@ export class NativeModuleResolver implements WebModuleResolutionPort {
|
||||
id: "chromium-native-module-resolver",
|
||||
version: browser.version(),
|
||||
};
|
||||
result = ok({
|
||||
return {
|
||||
engine,
|
||||
resolutions: parsed.data.entries.map((entry) => entry.resolution),
|
||||
diagnostics,
|
||||
@@ -234,42 +347,24 @@ export class NativeModuleResolver implements WebModuleResolutionPort {
|
||||
engine,
|
||||
executable_path: executable,
|
||||
}),
|
||||
});
|
||||
} catch (cause: unknown) {
|
||||
result = err(
|
||||
deadline.interruption === "cancelled"
|
||||
? new AnalysisCancelledError(OPERATION)
|
||||
: deadline.interruption === "timeout"
|
||||
? new AnalysisTimeoutError(OPERATION, TIMEOUT_MS)
|
||||
: cause instanceof AnalysisOutputError ||
|
||||
cause instanceof AnalysisCapabilityUnavailableError ||
|
||||
cause instanceof BrowserObservationError
|
||||
? cause
|
||||
: new ProviderAdapterError("native-module-resolver", OPERATION, {
|
||||
cause,
|
||||
diagnostics: {
|
||||
executable,
|
||||
error_message:
|
||||
cause instanceof Error ? cause.message : String(cause),
|
||||
};
|
||||
};
|
||||
|
||||
const cleanupFailure = (
|
||||
cause: unknown,
|
||||
observation?: WebModuleResolutionBatch["rawResult"],
|
||||
): ProviderCleanupError =>
|
||||
new ProviderCleanupError(
|
||||
"native-module-resolver",
|
||||
["chromium-browser"],
|
||||
{
|
||||
reason: cause instanceof Error ? cause.message : String(cause),
|
||||
...(observation === undefined
|
||||
? {}
|
||||
: { partial_observation: observation }),
|
||||
},
|
||||
}),
|
||||
{ cause, operation: OPERATION },
|
||||
);
|
||||
} finally {
|
||||
deadline.signal.removeEventListener("abort", onAbort);
|
||||
deadline.dispose();
|
||||
try {
|
||||
await close();
|
||||
} catch (cause: unknown) {
|
||||
result = err(
|
||||
new BrowserObservationError(OPERATION, "cleanup_failed", { cause }),
|
||||
);
|
||||
}
|
||||
}
|
||||
if (result.ok && options?.signal?.aborted)
|
||||
return err(new AnalysisCancelledError(OPERATION));
|
||||
return result;
|
||||
}
|
||||
}
|
||||
|
||||
const scriptJson = (value: unknown): string =>
|
||||
JSON.stringify(value).replaceAll("<", "\\u003c");
|
||||
|
||||
@@ -42,8 +42,12 @@ export const registerWebModuleCommands = (
|
||||
.describe("HTTP(S) import-map base URL"),
|
||||
}),
|
||||
run: ({ args, options }) =>
|
||||
withCommandCancellation((signal) =>
|
||||
logCliCommand(logger, CLI_COMMANDS.traceWebModuleImports, async () => {
|
||||
withCommandCancellation(async (signal) => {
|
||||
try {
|
||||
return await logCliCommand(
|
||||
logger,
|
||||
CLI_COMMANDS.traceWebModuleImports,
|
||||
async () => {
|
||||
const result = await service.trace(
|
||||
{
|
||||
manifest_path: args.manifestPath,
|
||||
@@ -63,8 +67,14 @@ export const registerWebModuleCommands = (
|
||||
},
|
||||
{ signal },
|
||||
);
|
||||
return result.ok ? result.value : projectAnalysisError(result.error);
|
||||
return result.ok
|
||||
? result.value
|
||||
: projectAnalysisError(result.error);
|
||||
},
|
||||
);
|
||||
} finally {
|
||||
await service.close();
|
||||
}
|
||||
}),
|
||||
),
|
||||
});
|
||||
};
|
||||
|
||||
@@ -216,6 +216,8 @@ export const createServer = (
|
||||
options.evmInterface ?? createEvmInterfaceService(environment);
|
||||
const webSourceLocation =
|
||||
options.webSourceLocation ?? createWebSourceLocationService(environment);
|
||||
const webModuleTrace =
|
||||
options.webModuleTrace ?? createWebModuleTraceService(environment);
|
||||
const javascriptRecovery =
|
||||
options.javascriptRecovery ?? createJavaScriptRecoveryProvider(environment);
|
||||
const artifactResources = new ArtifactResourceScope();
|
||||
@@ -228,6 +230,7 @@ export const createServer = (
|
||||
delivery,
|
||||
evmInterface,
|
||||
webSourceLocation,
|
||||
webModuleTrace,
|
||||
javascriptRecovery,
|
||||
};
|
||||
const startedAt = new Date().toISOString();
|
||||
@@ -257,6 +260,7 @@ export const createServer = (
|
||||
adbDevice.close(),
|
||||
evmInterface.close(),
|
||||
webSourceLocation.close(),
|
||||
webModuleTrace.close(),
|
||||
javascriptRecovery.close?.() ?? Promise.resolve(),
|
||||
artifactResources.close(),
|
||||
firmwareAnalysis.close(),
|
||||
|
||||
@@ -81,3 +81,32 @@ it("publishes valid SDK schemas and records the named module trace without a bin
|
||||
});
|
||||
expect(invalid.isError).toBe(true);
|
||||
});
|
||||
|
||||
it("retains the module resolver cleanup owner across failed server shutdown", async () => {
|
||||
let closes = 0;
|
||||
const service = new WebModuleTraceService(
|
||||
{ load: () => Promise.resolve(ok(webModuleArtifactsFixture())) },
|
||||
{
|
||||
...webModuleResolverFixture,
|
||||
close: async () => {
|
||||
closes += 1;
|
||||
if (closes === 1) throw new Error("fixture browser close failed");
|
||||
},
|
||||
},
|
||||
);
|
||||
const session = createTestBinarySession(() => {
|
||||
throw new Error("binary provider must not start");
|
||||
});
|
||||
const server = createServer(
|
||||
{ kind: "session", session },
|
||||
{ webModuleTrace: service },
|
||||
);
|
||||
onTestFinished(async () => {
|
||||
await server.close();
|
||||
await session.close();
|
||||
});
|
||||
await expect(server.close()).rejects.toBeInstanceOf(AggregateError);
|
||||
await server.close();
|
||||
await server.close();
|
||||
expect(closes).toBe(2);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user