diff --git a/packages/cli-kit/package.json b/packages/cli-kit/package.json index e102e827e5..cedbf26541 100644 --- a/packages/cli-kit/package.json +++ b/packages/cli-kit/package.json @@ -131,7 +131,6 @@ "find-up": "6.3.0", "form-data": "4.0.4", "fs-extra": "11.1.0", - "get-port-please": "3.1.2", "gradient-string": "2.0.2", "graphql": "16.10.0", "graphql-request": "6.1.0", diff --git a/packages/cli-kit/src/public/node/tcp-retry.test.ts b/packages/cli-kit/src/public/node/tcp-retry.test.ts new file mode 100644 index 0000000000..955ab8737d --- /dev/null +++ b/packages/cli-kit/src/public/node/tcp-retry.test.ts @@ -0,0 +1,63 @@ +import {AbortError} from './error.js' +import {describe, expect, test, vi, beforeEach} from 'vitest' +import {EventEmitter} from 'events' + +vi.mock('./system.js', async (importOriginal) => { + const actual: any = await importOriginal() + return {...actual, sleep: vi.fn()} +}) + +let callCount = 0 +let failCount = 0 + +vi.mock('net', () => { + return { + createServer: () => { + callCount++ + const server = new EventEmitter() as any + server.unref = () => server + server.listen = (_port: number, _host: string, cb?: () => void) => { + if (callCount <= failCount) { + process.nextTick(() => server.emit('error', new Error('mock port allocation failure'))) + } else { + server.address = () => ({port: 9999}) + process.nextTick(() => cb?.()) + } + return server + } + server.close = (cb?: () => void) => { + cb?.() + return server + } + return server + }, + } +}) + +beforeEach(() => { + callCount = 0 + failCount = 0 +}) + +describe('getAvailableTCPPort retry behavior', () => { + test('retries and returns port after transient error', async () => { + const {getAvailableTCPPort} = await import('./tcp.js') + const {sleep} = await import('./system.js') + + failCount = 1 + + const got = await getAvailableTCPPort(undefined, {waitTimeInSeconds: 0}) + expect(got).toBe(9999) + expect(sleep).toHaveBeenCalledOnce() + }) + + test('throws AbortError when all retries are exhausted', async () => { + const {getAvailableTCPPort} = await import('./tcp.js') + + failCount = 100 + + await expect(() => getAvailableTCPPort(undefined, {maxTries: 3, waitTimeInSeconds: 0})).rejects.toThrowError( + AbortError, + ) + }) +}) diff --git a/packages/cli-kit/src/public/node/tcp.test.ts b/packages/cli-kit/src/public/node/tcp.test.ts index 530b595fa9..3a42698e4f 100644 --- a/packages/cli-kit/src/public/node/tcp.test.ts +++ b/packages/cli-kit/src/public/node/tcp.test.ts @@ -1,100 +1,93 @@ import {getAvailableTCPPort, checkPortAvailability} from './tcp.js' -import * as system from './system.js' -import {AbortError} from './error.js' -import * as port from 'get-port-please' -import {describe, expect, test, vi} from 'vitest' - -vi.mock('get-port-please') - -const errorMessage = 'Unable to generate random port' +import {describe, expect, test} from 'vitest' +import {createServer} from 'net' describe('getAvailableTCPPort', () => { - test('returns random port if the number retries is not exceeded', async () => { - // Given - vi.mocked(port.getRandomPort).mockRejectedValueOnce(new Error(errorMessage)) - vi.mocked(port.getRandomPort).mockResolvedValue(5) - const debugError = vi.spyOn(system, 'sleep') - - // When - const got = await getAvailableTCPPort(undefined, {waitTimeInSeconds: 0}) - - // Then - expect(got).toBe(5) - expect(debugError).toHaveBeenCalledOnce() + test('returns a valid port number', async () => { + const port = await getAvailableTCPPort() + expect(port).toBeGreaterThan(0) + expect(port).toBeLessThanOrEqual(65535) }) - test('throws an abort exception with same error message received from third party getRandomPort if the number retries is exceeded', async () => { - // Given - const maxTries = 5 - for (let i = 0; i < maxTries; i++) { - vi.mocked(port.getRandomPort).mockRejectedValueOnce(new Error(errorMessage)) - } - - // When/Then - await expect(() => getAvailableTCPPort(undefined, {waitTimeInSeconds: 0})).rejects.toThrowError( - new AbortError(errorMessage), - ) + test('returns the preferred port when it is available', async () => { + const freePort = await getAvailableTCPPort() + const got = await getAvailableTCPPort(freePort) + expect(got).toBe(freePort) }) - test('returns the provided port when it is available', async () => { - // Given - vi.mocked(port.checkPort).mockResolvedValue(666) - - // When - const got = await getAvailableTCPPort(666) - - // Then - expect(got).toBe(666) + test('returns a different port when the preferred one is in use', async () => { + const server = createServer() + const occupiedPort = await new Promise((resolve) => { + server.listen(0, 'localhost', () => { + const address = server.address() + resolve((address as {port: number}).port) + }) + }) + + try { + const got = await getAvailableTCPPort(occupiedPort) + expect(got).not.toBe(occupiedPort) + expect(got).toBeGreaterThan(0) + } finally { + server.close() + } }) - test('returns a random port when the provided one is not available', async () => { - // Given - vi.mocked(port.checkPort).mockResolvedValue(false) - vi.mocked(port.getRandomPort).mockResolvedValue(5) - - // When - const got = await getAvailableTCPPort(666) - - // Then - expect(got).toBe(5) + test('returns unique ports across multiple calls', async () => { + const ports = new Set() + for (let i = 0; i < 5; i++) { + // eslint-disable-next-line no-await-in-loop + const port = await getAvailableTCPPort() + ports.add(port) + } + expect(ports.size).toBe(5) }) - test('reserves random ports and does not reuse them', async () => { - vi.mocked(port.checkPort).mockResolvedValue(false) - vi.mocked(port.getRandomPort).mockResolvedValueOnce(55).mockResolvedValueOnce(55).mockResolvedValueOnce(66) - - let got = await getAvailableTCPPort(123) - expect(got).toBe(55) - - got = await getAvailableTCPPort(123) - expect(got).toBe(66) + test('returns unique ports and all are bindable', async () => { + const ports: number[] = [] + for (let i = 0; i < 3; i++) { + // eslint-disable-next-line no-await-in-loop + ports.push(await getAvailableTCPPort()) + } + expect(new Set(ports).size).toBe(3) + + // Verify all ports are actually bindable + const servers = await Promise.all( + ports.map( + (port) => + new Promise>((resolve, reject) => { + const server = createServer() + server.once('error', reject) + server.listen(port, 'localhost', () => resolve(server)) + }), + ), + ) + // All three bound successfully — clean up + await Promise.all(servers.map((server) => new Promise((resolve) => server.close(() => resolve())))) }) }) describe('checkPortAvailability', () => { test('returns true when port is available', async () => { - // Given - const portNumber = 3000 - vi.mocked(port.checkPort).mockResolvedValue(portNumber) - - // When - const result = await checkPortAvailability(portNumber) - - // Then + const freePort = await getAvailableTCPPort() + const result = await checkPortAvailability(freePort) expect(result).toBe(true) - expect(port.checkPort).toHaveBeenCalledWith(portNumber, 'localhost') }) - test('returns false when port is not available', async () => { - // Given - const portNumber = 3000 - vi.mocked(port.checkPort).mockResolvedValue(false) - - // When - const result = await checkPortAvailability(portNumber) - - // Then - expect(result).toBe(false) - expect(port.checkPort).toHaveBeenCalledWith(portNumber, 'localhost') + test('returns false when port is in use', async () => { + const server = createServer() + const occupiedPort = await new Promise((resolve) => { + server.listen(0, 'localhost', () => { + const address = server.address() + resolve((address as {port: number}).port) + }) + }) + + try { + const result = await checkPortAvailability(occupiedPort) + expect(result).toBe(false) + } finally { + server.close() + } }) }) diff --git a/packages/cli-kit/src/public/node/tcp.ts b/packages/cli-kit/src/public/node/tcp.ts index 86d05ec552..ed194b450b 100644 --- a/packages/cli-kit/src/public/node/tcp.ts +++ b/packages/cli-kit/src/public/node/tcp.ts @@ -1,7 +1,7 @@ import {sleep} from './system.js' import {AbortError} from './error.js' import {outputDebug, outputContent, outputToken} from './output.js' -import * as port from 'get-port-please' +import {createServer} from 'net' interface GetTCPPortOptions { waitTimeInSeconds?: number @@ -23,14 +23,14 @@ export async function getAvailableTCPPort(preferredPort?: number, options?: GetT return preferredPort } outputDebug(outputContent`Getting a random port...`) - let randomPort = await retryOnError(() => port.getRandomPort(host()), options?.maxTries, options?.waitTimeInSeconds) + let randomPort = await retryOnError(() => getRandomPort(), options?.maxTries, options?.waitTimeInSeconds) for (let i = 0; i < (options?.maxTries ?? 5); i++) { if (!obtainedRandomPorts.has(randomPort)) { break } // eslint-disable-next-line no-await-in-loop - randomPort = await retryOnError(() => port.getRandomPort(host()), options?.maxTries, options?.waitTimeInSeconds) + randomPort = await retryOnError(() => getRandomPort(), options?.maxTries, options?.waitTimeInSeconds) } outputDebug(outputContent`Random port obtained: ${outputToken.raw(`${randomPort}`)}`) @@ -45,13 +45,36 @@ export async function getAvailableTCPPort(preferredPort?: number, options?: GetT * @returns A promise that resolves with a boolean indicating if the port is available. */ export async function checkPortAvailability(portNumber: number): Promise { - return (await port.checkPort(portNumber, host())) === portNumber + return new Promise((resolve) => { + const server = createServer() + server.unref() + server.once('error', () => resolve(false)) + server.listen(portNumber, 'localhost', () => { + server.close(() => resolve(true)) + }) + }) } -function host(): string | undefined { - // The get-port-please library does not work as expected when HOST env var is defined, - // so explicitly set the host to localhost to avoid conflicts - return 'localhost' +/** + * Gets a random available port by binding to port 0 on localhost. + * + * @returns A promise that resolves with an available port number. + */ +function getRandomPort(): Promise { + return new Promise((resolve, reject) => { + const server = createServer() + server.unref() + server.once('error', reject) + server.listen(0, 'localhost', () => { + const address = server.address() + if (address && typeof address === 'object') { + const assignedPort = address.port + server.close(() => resolve(assignedPort)) + } else { + server.close(() => reject(new Error('Unable to determine assigned port'))) + } + }) + }) } /** diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index c672a98396..adceb57523 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -387,9 +387,6 @@ importers: fs-extra: specifier: 11.1.0 version: 11.1.0 - get-port-please: - specifier: 3.1.2 - version: 3.1.2 gradient-string: specifier: 2.0.2 version: 2.0.2 @@ -5838,9 +5835,6 @@ packages: resolution: {integrity: sha512-pjzuKtY64GYfWizNAJ0fr9VqttZkNiK2iS430LtIHzjBEr6bX8Am2zm4sW4Ro5wjWW5cAlRL1qAMTcXbjNAO2Q==} engines: {node: '>=8.0.0'} - get-port-please@3.1.2: - resolution: {integrity: sha512-Gxc29eLs1fbn6LQ4jSU4vXjlwyZhF5HsGuMAa7gqBP4Rw4yxxltyDUuF5MBclFzDTXO+ACchGQoeela4DSfzdQ==} - get-port@7.1.0: resolution: {integrity: sha512-QB9NKEeDg3xxVwCCwJQ9+xycaz6pBB6iQ76wiWMl1927n0Kir6alPiP+yuiICLLU4jpMe08dXfpebuQppFA2zw==} engines: {node: '>=16'} @@ -15479,8 +15473,6 @@ snapshots: get-package-type@0.1.0: {} - get-port-please@3.1.2: {} - get-port@7.1.0: {} get-proto@1.0.1: