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/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 8dd11e5ab..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 @@ -1004,14 +1012,53 @@ 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(); 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(); }