shifathosn
/
rea
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(hopper): report launcher failures before bridge timeout (#1105) 

* fix(hopper): report launcher failures before bridge timeout

* fix(hopper): separate helper ownership from GUI lifetime
This commit is contained in:
Ysopwnable 2026-10-09 01:51:45 +08:00 committed by GitHub
parent 7914e36ab9
commit cd756b07d5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 270 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/domain/analysisErrorPresentation.ts
View File

@ -209,6 +209,8 @@ const RESTART_OWNED_PROVIDER =
const hopperProcessRemediation = (error: HopperProcessError): string => {
if (error.failureCode !== undefined && error.userMessage !== undefined)
return error.userMessage;
if (error.stage === "launch")
return "Review the captured launcher diagnostics and Hopper setup, then retry opening the target.";
if (providerRetryAction(error.providerState) === "restart_provider")
return RESTART_OWNED_PROVIDER;
return "Read binary_session.provider_operation_health for this request before retrying or restarting the owned provider.";
@ -229,7 +231,9 @@ const hopperProcessMessage = (error: HopperProcessError): string => {
? "no exit code was observed"
: `exit code ${String(error.exitCode)}`;
const recovery =
error.failureCode === undefined ? ` ${RESTART_OWNED_PROVIDER}` : "";
error.failureCode === undefined && stage !== "launch"
? ` ${RESTART_OWNED_PROVIDER}`
: "";
return `Hopper exited during ${where} (${exit}).${startup}${recovery}${request}`;
}
if (error.providerState === "unreachable")

8
src/domain/analysisErrorProjection.ts
View File

@ -352,7 +352,7 @@ const providerErrorDetails = (
provider_state: error.providerState,
retry_action: providerRetryAction(
error.providerState,
error.failureCode !== undefined,
error.failureCode !== undefined || error.stage === "launch",
),
...(error.failureCode === undefined
? {}
@ -362,6 +362,9 @@ const providerErrorDetails = (
...(error.diagnostic === undefined
? {}
: { diagnostics: { ...error.diagnostic } }),
...(error.launcherFailure === undefined
? {}
: { launcher: error.launcherFailure }),
};
}
if (error instanceof HopperStartError)
@ -372,6 +375,9 @@ const providerErrorDetails = (
...(error.ownerRunId === undefined
? {}
: { owner_run_id: error.ownerRunId }),
...(error.launcherFailure === undefined
? {}
: { launcher: error.launcherFailure }),
};
return undefined;
};

14
src/domain/hopperErrors.ts
View File

@ -13,6 +13,16 @@ import {
/** Base class for failures produced specifically by the Hopper provider. */
export abstract class HopperError extends AnalysisError {}
/** Observed launcher outcome; a helper exit does not establish GUI lifetime. */
export type HopperLauncherOutcome = {
readonly exit_code: number | null;
readonly signal: string | null;
readonly stdout: { readonly text: string; readonly bytes: number };
readonly stderr: { readonly text: string; readonly bytes: number };
readonly output_closed: boolean;
readonly diagnostic_truncated: boolean;
};
/** Hopper did not respond within the configured operation deadline. */
export class HopperTimeoutError extends HopperError {
readonly _tag = "HopperTimeoutError";
@ -93,6 +103,7 @@ export class HopperProcessError extends HopperError {
readonly requestId?: number,
providerState?: HopperProcessProviderState,
stage?: ProviderFailureStage,
readonly launcherFailure?: HopperLauncherOutcome,
) {
super(`Hopper bridge stopped unexpectedly with code ${String(exitCode)}`);
const failure = hopperStartupFailure(exitCode);
@ -115,15 +126,18 @@ export class HopperStartError extends HopperError {
readonly _tag = "HopperStartError";
override readonly userMessage: string | undefined;
readonly ownerRunId: string | undefined;
readonly launcherFailure: HopperLauncherOutcome | undefined;
constructor(
options?: ErrorOptions & {
readonly userMessage?: string;
readonly ownerRunId?: string;
readonly launcherFailure?: HopperLauncherOutcome;
},
) {
super("Hopper application bridge could not be started", options);
this.userMessage = options?.userMessage;
this.ownerRunId = options?.ownerRunId;
this.launcherFailure = options?.launcherFailure;
}
}

27
src/hopper/BridgeLauncher.ts
View File

@ -39,9 +39,14 @@ export interface BridgeSession {
/** Process handle returned by a bridge launcher. */
export type BridgeLaunch =
| (ProviderProcessLaunch & { readonly shutdownMode: "bridge-request" })
| (ProviderProcessLaunch & {
readonly shutdownMode: "bridge-request";
/** The owned helper may hand off to an external GUI application. */
readonly providerLifetime: "launcher-process" | "external-application";
})
| (Extract<ProviderProcessLaunch, { readonly ownsProcessLifetime: true }> & {
readonly shutdownMode: "process-cleanup";
readonly providerLifetime: "launcher-process";
readonly cleanup: NonNullable<ProviderProcessLaunch["cleanup"]>;
});
@ -211,15 +216,23 @@ export class HopperApplicationLauncher implements BridgeLauncher {
await cleanupOwnedProcessGroup(started.ownership);
return err(new HopperStartError({ cause }));
}
return ok({
const ownedLauncher = {
process: started.process,
ownsProcessLifetime: true,
ownsProcessLifetime: true as const,
ownership: started.ownership,
shutdownMode: ownsProcessLifetime
? "process-cleanup"
: "bridge-request",
cleanup,
});
};
return ownsProcessLifetime
? ok({
...ownedLauncher,
providerLifetime: "launcher-process",
shutdownMode: "process-cleanup",
})
: ok({
...ownedLauncher,
providerLifetime: "external-application",
shutdownMode: "bridge-request",
});
} catch (cause: unknown) {
return err(hopperLaunchFailure(cause, signal));
}

1
src/hopper/HopperCleanup.test.ts
View File

@ -30,6 +30,7 @@ const processCleanupLaunch = (): BridgeLaunch => ({
kill: () => true,
}),
ownsProcessLifetime: true,
providerLifetime: "launcher-process",
shutdownMode: "process-cleanup",
cleanup: () =>
Promise.resolve({

83
src/hopper/HopperClient.ts
View File

@ -8,6 +8,7 @@ import { AnalysisCapabilityUnavailableError } from "../domain/analysisErrorCore.
import {
HopperCancelledError,
type HopperError,
type HopperLauncherOutcome,
HopperProcessError,
HopperProtocolError,
HopperRemoteError,
@ -76,7 +77,6 @@ type HopperClientCloseOptions = {
readonly retainDocument?: boolean;
};
/** Safe launcher telemetry; stderr content is intentionally never exposed. */
/**
* Owns one authenticated NDJSON-over-Unix-socket bridge session.
*
@ -122,9 +122,12 @@ export class HopperClient {
readonly #onSocketClose = (): void => {
if (this.#closing) return;
const launch = this.#launch;
if (launch?.ownsProcessLifetime && this.#launcherExitCode !== undefined)
if (
launch?.providerLifetime === "launcher-process" &&
this.#launcherExitCode !== undefined
)
return;
if (launch?.ownsProcessLifetime) {
if (launch?.providerLifetime === "launcher-process") {
setImmediate(() => {
if (this.#closing || this.#launcherExitCode !== undefined) return;
const exitCode = launch.process.exitCode ?? null;
@ -503,17 +506,19 @@ export class HopperClient {
if (this.#closing) return err(new HopperProcessError(null));
if (
this.#launcherExitCode !== undefined &&
hopperStartupFailure(this.#launcherExitCode) !== undefined
)
return err(
new HopperProcessError(
this.#launcherExitCode,
this.#launcherFailureDiagnostic,
undefined,
undefined,
"exited",
),
this.#launch !== undefined &&
(this.#launch.providerLifetime === "launcher-process" ||
this.#launcherExitCode !== 0)
) {
const failure = await this.#launcherStartupFailure(
this.#launch.providerLifetime === "launcher-process",
this.#launcherExitCode,
deadline,
);
return deadline.interruption === "cancelled"
? err(startupInterruption(deadline))
: err(failure);
}
try {
await access(socketPath);
} catch (cause: unknown) {
@ -538,6 +543,52 @@ export class HopperClient {
return err(new HopperTimeoutError(this.#options.startupTimeoutMs));
}
async #launcherStartupFailure(
ownsProviderLifetime: boolean,
exitCode: number | null,
deadline: ProviderStartupDeadline,
): Promise<HopperError> {
const process = this.#process;
const token = this.#token;
// Descendants may inherit the helper's pipes. Bound drainage independently
// from readiness, and report whether producer output actually closed.
const outputClosed =
(await process?.waitForOutputClose(
Math.min(1_000, deadline.remainingMs()),
)) ?? false;
const snapshot = process?.snapshot();
const redact = (text: string): string =>
token === undefined
? text
: text.replaceAll(token, "[redacted transport credential]");
const launcherFailure: HopperLauncherOutcome | undefined =
snapshot === undefined
? undefined
: {
exit_code: exitCode,
signal: snapshot.signal ?? null,
stdout: { ...snapshot.stdout, text: redact(snapshot.stdout.text) },
stderr: { ...snapshot.stderr, text: redact(snapshot.stderr.text) },
output_closed: outputClosed,
diagnostic_truncated: snapshot.diagnosticTruncated === true,
};
if (ownsProviderLifetime)
return new HopperProcessError(
exitCode,
this.#launcherFailureDiagnostic,
undefined,
undefined,
"exited",
"launch",
launcherFailure,
);
return new HopperStartError({
userMessage:
"Hopper's launcher helper failed before bridge readiness. Review details.launcher and complete Hopper setup before retrying.",
...(launcherFailure === undefined ? {} : { launcherFailure }),
});
}
async #request(
method: string,
params: JsonValue,
@ -654,7 +705,7 @@ export class HopperClient {
type: "launcher-exit",
code: event.code,
});
if (launch.ownsProcessLifetime && !this.#closing) {
if (launch.providerLifetime === "launcher-process" && !this.#closing) {
this.#rememberExit(event.code);
this.#failAll(
new HopperProcessError(
@ -726,7 +777,9 @@ export class HopperClient {
const previous = this.#operationFailure;
const state = processFailureState(error);
const startupFailure =
error.failureCode !== undefined || previous?.startupFailure === true;
error.stage === "launch" ||
error.failureCode !== undefined ||
previous?.startupFailure === true;
const merged = mergeFailedRequests(previous?.requests ?? [], requests);
if (
previous !== undefined &&

3
tests/boundary/providers/hopper/hopperClient.cleanup.test.ts
View File

@ -45,6 +45,7 @@ class OwnedFixtureLauncher implements BridgeLauncher {
return ok({
process: started.process,
ownsProcessLifetime: true as const,
providerLifetime: "launcher-process" as const,
ownership: started.ownership,
shutdownMode: "process-cleanup" as const,
cleanup: () => cleanupOwnedProcessGroup(started.ownership),
@ -68,6 +69,7 @@ class UnconfirmedFixtureLauncher implements BridgeLauncher {
{ stdio: ["ignore", "ignore", "pipe"] },
),
ownsProcessLifetime: false,
providerLifetime: "external-application" as const,
shutdownMode: "bridge-request" as const,
}),
);
@ -90,6 +92,7 @@ class UnconfirmedOwnedFixtureLauncher implements BridgeLauncher {
{ stdio: ["ignore", "ignore", "pipe"] },
),
ownsProcessLifetime: true as const,
providerLifetime: "launcher-process" as const,
shutdownMode: "bridge-request" as const,
}),
);

1
tests/boundary/providers/hopper/hopperClient.fixture.ts
View File

@ -55,6 +55,7 @@ export class HopperFixtureLauncher implements BridgeLauncher {
ok({
process: child,
ownsProcessLifetime: true as const,
providerLifetime: "launcher-process" as const,
shutdownMode: "bridge-request" as const,
}),
);

5
tests/boundary/providers/hopper/hopperClient.health.test.ts
View File

@ -40,7 +40,10 @@ class LifecycleLauncher implements BridgeLauncher {
await this.fixture.launch(session);
return ok({
process: this.observed,
ownsProcessLifetime: this.owned,
ownsProcessLifetime: true as const,
providerLifetime: this.owned
? ("launcher-process" as const)
: ("external-application" as const),
shutdownMode: "bridge-request" as const,
});
}

136
tests/boundary/providers/hopper/hopperClient.helperFailure.test.ts Normal file
View File

@ -0,0 +1,136 @@
import { spawn } from "node:child_process";
import { expect, it, onTestFinished } from "vitest";
import { projectAnalysisError } from "../../../../src/domain/analysisErrorProjection.js";
import { ok } from "../../../../src/domain/result.js";
import type { BridgeLauncher } from "../../../../src/hopper/BridgeLauncher.js";
import { HopperApplicationLauncher } from "../../../../src/hopper/BridgeLauncher.js";
import { HopperClient } from "../../../../src/hopper/HopperClient.js";
it("does not equate an owned native helper exit with an external GUI exit", async () => {
const launcher = new HopperApplicationLauncher({
launcherPath: process.execPath,
targetPath: "/target",
targetKind: "executable",
loaderArgs: [
"-e",
'process.stderr.write("Hopper has not been found.\\n", () => process.exit(3))',
"--",
],
bridgeScriptPath: "/rea/hopper_bridge.py",
launchMode: "native",
});
const client = new HopperClient({ launcher, startupTimeoutMs: 10_000 });
onTestFinished(() => client.close());
const result = await client.start();
expect(result).toMatchObject({
ok: false,
error: { _tag: "HopperStartError" },
});
if (result.ok) throw new Error("Expected the native helper to fail");
expect(projectAnalysisError(result.error)).toMatchObject({
details: {
provider_state: "unknown",
launcher: { exit_code: 3 },
},
});
expect(client.operationHealth()).toMatchObject({
state: "unknown",
exitCode: null,
});
});
it.each(["exit", "signal", "exit75"] as const)(
"reports a failed non-owning launcher %s without waiting for bridge timeout or inventing a GUI exit",
async (termination) => {
let token = "";
const launcher: BridgeLauncher = {
launch(session) {
token = session.token;
const stdout = `API_KEY=local-evidence-kept ${token}\n`;
const stderr = `Hopper has not been found. Spotlight is unavailable. ${token}\n`;
const stop =
termination === "signal"
? 'process.kill(process.pid, "SIGTERM")'
: `process.exit(${termination === "exit75" ? 75 : 1})`;
return Promise.resolve(
ok({
process: spawn(
process.execPath,
[
"-e",
`process.stdout.write(${JSON.stringify(stdout)}, () => process.stderr.write(${JSON.stringify(stderr)}, () => ${stop}));`,
],
{ stdio: ["ignore", "pipe", "pipe"] },
),
ownsProcessLifetime: false as const,
providerLifetime: "external-application" as const,
shutdownMode: "bridge-request" as const,
}),
);
},
};
const client = new HopperClient({ launcher, startupTimeoutMs: 10_000 });
onTestFinished(() => client.close());
const result = await client.start();
expect(result).toMatchObject({
ok: false,
error: { _tag: "HopperStartError" },
});
if (result.ok) throw new Error("Expected the launcher to fail");
const projected = projectAnalysisError(result.error);
expect(projected).toMatchObject({
code: "provider_unavailable",
details: {
stage: "launch",
launcher: {
exit_code:
termination === "signal" ? null : termination === "exit75" ? 75 : 1,
signal: termination === "signal" ? "SIGTERM" : null,
stdout: {
text: expect.stringContaining("API_KEY=local-evidence-kept"),
},
stderr: { text: expect.stringContaining("Spotlight is unavailable") },
},
},
});
expect(JSON.stringify(projected).includes(token)).toBe(false);
expect(client.operationHealth()).toMatchObject({
state: "unknown",
stage: "launch",
exitCode: null,
});
},
);
it.each([0, 1])(
"reports owned process exit %s before bridge readiness as an observed exit",
async (code) => {
const launcher: BridgeLauncher = {
launch() {
return Promise.resolve(
ok({
process: spawn(process.execPath, ["-e", `process.exit(${code})`], {
stdio: ["ignore", "pipe", "pipe"],
}),
ownsProcessLifetime: true as const,
providerLifetime: "launcher-process" as const,
shutdownMode: "bridge-request" as const,
}),
);
},
};
const client = new HopperClient({ launcher, startupTimeoutMs: 10_000 });
onTestFinished(() => client.close());
const result = await client.start();
expect(result).toMatchObject({
ok: false,
error: { _tag: "HopperProcessError", exitCode: code, stage: "launch" },
});
expect(client.operationHealth()).toMatchObject({
state: "exited",
exitCode: code,
});
},
);

14
tests/boundary/providers/hopper/hopperClient.startupFailures.test.ts
View File

@ -27,6 +27,7 @@ class SilentLauncher implements BridgeLauncher {
},
),
ownsProcessLifetime: true as const,
providerLifetime: "launcher-process" as const,
shutdownMode: "bridge-request" as const,
}),
);
@ -52,7 +53,10 @@ class CancelThenFixtureLauncher implements BridgeLauncher {
}
class ExitingLauncher implements BridgeLauncher {
constructor(readonly code: number) {}
constructor(
readonly code: number,
readonly ownsProcessLifetime = true,
) {}
launch() {
return Promise.resolve(
@ -60,7 +64,10 @@ class ExitingLauncher implements BridgeLauncher {
process: spawn(process.execPath, ["-e", `process.exit(${this.code})`], {
stdio: ["ignore", "ignore", "pipe"],
}),
ownsProcessLifetime: true as const,
ownsProcessLifetime: this.ownsProcessLifetime,
providerLifetime: this.ownsProcessLifetime
? ("launcher-process" as const)
: ("external-application" as const),
shutdownMode: "bridge-request" as const,
}),
);
@ -97,6 +104,7 @@ class DiagnosticExitingLauncher implements BridgeLauncher {
{ stdio: ["ignore", "ignore", "pipe"] },
),
ownsProcessLifetime: true as const,
providerLifetime: "launcher-process" as const,
shutdownMode: "bridge-request" as const,
}),
);
@ -281,7 +289,7 @@ describe("HopperClient startup failures", () => {
it("allows a short-lived launcher to hand off bridge startup", async () => {
const client = new HopperClient({
launcher: new ExitingLauncher(0),
launcher: new ExitingLauncher(0, false),
startupTimeoutMs: 100,
});
clients.push(client);