fix(browser): reject startup when control ports are occupied (#109994)

Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
wahaha1223
2026-07-28 18:01:02 +08:00
committed by GitHub
parent 9c051c6513
commit 4c2d06be2b
5 changed files with 80 additions and 9 deletions
@@ -1,4 +1,5 @@
// Browser tests cover bridge server.auth plugin behavior.
import { createServer } from "node:http";
import { afterEach, describe, expect, it, vi } from "vitest";
import { getBridgeAuthForPort } from "./bridge-auth-registry.js";
import { startBrowserBridgeServer, stopBrowserBridgeServer } from "./bridge-server.js";
@@ -96,6 +97,33 @@ describe("startBrowserBridgeServer auth", () => {
).rejects.toThrow(/requires auth/i);
});
it("rejects startup when the bridge port is already in use", async () => {
const blocker = createServer();
await new Promise<void>((resolve) => {
blocker.listen(0, "127.0.0.1", resolve);
});
const address = blocker.address();
if (!address || typeof address === "string") {
throw new Error("expected blocker TCP address");
}
try {
await expect(
startBrowserBridgeServer({
resolved: buildResolvedConfig(),
authToken: "secret-token",
host: "127.0.0.1",
port: address.port,
skipRouteRegistrationForTest: true,
}),
).rejects.toMatchObject({ code: "EADDRINUSE" });
} finally {
await new Promise<void>((resolve) => {
blocker.close(() => resolve());
});
}
});
it("closes ingress but retains exact bridge cleanup state for retry", async () => {
const bridge = await startBrowserBridgeServer({
resolved: buildResolvedConfig(),
@@ -11,6 +11,7 @@ import { normalizeOptionalString } from "openclaw/plugin-sdk/string-coerce-runti
import { isLoopbackHost } from "../gateway/net.js";
import { deleteBridgeAuthForPort, setBridgeAuthForPort } from "./bridge-auth-registry.js";
import type { ResolvedBrowserConfig } from "./config.js";
import { listenBrowserHttpServer } from "./http-listen.js";
import type { BrowserRouteRegistrar } from "./routes/types.js";
import { stopBrowserBridgeRuntime } from "./runtime-lifecycle.js";
import type { BrowserServerState, ProfileContext } from "./server-context.js";
@@ -154,10 +155,7 @@ export async function startBrowserBridgeServer(params: {
registerBrowserRoutes(app as unknown as BrowserRouteRegistrar, ctx);
}
const server = await new Promise<Server>((resolve, reject) => {
const s = app.listen(port, host, () => resolve(s));
s.once("error", reject);
});
const server = await listenBrowserHttpServer(app, port, host);
const address = server.address() as AddressInfo | null;
const resolvedPort = address?.port ?? port;
@@ -0,0 +1,24 @@
import { createServer, type RequestListener, type Server } from "node:http";
export function listenBrowserHttpServer(
app: RequestListener,
port: number,
host: string,
): Promise<Server> {
return new Promise((resolve, reject) => {
const server = createServer(app);
const onError = (error: Error) => {
server.off("listening", onListening);
reject(error);
};
const onListening = () => {
server.off("error", onError);
resolve(server);
};
// Install both terminal listeners before listen so Express cannot settle
// its callback with a bind error before this startup Promise rejects.
server.once("error", onError);
server.once("listening", onListening);
server.listen(port, host);
});
}
@@ -1,4 +1,5 @@
// Browser tests cover server.auth fail closed plugin behavior.
import { createServer } from "node:http";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { startBrowserControlServerFromConfig, stopBrowserControlServer } from "../server.js";
import { getFreePort } from "./test-port.js";
@@ -127,4 +128,27 @@ describe("browser control auth bootstrap failures", () => {
expect(started).toBeNull();
});
it("returns null when the browser control port is already in use", async () => {
const blocker = createServer();
await new Promise<void>((resolve) => {
blocker.listen(0, "127.0.0.1", resolve);
});
const address = blocker.address();
if (!address || typeof address === "string") {
throw new Error("expected blocker TCP address");
}
mocks.controlPort = address.port;
mocks.ensureBrowserControlAuth.mockResolvedValueOnce({ auth: { token: "test-token" } });
mocks.resolveBrowserControlAuth.mockReturnValueOnce({ token: "test-token" });
mocks.shouldAutoGenerateBrowserAuth.mockReturnValueOnce(false);
try {
await expect(startBrowserControlServerFromConfig()).resolves.toBeNull();
} finally {
await new Promise<void>((resolve) => {
blocker.close(() => resolve());
});
}
});
});
+2 -5
View File
@@ -1,7 +1,6 @@
/**
* Browser control HTTP server startup and shutdown entrypoints.
*/
import type { Server } from "node:http";
import express from "express";
import {
createBrowserControlContext,
@@ -18,6 +17,7 @@ import {
resolveBrowserControlAuth,
shouldAutoGenerateBrowserAuth,
} from "./browser/control-auth.js";
import { listenBrowserHttpServer } from "./browser/http-listen.js";
import { registerBrowserRoutes } from "./browser/routes/index.js";
import type { BrowserRouteRegistrar } from "./browser/routes/types.js";
import type { BrowserServerState } from "./browser/server-context.js";
@@ -85,10 +85,7 @@ async function startBrowserControlServerUnlocked(): Promise<BrowserServerState |
registerBrowserRoutes(app as unknown as BrowserRouteRegistrar, ctx);
const port = resolved.controlPort;
const server = await new Promise<Server>((resolve, reject) => {
const s = app.listen(port, "127.0.0.1", () => resolve(s));
s.once("error", reject);
}).catch((err: unknown) => {
const server = await listenBrowserHttpServer(app, port, "127.0.0.1").catch((err: unknown) => {
logServer.error(`openclaw browser server failed to bind 127.0.0.1:${port}: ${String(err)}`);
return null;
});