From 935826563d2226cca8e13321ac3e6a0784a494d1 Mon Sep 17 00:00:00 2001 From: Philipp Winterle Date: Mon, 21 Sep 2026 15:55:25 +0200 Subject: [PATCH 1/2] Reject admin server start when the port can't be bound Express 5 passes listen errors to the listen callback, so a failed bind (e.g. EADDRINUSE) resolved start() as if the server were running. Later stop() then failed with 'Server is not running', and every client got ECONNREFUSED. Now start() rejects with the bind error and leaves the admin server stopped, so it can be stopped or started again. --- src/admin/admin-server.ts | 18 ++++++++++-- test/integration/remote-client.spec.ts | 38 ++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/src/admin/admin-server.ts b/src/admin/admin-server.ts index bd694c44a..5590159fd 100644 --- a/src/admin/admin-server.ts +++ b/src/admin/admin-server.ts @@ -310,8 +310,22 @@ export class AdminServer ? { port: listenOptions, host: '127.0.0.1' } : { host: '127.0.0.1', ...listenOptions }; - await new Promise((resolve, reject) => { - this.server = makeDestroyable(this.app.listen(resolvedListenOptions, () => resolve())); + try { + await this.listen(resolvedListenOptions); + } catch (e) { + // Failed to bind (e.g. EADDRINUSE), so there's no running server to track: + this.server = null; + throw e; + } + } + + private listen(listenOptions: { port: number, host: string }) { + return new Promise((resolve, reject) => { + // Express 5 passes listen errors to this callback too, so we must reject here, + // or a failed bind would look like a successful start: + this.server = makeDestroyable(this.app.listen(listenOptions, (error?: Error) => + error ? reject(error) : resolve() + )); this.server.on('error', reject); diff --git a/test/integration/remote-client.spec.ts b/test/integration/remote-client.spec.ts index 8dd11e5ab..3e54fa312 100644 --- a/test/integration/remote-client.spec.ts +++ b/test/integration/remote-client.spec.ts @@ -1004,6 +1004,44 @@ nodeOnly(() => { }); }); + describe("when the admin server port is already taken", () => { + let blockingServer: net.Server; + let port: number; + + beforeEach(async () => { + port = await getPort(); + blockingServer = net.createServer(); + await new Promise((resolve) => + blockingServer.listen({ port, host: '127.0.0.1' }, resolve) + ); + }); + + afterEach(() => new Promise((resolve) => blockingServer.close(() => resolve()))); + + it("fails to start", async () => { + const adminServer = getAdminServer(); + + await expect(adminServer.start(port)) + .to.eventually.be.rejectedWith(/EADDRINUSE/); + }); + + it("can still be stopped after failing to start", async () => { + const adminServer = getAdminServer(); + await adminServer.start(port).catch(() => {}); + + await expect(adminServer.stop()).to.eventually.be.fulfilled; + }); + + it("can be started on a free port after failing to start", async () => { + const adminServer = getAdminServer(); + await adminServer.start(port).catch(() => {}); + + const freePort = await getPort(); + await adminServer.start(freePort); + await adminServer.stop(); + }); + }); + describe("with message body decoding disabled", () => { const server = getAdminServer(); From b8f25dc83093490e4f23165dc74ab18e16dc04d9 Mon Sep 17 00:00:00 2001 From: Philipp Winterle Date: Mon, 21 Sep 2026 16:00:36 +0200 Subject: [PATCH 2/2] Run test admin servers on a port outside the ephemeral range The node tests started every admin server on the default port 45454, which is inside the default Linux ephemeral port range (32768-60999). An outgoing connection from another test can take that port, so the admin server fails to bind and every remote-client test after it fails with ECONNREFUSED. Tests now use port 30454, which isn't in the default ephemeral range on Linux, macOS or Windows. --- test/integration/plugins.spec.ts | 27 ++++++---- .../proxying/upstream-proxying.spec.ts | 8 +-- test/integration/remote-client.spec.ts | 53 +++++++++++-------- .../raw-passthrough-events.spec.ts | 7 ++- .../subscriptions/request-events.spec.ts | 12 +++-- test/test-utils.ts | 7 +++ 6 files changed, 71 insertions(+), 43 deletions(-) diff --git a/test/integration/plugins.spec.ts b/test/integration/plugins.spec.ts index 5dcb8c746..353e24ab3 100644 --- a/test/integration/plugins.spec.ts +++ b/test/integration/plugins.spec.ts @@ -1,6 +1,11 @@ import gql from "graphql-tag"; import { PluggableAdmin, MockttpPluggableAdmin } from "../.."; -import { expect, nodeOnly } from "../test-utils"; +import { + expect, + nodeOnly, + TEST_ADMIN_SERVER_PORT, + TEST_ADMIN_SERVER_URL +} from "../test-utils"; nodeOnly(() => { describe("Admin server plugins", function () { @@ -24,9 +29,9 @@ nodeOnly(() => { } } }); - await adminServer.start(); + await adminServer.start(TEST_ADMIN_SERVER_PORT); - adminClient = new PluggableAdmin.AdminClient(); + adminClient = new PluggableAdmin.AdminClient({ adminServerUrl: TEST_ADMIN_SERVER_URL }); await adminClient.start({ myPlugin: {} }); @@ -55,11 +60,11 @@ nodeOnly(() => { } } }); - await adminServer.start(); + await adminServer.start(TEST_ADMIN_SERVER_PORT); let client = adminClient = new PluggableAdmin.AdminClient<{ myPlugin: PluggableAdmin.AdminPlugin<{}, { aMetadataField: boolean }> - }>(); + }>({ adminServerUrl: TEST_ADMIN_SERVER_URL }); const startResult = await client.start({ myPlugin: {} }); @@ -83,9 +88,9 @@ nodeOnly(() => { } } }); - await adminServer.start(); + await adminServer.start(TEST_ADMIN_SERVER_PORT); - adminClient = new PluggableAdmin.AdminClient(); + adminClient = new PluggableAdmin.AdminClient({ adminServerUrl: TEST_ADMIN_SERVER_URL }); await adminClient.start({ myPlugin: {} }); @@ -118,9 +123,9 @@ nodeOnly(() => { http: MockttpPluggableAdmin.MockttpAdminPlugin } }); - await adminServer.start(); + await adminServer.start(TEST_ADMIN_SERVER_PORT); - const client = adminClient = new PluggableAdmin.AdminClient(); + const client = adminClient = new PluggableAdmin.AdminClient({ adminServerUrl: TEST_ADMIN_SERVER_URL }); await adminClient.start({ myPlugin: {}, http: {} @@ -169,9 +174,9 @@ nodeOnly(() => { } } }); - await adminServer.start(); + await adminServer.start(TEST_ADMIN_SERVER_PORT); - adminClient = new PluggableAdmin.AdminClient(); + adminClient = new PluggableAdmin.AdminClient({ adminServerUrl: TEST_ADMIN_SERVER_URL }); await adminClient.start({ myPlugin: {} }); diff --git a/test/integration/proxying/upstream-proxying.spec.ts b/test/integration/proxying/upstream-proxying.spec.ts index 77abaf813..2718ebf0b 100644 --- a/test/integration/proxying/upstream-proxying.spec.ts +++ b/test/integration/proxying/upstream-proxying.spec.ts @@ -6,7 +6,9 @@ import * as url from 'url'; import { getLocal, Mockttp, MockedEndpoint, getAdminServer, getRemote } from "../../.."; import { expect, - nodeOnly + nodeOnly, + TEST_ADMIN_SERVER_PORT, + TEST_ADMIN_SERVER_URL } from "../../test-utils"; const INITIAL_ENV = _.cloneDeep(process.env); @@ -359,11 +361,11 @@ nodeOnly(() => { const adminServer = getAdminServer(); - before(() => adminServer.start()); + before(() => adminServer.start(TEST_ADMIN_SERVER_PORT)); after(() => adminServer.stop()); beforeEach(async () => { - server = getRemote(); + server = getRemote({ adminServerUrl: TEST_ADMIN_SERVER_URL }); await server.start(); // Configure Request to use the *first* server as a proxy diff --git a/test/integration/remote-client.spec.ts b/test/integration/remote-client.spec.ts index 3e54fa312..e155ef5f6 100644 --- a/test/integration/remote-client.spec.ts +++ b/test/integration/remote-client.spec.ts @@ -25,7 +25,9 @@ import { browserOnly, delay, getDeferred, - defaultNodeConnectionHeader + defaultNodeConnectionHeader, + TEST_ADMIN_SERVER_PORT, + TEST_ADMIN_SERVER_URL } from "../test-utils"; import type { MockttpClient } from "../../dist/client/mockttp-client"; @@ -55,9 +57,9 @@ nodeOnly(() => { describe("with no configuration", () => { const server = getAdminServer(); - const remoteServer = getRemote(); + const remoteServer = getRemote({ adminServerUrl: TEST_ADMIN_SERVER_URL }); - before(() => server.start()); + before(() => server.start(TEST_ADMIN_SERVER_PORT)); after(() => server.stop()); beforeEach(() => remoteServer.start()); @@ -560,7 +562,7 @@ nodeOnly(() => { it("should support explicitly resetting all servers", async () => { await remoteServer.forGet("/mocked-endpoint").thenReply(200, "mocked data"); - await resetAdminServer(); + await resetAdminServer({ adminServerUrl: TEST_ADMIN_SERVER_URL }); const result = await request.get(remoteServer.urlFor("/mocked-endpoint")).catch((e) => e); @@ -571,7 +573,7 @@ nodeOnly(() => { it("should reject multiple clients trying to control the same port", async () => { const port = remoteServer.port!; - await expect(getRemote().start(port)) + await expect(getRemote({ adminServerUrl: TEST_ADMIN_SERVER_URL }).start(port)) .to.eventually.be.rejectedWith(`Failed to start mock session: listen EADDRINUSE`); }); @@ -590,7 +592,7 @@ nodeOnly(() => { }); it("should reject Mockttp clients trying to use that port", async () => { - await expect(getRemote().start(port)) + await expect(getRemote({ adminServerUrl: TEST_ADMIN_SERVER_URL }).start(port)) .to.eventually.be.rejectedWith(/Failed to start mock session: listen EADDRINUSE/); }); }); @@ -619,9 +621,9 @@ nodeOnly(() => { } } }); - let client = getRemote(); + let client = getRemote({ adminServerUrl: TEST_ADMIN_SERVER_URL }); - before(() => server.start()); + before(() => server.start(TEST_ADMIN_SERVER_PORT)); after(() => server.stop()); beforeEach(() => client.start()); @@ -651,9 +653,9 @@ nodeOnly(() => { } }); - let client = getRemote(); + let client = getRemote({ adminServerUrl: TEST_ADMIN_SERVER_URL }); - before(() => server.start()); + before(() => server.start(TEST_ADMIN_SERVER_PORT)); after(() => server.stop()); beforeEach(() => client.start()); @@ -724,9 +726,9 @@ nodeOnly(() => { let adminServer = getAdminServer({ webSocketKeepAlive: 50 }); - let client = getRemote(); + let client = getRemote({ adminServerUrl: TEST_ADMIN_SERVER_URL }); - before(() => adminServer.start()); + before(() => adminServer.start(TEST_ADMIN_SERVER_PORT)); after(() => adminServer.stop()); beforeEach(() => client.start()); @@ -770,19 +772,20 @@ nodeOnly(() => { let client: Mockttp; - before(() => server.start()); + before(() => server.start(TEST_ADMIN_SERVER_PORT)); after(() => server.stop()); afterEach(() => client.stop()); it("rejects clients with no origin", async () => { - client = getRemote(); + client = getRemote({ adminServerUrl: TEST_ADMIN_SERVER_URL }); await expect(client.start()).to.be.rejectedWith('403'); }); it("rejects clients with the wrong origin", async () => { client = getRemote({ + adminServerUrl: TEST_ADMIN_SERVER_URL, client: { headers: { origin: 'https://twitter.com' @@ -795,6 +798,7 @@ nodeOnly(() => { it("rejects clients with the wrong origin protocol", async () => { client = getRemote({ + adminServerUrl: TEST_ADMIN_SERVER_URL, client: { headers: { origin: 'http://example.com' @@ -807,6 +811,7 @@ nodeOnly(() => { it("allows clients that specify the correct origin", async () => { client = getRemote({ + adminServerUrl: TEST_ADMIN_SERVER_URL, client: { headers: { origin: 'https://example.com' @@ -819,6 +824,7 @@ nodeOnly(() => { it("rejects subscriptions for clients that specify no origin", async () => { client = getRemote({ + adminServerUrl: TEST_ADMIN_SERVER_URL, client: { headers: { origin: 'https://example.com' @@ -830,7 +836,7 @@ nodeOnly(() => { // Manually send a subscription socket with no Origin (can't start an invalid client // and test, because the client fails to start given a bad origin) - const ws = new WebSocket(`ws://localhost:45454/session/${client.port}/subscription`); + const ws = new WebSocket(`ws://localhost:${TEST_ADMIN_SERVER_PORT}/session/${client.port}/subscription`); await expect(new Promise((resolve, reject) => { ws.addEventListener('open', resolve); @@ -840,6 +846,7 @@ nodeOnly(() => { it("rejects subscriptions for clients that specify the wrong origin", async () => { client = getRemote({ + adminServerUrl: TEST_ADMIN_SERVER_URL, client: { headers: { origin: 'https://example.com' @@ -851,7 +858,7 @@ nodeOnly(() => { // Manually send a subscription socket with the wrong Origin (can't start an invalid client // and test, because the client fails to start given a bad origin) - const ws = new WebSocket(`ws://localhost:45454/session/${client.port}/subscription`, { + const ws = new WebSocket(`ws://localhost:${TEST_ADMIN_SERVER_PORT}/session/${client.port}/subscription`, { headers: { origin: 'https://twitter.com' } @@ -865,6 +872,7 @@ nodeOnly(() => { it("allows subscriptions for clients that specify the correct origin", async () => { client = getRemote({ + adminServerUrl: TEST_ADMIN_SERVER_URL, client: { headers: { origin: 'https://example.com' @@ -876,7 +884,7 @@ nodeOnly(() => { // Manually send a subscription socket with the right Origin for consistency with above const id = getClientSessionId(client); - const ws = new WebSocket(`ws://localhost:45454/session/${id}/subscription`, { + const ws = new WebSocket(`ws://localhost:${TEST_ADMIN_SERVER_PORT}/session/${id}/subscription`, { headers: { origin: 'https://example.com' } @@ -896,15 +904,15 @@ nodeOnly(() => { const adminServer = getAdminServer(); - const client1 = getRemote(); - const client2 = getRemote(); + const client1 = getRemote({ adminServerUrl: TEST_ADMIN_SERVER_URL }); + const client2 = getRemote({ adminServerUrl: TEST_ADMIN_SERVER_URL }); afterEach(() => Promise.all([ client1.stop(), client2.stop() ])); - beforeEach(() => adminServer.start()); + beforeEach(() => adminServer.start(TEST_ADMIN_SERVER_PORT)); afterEach(() => adminServer.stop()); it("should expose events for mock server start & stop", async () => { @@ -984,7 +992,7 @@ nodeOnly(() => { await client1.start(); const clientPort = client1.port; - await resetAdminServer(); + await resetAdminServer({ adminServerUrl: TEST_ADMIN_SERVER_URL }); await client2.start(clientPort); // Client 1 should be broken now, because it was reset. It should _not_ try to @@ -1046,10 +1054,11 @@ nodeOnly(() => { const server = getAdminServer(); const client = getRemote({ + adminServerUrl: TEST_ADMIN_SERVER_URL, messageBodyDecoding: 'none' }); - before(() => server.start()); + before(() => server.start(TEST_ADMIN_SERVER_PORT)); after(() => server.stop()); beforeEach(() => client.start()); diff --git a/test/integration/subscriptions/raw-passthrough-events.spec.ts b/test/integration/subscriptions/raw-passthrough-events.spec.ts index 95538784f..4f29bceb3 100644 --- a/test/integration/subscriptions/raw-passthrough-events.spec.ts +++ b/test/integration/subscriptions/raw-passthrough-events.spec.ts @@ -8,7 +8,9 @@ import { makeDestroyable, nodeOnly, delay, - getDeferred + getDeferred, + TEST_ADMIN_SERVER_PORT, + TEST_ADMIN_SERVER_URL } from "../../test-utils"; nodeOnly(() => { @@ -144,12 +146,13 @@ nodeOnly(() => { describe("with a remote client", () => { const adminServer = getAdminServer(); const remoteClient = getRemote({ + adminServerUrl: TEST_ADMIN_SERVER_URL, socks: true, passthrough: ['unknown-protocol'] }); beforeEach(async () => { - await adminServer.start(); + await adminServer.start(TEST_ADMIN_SERVER_PORT); await remoteClient.start() }); afterEach(async () => { diff --git a/test/integration/subscriptions/request-events.spec.ts b/test/integration/subscriptions/request-events.spec.ts index 1978ef556..c8ad3745b 100644 --- a/test/integration/subscriptions/request-events.spec.ts +++ b/test/integration/subscriptions/request-events.spec.ts @@ -19,7 +19,9 @@ import { sendRawRequest, defaultNodeConnectionHeader, delay, - pollUntil + pollUntil, + TEST_ADMIN_SERVER_PORT, + TEST_ADMIN_SERVER_URL } from "../../test-utils"; // Headers we ignore when checking the received values, because they can vary depending @@ -198,9 +200,9 @@ describe("Request initiated subscriptions", () => { nodeOnly(() => { describe("with a remote client", () => { let adminServer = getAdminServer(); - let client = getRemote(); + let client = getRemote({ adminServerUrl: TEST_ADMIN_SERVER_URL }); - before(() => adminServer.start()); + before(() => adminServer.start(TEST_ADMIN_SERVER_PORT)); after(() => adminServer.stop()); beforeEach(() => client.start()); @@ -395,9 +397,9 @@ describe("Request subscriptions", () => { nodeOnly(() => { describe("with a remote client", () => { let adminServer = getAdminServer(); - let client = getRemote(); + let client = getRemote({ adminServerUrl: TEST_ADMIN_SERVER_URL }); - before(() => adminServer.start()); + before(() => adminServer.start(TEST_ADMIN_SERVER_PORT)); after(() => adminServer.stop()); beforeEach(() => client.start()); diff --git a/test/test-utils.ts b/test/test-utils.ts index d29c4e93d..a053dbfdd 100644 --- a/test/test-utils.ts +++ b/test/test-utils.ts @@ -128,6 +128,13 @@ export function httpGet(url: string): Promise { export const expect = chai.expect; +// A fixed admin server port for tests, outside the default ephemeral port ranges (Linux +// 32768-60999, macOS & Windows 49152-65535). The default 45454 is inside the Linux range, +// so an outgoing connection from another test can randomly hold it and block the admin +// server from binding. +export const TEST_ADMIN_SERVER_PORT = 30454; +export const TEST_ADMIN_SERVER_URL = `http://127.0.0.1:${TEST_ADMIN_SERVER_PORT}`; + export function browserOnly(body: Function) { if (!isNode) body(); }