-
Notifications
You must be signed in to change notification settings - Fork 111
fix: port detection, postgres nitro apps and nitro app testing #363
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
51b7160
90cfebb
04fda69
101fd54
16d54f6
1e64d5d
22d1e7c
0820c23
c72742b
a9228b4
0a89c37
90d3b21
28afae3
440a702
cde4a51
3aa7f80
a53a84b
90f3c3d
6c86870
fbb557a
ec106fb
27f8e5e
d18a210
da85ce0
320d63e
1d6920f
2c8e3b7
950720a
80369c6
9477405
db44b66
58435f2
9105681
3f8aa90
4e2f574
5955328
e8cd7d1
e4d067f
78506cb
eb35c3b
0a2376f
0412a8c
ec2162f
37c4481
6a43f2c
32850cd
1fc7f73
a492a2e
32f74ab
19098ba
7c24436
a5d25c9
5bdc023
41c5ee2
56e4c9c
0afd8a4
c5f6434
ebff0a3
25e714a
3f4a23c
d9e60ef
19ca4e2
f43e1c0
d618959
f467071
cbd4f9c
ae1b490
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| --- | ||
| "@workflow/world-local": patch | ||
| "@workflow/utils": patch | ||
| --- | ||
|
|
||
| Fix port detection and base URL resolution for dev servers |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,80 +1,165 @@ | ||
| import http from 'node:http'; | ||
| import { describe, expect, it } from 'vitest'; | ||
| import type { AddressInfo } from 'node:net'; | ||
| import { afterEach, describe, expect, it } from 'vitest'; | ||
| import { getPort } from './get-port'; | ||
|
|
||
| describe('getPort', () => { | ||
| it('should return undefined or a positive number', async () => { | ||
| let servers: http.Server[] = []; | ||
|
|
||
| afterEach(() => { | ||
| servers.forEach((server) => { | ||
| server.close(); | ||
| }); | ||
| servers = []; | ||
| }); | ||
|
|
||
| it('should return undefined when no ports are in use', async () => { | ||
| const port = await getPort(); | ||
| expect(port === undefined || typeof port === 'number').toBe(true); | ||
| if (port !== undefined) { | ||
| expect(port).toBeGreaterThan(0); | ||
| } | ||
|
|
||
| expect(port).toBeUndefined(); | ||
| }); | ||
|
|
||
| it('should handle servers listening on specific ports', async () => { | ||
| const server = http.createServer(); | ||
| servers.push(server); | ||
|
|
||
| // Listen on a specific port instead of 0 | ||
| const specificPort = 3000; | ||
| server.listen(specificPort); | ||
|
|
||
| const port = await getPort(); | ||
|
|
||
| expect(port).toEqual(specificPort); | ||
| }); | ||
|
|
||
| it('should return a port number when a server is listening', async () => { | ||
| it('should return the port number that the server is listening', async () => { | ||
| const server = http.createServer(); | ||
| servers.push(server); | ||
|
|
||
| server.listen(0); | ||
|
|
||
| try { | ||
| const port = await getPort(); | ||
| const address = server.address(); | ||
|
|
||
| // Port detection may not work immediately in all environments (CI, Docker, etc.) | ||
| // so we just verify the function returns a valid result | ||
| if (port !== undefined) { | ||
| expect(typeof port).toBe('number'); | ||
| expect(port).toBeGreaterThan(0); | ||
|
|
||
| // If we have the address, optionally verify it matches | ||
| if (address && typeof address === 'object') { | ||
| // In most cases it should match, but not required for test to pass | ||
| expect([port, undefined]).toContain(port); | ||
| } | ||
adriandlam marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| } finally { | ||
| await new Promise<void>((resolve, reject) => { | ||
| server.close((err) => (err ? reject(err) : resolve())); | ||
| }); | ||
| } | ||
| const port = await getPort(); | ||
| const addr = server.address() as AddressInfo; | ||
|
|
||
| expect(typeof port).toBe('number'); | ||
| expect(port).toEqual(addr.port); | ||
| }); | ||
|
|
||
| it('should return the smallest port when multiple servers are listening', async () => { | ||
| it('should return the first port of the server', async () => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The test assumes View Details📝 Patch Detailsdiff --git a/packages/utils/src/get-port.test.ts b/packages/utils/src/get-port.test.ts
index 0dbad85..3b21784 100644
--- a/packages/utils/src/get-port.test.ts
+++ b/packages/utils/src/get-port.test.ts
@@ -45,7 +45,7 @@ describe('getPort', () => {
expect(port).toEqual(addr.port);
});
- it('should return the first port of the server', async () => {
+ it('should return the smallest port when multiple servers are listening', async () => {
const server1 = http.createServer();
const server2 = http.createServer();
servers.push(server1);
@@ -56,8 +56,10 @@ describe('getPort', () => {
const port = await getPort();
const addr1 = server1.address() as AddressInfo;
+ const addr2 = server2.address() as AddressInfo;
- expect(port).toEqual(addr1.port);
+ const smallestPort = Math.min(addr1.port, addr2.port);
+ expect(port).toEqual(smallestPort);
});
it('should return consistent results when called multiple times', async () => {
diff --git a/packages/utils/src/get-port.ts b/packages/utils/src/get-port.ts
index 96eb402..d1eb9a3 100644
--- a/packages/utils/src/get-port.ts
+++ b/packages/utils/src/get-port.ts
@@ -7,7 +7,7 @@ import { execa } from 'execa';
export async function getPort(): Promise<number | undefined> {
const { pid, platform } = process;
- let port: number | undefined;
+ let ports: number[] = [];
try {
switch (platform) {
@@ -21,14 +21,22 @@ export async function getPort(): Promise<number | undefined> {
'-p',
pid.toString(),
]);
- const awkResult = await execa(
- 'awk',
- ['/LISTEN/ {split($9,a,":"); print a[length(a)]; exit}'],
- {
- input: lsofResult.stdout,
+ const lines = lsofResult.stdout.split('\n');
+ for (const line of lines) {
+ if (line.includes('LISTEN')) {
+ const parts = line.split(/\s+/);
+ if (parts.length > 8) {
+ const addr = parts[8];
+ const portMatch = addr.match(/:(\d+)$/);
+ if (portMatch) {
+ const p = parseInt(portMatch[1], 10);
+ if (!Number.isNaN(p) && !ports.includes(p)) {
+ ports.push(p);
+ }
+ }
+ }
}
- );
- port = parseInt(awkResult.stdout.trim(), 10);
+ }
break;
}
@@ -47,8 +55,10 @@ export async function getPort(): Promise<number | undefined> {
// Extract port from the local address column
const match = line.trim().match(/^\s*TCP\s+[\d.:]+:(\d+)\s+/);
if (match) {
- port = parseInt(match[1], 10);
- break;
+ const p = parseInt(match[1], 10);
+ if (!Number.isNaN(p) && !ports.includes(p)) {
+ ports.push(p);
+ }
}
}
}
@@ -63,5 +73,10 @@ export async function getPort(): Promise<number | undefined> {
return undefined;
}
- return Number.isNaN(port) ? undefined : port;
+ if (ports.length === 0) {
+ return undefined;
+ }
+
+ // Return the smallest port for consistent behavior
+ return Math.min(...ports);
}
AnalysisTest assumes getPort() returns server1's port but implementation returns first port from lsof/netstatWhat fails: The test "should return the first port of the server" in How to reproduce: Create two HTTP servers with random ports and call getPort(): server1.listen(0);
server2.listen(0);
const port = await getPort();Since OS assigns random ports, server1 doesn't always have a lower port than server2. The original implementation used lsof/netstat which returns the "first" listening socket, but lsof output order is not guaranteed to match server creation order. The test incorrectly assumed getPort() would return server1's port specifically. Result: The test assertion Expected: Per the original implementation (commit adf0cfe), getPort() should return the smallest port for predictable behavior. Updated implementation to collect all listening ports and return Math.min(...ports). Test updated to verify the smallest port is returned, not server1's specific port. Fix implemented:
|
||
| const server1 = http.createServer(); | ||
| const server2 = http.createServer(); | ||
| servers.push(server1); | ||
| servers.push(server2); | ||
|
|
||
| server1.listen(0); | ||
| server2.listen(0); | ||
|
|
||
| const port = await getPort(); | ||
| const addr1 = server1.address() as AddressInfo; | ||
|
|
||
| expect(port).toEqual(addr1.port); | ||
| }); | ||
|
|
||
| it('should return consistent results when called multiple times', async () => { | ||
| const server = http.createServer(); | ||
| servers.push(server); | ||
| server.listen(0); | ||
|
|
||
| const port1 = await getPort(); | ||
| const port2 = await getPort(); | ||
| const port3 = await getPort(); | ||
|
|
||
| expect(port1).toEqual(port2); | ||
| expect(port2).toEqual(port3); | ||
| }); | ||
|
|
||
| it('should handle IPv6 addresses', async () => { | ||
| const server = http.createServer(); | ||
| servers.push(server); | ||
|
|
||
| try { | ||
| server.listen(0, '::1'); // IPv6 localhost | ||
| const port = await getPort(); | ||
| const addr1 = server1.address(); | ||
| const addr2 = server2.address(); | ||
|
|
||
| // Port detection may not work in all environments | ||
| if ( | ||
| port !== undefined && | ||
| addr1 && | ||
| typeof addr1 === 'object' && | ||
| addr2 && | ||
| typeof addr2 === 'object' | ||
| ) { | ||
| // Should return the smallest port | ||
| expect(port).toBeLessThanOrEqual(Math.max(addr1.port, addr2.port)); | ||
| expect(port).toBeGreaterThan(0); | ||
| } else { | ||
| // If port detection doesn't work in this environment, just pass | ||
| expect(port === undefined || typeof port === 'number').toBe(true); | ||
| } | ||
| } finally { | ||
| await Promise.all([ | ||
| new Promise<void>((resolve, reject) => { | ||
| server1.close((err) => (err ? reject(err) : resolve())); | ||
| }), | ||
| new Promise<void>((resolve, reject) => { | ||
| server2.close((err) => (err ? reject(err) : resolve())); | ||
| }), | ||
| ]); | ||
| const addr = server.address() as AddressInfo; | ||
|
|
||
| expect(port).toEqual(addr.port); | ||
| } catch { | ||
| // Skip test if IPv6 is not available | ||
| console.log('IPv6 not available, skipping test'); | ||
| } | ||
| }); | ||
|
|
||
| it('should handle multiple calls in sequence', async () => { | ||
| const server = http.createServer(); | ||
| servers.push(server); | ||
|
|
||
| server.listen(0); | ||
|
|
||
| const port1 = await getPort(); | ||
| const port2 = await getPort(); | ||
| const addr = server.address() as AddressInfo; | ||
|
|
||
| // Should return the same port each time | ||
| expect(port1).toEqual(addr.port); | ||
| expect(port2).toEqual(addr.port); | ||
| }); | ||
|
|
||
| it('should handle closed servers', async () => { | ||
| const server = http.createServer(); | ||
|
|
||
| server.listen(0); | ||
| const addr = server.address() as AddressInfo; | ||
| const serverPort = addr.port; | ||
|
|
||
| // Close the server before calling getPort | ||
| server.close(); | ||
|
|
||
| const port = await getPort(); | ||
|
|
||
| // Port should not be the closed server's port | ||
| expect(port).not.toEqual(serverPort); | ||
| }); | ||
|
|
||
| it('should handle server restart on same port', async () => { | ||
| const server1 = http.createServer(); | ||
| servers.push(server1); | ||
| server1.listen(3000); | ||
|
|
||
| const port1 = await getPort(); | ||
| expect(port1).toEqual(3000); | ||
|
|
||
| server1.close(); | ||
| servers = servers.filter((s) => s !== server1); | ||
|
|
||
| // Small delay to ensure port is released | ||
| await new Promise((resolve) => setTimeout(resolve, 100)); | ||
|
|
||
| const server2 = http.createServer(); | ||
| servers.push(server2); | ||
| server2.listen(3000); | ||
|
|
||
| const port2 = await getPort(); | ||
| expect(port2).toEqual(3000); | ||
| }); | ||
|
|
||
| it('should handle concurrent getPort calls', async () => { | ||
| // Workflow makes lots of concurrent getPort calls | ||
| const server = http.createServer(); | ||
| servers.push(server); | ||
| server.listen(0); | ||
adriandlam marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| const addr = server.address() as AddressInfo; | ||
|
|
||
| // Call getPort concurrently 10 times | ||
| const results = await Promise.all( | ||
| Array(10) | ||
| .fill(0) | ||
| .map(() => getPort()) | ||
| ); | ||
|
|
||
| // All should return the same port without errors | ||
| results.forEach((port) => { | ||
| expect(port).toEqual(addr.port); | ||
| }); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,23 +1,70 @@ | ||
| import { pidToPorts } from 'pid-port'; | ||
| import { execa } from 'execa'; | ||
|
|
||
| /** | ||
| * Gets the port number that the process is listening on. | ||
| * @returns The port number that the process is listening on, or undefined if the process is not listening on any port. | ||
| * NOTE: Can't move this to @workflow/utils because it's being imported into @workflow/errors for RetryableError (inside workflow runtime) | ||
| */ | ||
| export async function getPort(): Promise<number | undefined> { | ||
| const { pid, platform } = process; | ||
|
|
||
| let port: number | undefined; | ||
|
|
||
| try { | ||
| const pid = process.pid; | ||
| const ports = await pidToPorts(pid); | ||
| if (!ports || ports.size === 0) { | ||
| return undefined; | ||
| } | ||
| switch (platform) { | ||
| case 'linux': | ||
| case 'darwin': { | ||
| const lsofResult = await execa('lsof', [ | ||
| '-a', | ||
| '-i', | ||
| '-P', | ||
| '-n', | ||
| '-p', | ||
| pid.toString(), | ||
| ]); | ||
| const awkResult = await execa( | ||
| 'awk', | ||
| ['/LISTEN/ {split($9,a,":"); print a[length(a)]; exit}'], | ||
| { | ||
| input: lsofResult.stdout, | ||
| } | ||
| ); | ||
| port = parseInt(awkResult.stdout.trim(), 10); | ||
| break; | ||
| } | ||
|
|
||
| case 'win32': { | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. had to spin up a windows vm for this |
||
| // Use cmd to run the piped command | ||
| const result = await execa('cmd', [ | ||
| '/c', | ||
| `netstat -ano | findstr ${pid} | findstr LISTENING`, | ||
| ]); | ||
|
|
||
| const smallest = Math.min(...ports); | ||
| return smallest; | ||
| } catch { | ||
| // If port detection fails (e.g., `ss` command not available in production), | ||
| // return undefined and fall back to default port | ||
| const stdout = result.stdout.trim(); | ||
|
|
||
| if (stdout) { | ||
| const lines = stdout.split('\n'); | ||
| for (const line of lines) { | ||
| // Extract port from the local address column | ||
| // Matches both IPv4 (e.g., "127.0.0.1:3000") and IPv6 bracket notation (e.g., "[::1]:3000") | ||
| const match = line | ||
| .trim() | ||
| .match(/^\s*TCP\s+(?:\[[\da-f:]+\]|[\d.]+):(\d+)\s+/i); | ||
| if (match) { | ||
| port = parseInt(match[1], 10); | ||
| break; | ||
| } | ||
| } | ||
| } | ||
| break; | ||
| } | ||
| } | ||
| } catch (error) { | ||
| // In dev, it's helpful to know why detection failed | ||
| if (process.env.NODE_ENV === 'development') { | ||
| console.debug('[getPort] Detection failed:', error); | ||
| } | ||
| return undefined; | ||
| } | ||
|
|
||
| return Number.isNaN(port) ? undefined : port; | ||
| } | ||

Uh oh!
There was an error while loading. Please reload this page.