mirror of
https://github.com/morluto/rea.git
synced 2026-10-26 17:35:11 +00:00
fix(hopper): retain launcher exits observed before startup interruption
Record whether the current owned launcher exit was known when the startup deadline aborted. Reuse the existing launcher failure projection for exit-first ordering while preserving the exact caller cancellation reason when cancellation occurs first. Validation: root focused run passed 38/40 tests including both new real-child ordering controls; two existing cleanup tests timed out and then passed unchanged in a targeted 13/13 rerun. Full check:ci passed. This uses inert Node children, not real Hopper. Keep this audit-branch candidate out of main pending required real-engine qualification. Refs #5429
This commit is contained in:
1 parent
bf79235da3
commit
d81a0f02e5
2 files changed
+177
-1
No files matched your search
@@ -132,6 +132,7 @@ export class HopperClient {
|
||||
#nextId = 1;
|
||||
#closing = false;
|
||||
#launcherExitCode: number | null | undefined;
|
||||
#launcherExitPrecededInterruption = false;
|
||||
#launcherFailureDiagnostic: HopperStartupFailureDiagnostic | undefined;
|
||||
#operationFailure: OperationFailureRecord | undefined;
|
||||
#startupFailureResult: AnalysisError | undefined;
|
||||
@@ -393,12 +394,23 @@ export class HopperClient {
|
||||
this.#options.startupTimeoutMs,
|
||||
signal,
|
||||
);
|
||||
this.#launcherExitPrecededInterruption = false;
|
||||
const recordObservedExitOrder = (): void => {
|
||||
this.#launcherExitPrecededInterruption =
|
||||
this.#resources.launch !== undefined &&
|
||||
this.#launcherExitCode !== undefined;
|
||||
};
|
||||
deadline.signal.addEventListener("abort", recordObservedExitOrder, {
|
||||
once: true,
|
||||
});
|
||||
if (deadline.signal.aborted) recordObservedExitOrder();
|
||||
try {
|
||||
const result = await this.#startWithin(deadline);
|
||||
return result.ok && deadline.signal.aborted
|
||||
? this.#startupFailure(await this.#startupInterruption(deadline))
|
||||
: result;
|
||||
} finally {
|
||||
deadline.signal.removeEventListener("abort", recordObservedExitOrder);
|
||||
deadline.dispose();
|
||||
}
|
||||
}
|
||||
@@ -831,7 +843,20 @@ export class HopperClient {
|
||||
|
||||
async #startupInterruption(
|
||||
deadline: ProviderStartupDeadline,
|
||||
): Promise<HopperCancelledError | HopperTimeoutError | HopperProtocolError> {
|
||||
): Promise<HopperError> {
|
||||
const launch = this.#resources.launch;
|
||||
const exitCode = this.#launcherExitCode;
|
||||
if (
|
||||
this.#launcherExitPrecededInterruption &&
|
||||
launch !== undefined &&
|
||||
exitCode !== undefined &&
|
||||
(launch.providerLifetime === "launcher-process" || exitCode !== 0)
|
||||
)
|
||||
return this.#launcherStartupFailure(
|
||||
launch.providerLifetime === "launcher-process",
|
||||
exitCode,
|
||||
deadline,
|
||||
);
|
||||
if (deadline.signal.reason === STARTUP_OWNER_CLOSED)
|
||||
return new HopperProtocolError("Hopper session closed during startup");
|
||||
if (deadline.interruption === "cancelled")
|
||||
|
||||
@@ -1,8 +1,14 @@
|
||||
import { access } from "node:fs/promises";
|
||||
import { spawn, type ChildProcess } from "node:child_process";
|
||||
|
||||
import { expect, it, vi } from "vitest";
|
||||
|
||||
import { HopperClient } from "../../../../src/hopper/HopperClient.js";
|
||||
import type { BridgeLauncher } from "../../../../src/hopper/BridgeLauncher.js";
|
||||
import {
|
||||
HopperCancelledError,
|
||||
HopperProcessError,
|
||||
} from "../../../../src/domain/hopperErrors.js";
|
||||
import { ok } from "../../../../src/domain/result.js";
|
||||
import { ProviderRunLineage } from "../../../../src/process/ProviderRunLineage.js";
|
||||
import { waitForExit } from "../../../support/process/processFixture.js";
|
||||
@@ -11,6 +17,151 @@ import {
|
||||
trackHopperClient,
|
||||
} from "./hopperClient.fixture.js";
|
||||
|
||||
const ownedChildLauncher = (
|
||||
source: string,
|
||||
onSpawn?: (child: ChildProcess, sessionDirectory: string) => void,
|
||||
): BridgeLauncher => ({
|
||||
async launch(session) {
|
||||
const child = spawn(process.execPath, ["-e", source], {
|
||||
stdio: ["ignore", "pipe", "pipe"],
|
||||
});
|
||||
await new Promise<void>((resolve, reject) => {
|
||||
child.once("spawn", resolve);
|
||||
child.once("error", reject);
|
||||
});
|
||||
onSpawn?.(child, session.directory);
|
||||
return ok({
|
||||
process: child,
|
||||
ownsProcessLifetime: true,
|
||||
providerLifetime: "launcher-process",
|
||||
shutdownMode: "process-cleanup",
|
||||
cleanup: async () => {
|
||||
const signaled = child.exitCode === null && child.signalCode === null;
|
||||
if (signaled) child.kill("SIGTERM");
|
||||
return (await waitForExit(child, 5_000))
|
||||
? { cleaned: true as const, signaled }
|
||||
: {
|
||||
cleaned: false as const,
|
||||
reason: "Fixture child did not exit",
|
||||
};
|
||||
},
|
||||
});
|
||||
},
|
||||
});
|
||||
|
||||
it("keeps an owned launcher exit observed before abort as the startup failure", async () => {
|
||||
const controller = new AbortController();
|
||||
const abortReason = new Error("caller aborted after observed launcher exit");
|
||||
let ownedChild: ChildProcess | undefined;
|
||||
let runtimeDirectory: string | undefined;
|
||||
let observeExit!: () => void;
|
||||
const exitObserved = new Promise<void>((resolve) => {
|
||||
observeExit = resolve;
|
||||
});
|
||||
const client = trackHopperClient(
|
||||
new HopperClient({
|
||||
startupTimeoutMs: 10_000,
|
||||
launcher: ownedChildLauncher("process.exit(3)", (child, directory) => {
|
||||
ownedChild = child;
|
||||
runtimeDirectory = directory;
|
||||
}),
|
||||
onDiagnostic: (event) => {
|
||||
if (event.type !== "launcher-exit") return;
|
||||
observeExit();
|
||||
setImmediate(() => controller.abort(abortReason));
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
try {
|
||||
const started = await client.start(controller.signal);
|
||||
await exitObserved;
|
||||
expect(controller.signal.reason).toBe(abortReason);
|
||||
expect(started).toMatchObject({
|
||||
ok: false,
|
||||
error: {
|
||||
_tag: "HopperProcessError",
|
||||
exitCode: 3,
|
||||
stage: "launch",
|
||||
},
|
||||
});
|
||||
if (started.ok) throw new Error("Expected the owned launcher failure");
|
||||
if (!(started.error instanceof HopperProcessError))
|
||||
throw new Error("Expected HopperProcessError");
|
||||
expect(started.error.cleanupIncomplete).toBe(false);
|
||||
expect(started.error.launcherFailure).toMatchObject({ exit_code: 3 });
|
||||
if (ownedChild === undefined) throw new Error("Missing owned launcher");
|
||||
expect(await waitForExit(ownedChild, 5_000)).toBe(true);
|
||||
expect(ownedChild.exitCode).toBe(3);
|
||||
if (runtimeDirectory === undefined)
|
||||
throw new Error("Missing owned runtime directory");
|
||||
await expect(access(runtimeDirectory)).rejects.toMatchObject({
|
||||
code: "ENOENT",
|
||||
});
|
||||
} finally {
|
||||
await client.close();
|
||||
}
|
||||
|
||||
const control = trackHopperClient(
|
||||
new HopperClient({
|
||||
startupTimeoutMs: 10_000,
|
||||
launcher: ownedChildLauncher("process.exit(3)"),
|
||||
}),
|
||||
);
|
||||
try {
|
||||
const started = await control.start();
|
||||
expect(started).toMatchObject({
|
||||
ok: false,
|
||||
error: { _tag: "HopperProcessError", exitCode: 3, stage: "launch" },
|
||||
});
|
||||
} finally {
|
||||
await control.close();
|
||||
}
|
||||
});
|
||||
|
||||
it("preserves the exact caller reason when cancellation precedes launcher exit", async () => {
|
||||
const controller = new AbortController();
|
||||
const abortReason = new Error("caller cancelled before launcher exit");
|
||||
let launchChild!: () => void;
|
||||
let runtimeDirectory: string | undefined;
|
||||
const childLaunched = new Promise<void>((resolve) => {
|
||||
launchChild = resolve;
|
||||
});
|
||||
const client = trackHopperClient(
|
||||
new HopperClient({
|
||||
startupTimeoutMs: 10_000,
|
||||
launcher: ownedChildLauncher(
|
||||
'process.on("SIGTERM", () => process.exit(0)); setInterval(() => {}, 1000);',
|
||||
(_child, directory) => {
|
||||
runtimeDirectory = directory;
|
||||
launchChild();
|
||||
},
|
||||
),
|
||||
}),
|
||||
);
|
||||
|
||||
try {
|
||||
const starting = client.start(controller.signal);
|
||||
await childLaunched;
|
||||
controller.abort(abortReason);
|
||||
const started = await starting;
|
||||
expect(started).toMatchObject({
|
||||
ok: false,
|
||||
error: { _tag: "HopperCancelledError", cause: abortReason },
|
||||
});
|
||||
if (started.ok) throw new Error("Expected caller cancellation");
|
||||
expect(started.error).toBeInstanceOf(HopperCancelledError);
|
||||
expect(started.error.cause).toBe(abortReason);
|
||||
if (runtimeDirectory === undefined)
|
||||
throw new Error("Missing owned runtime directory");
|
||||
await expect(access(runtimeDirectory)).rejects.toMatchObject({
|
||||
code: "ENOENT",
|
||||
});
|
||||
} finally {
|
||||
await client.close();
|
||||
}
|
||||
});
|
||||
|
||||
it("preserves a warm provider when an already cancelled caller joins", async () => {
|
||||
const launcher = new HopperFixtureLauncher();
|
||||
const client = trackHopperClient(new HopperClient({ launcher }));
|
||||
|
||||
Reference in new issue
Block a user