Skip to content

Commit 4d31316

Browse files
gmaclennanclaude
andauthored
fix: fix bug when re-listening the server (#16)
* fix: small type fix * test: add failing test for server restart with changed ports The deferredListen promise is not reset when close() is called, causing getRemotePort() to return stale port values after server restart. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: reset deferredListen promise on server close Ensures that when the server is restarted with listen() after close(), the getRemotePort function returns the new port values instead of the stale values from the previous listen() call. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: increase test timeout on node 18 --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent a2bed04 commit 4d31316

2 files changed

Lines changed: 147 additions & 4 deletions

File tree

src/index.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import assert from 'node:assert'
22
import { once } from 'node:events'
33
import http from 'node:http'
4-
import { type AddressInfo } from 'node:net'
4+
import { type AddressInfo, type Socket } from 'node:net'
55

66
import { createServerAdapter } from '@whatwg-node/server'
77
import pDefer from 'p-defer'
@@ -56,7 +56,7 @@ export function createServer(options: ServerOptions) {
5656
options.keyPair = Agent.keyPair()
5757
}
5858

59-
const deferredListen = pDefer<ListenResult>()
59+
let deferredListen = pDefer<ListenResult>()
6060
const context = new Context({
6161
...options,
6262
keyPair: options.keyPair,
@@ -86,8 +86,8 @@ export function createServer(options: ServerOptions) {
8686
})
8787

8888
// Track connections for proper cleanup
89-
const connections = new Set<any>()
90-
const onConnection = (socket: any) => {
89+
const connections = new Set<Socket>()
90+
const onConnection = (socket: Socket) => {
9191
connections.add(socket)
9292
socket.once('close', () => {
9393
connections.delete(socket)
@@ -124,6 +124,8 @@ export function createServer(options: ServerOptions) {
124124
once(localHttpServer, 'close'),
125125
once(secretStreamServer, 'close'),
126126
])
127+
// Reset deferred listen for potential restart with different ports
128+
deferredListen = pDefer<ListenResult>()
127129
},
128130
}
129131
}

test/server-restart.test.ts

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
import fs from 'node:fs/promises'
2+
import os from 'node:os'
3+
import path from 'node:path'
4+
5+
import { Agent as SecretStreamAgent } from 'secret-stream-http'
6+
import { describe, it, expect } from 'vitest'
7+
8+
import { createServer } from '../src/index.js'
9+
import { DEMOTILES_Z2, OSM_BRIGHT_Z6 } from './helpers.js'
10+
11+
describe('Server Restart', () => {
12+
it('should use new ports when restarted with different ports', async (t) => {
13+
const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'map-server-test-'))
14+
const tmpCustomMapPath = path.join(tmpDir, 'custom-map.smp')
15+
await fs.copyFile(OSM_BRIGHT_Z6, tmpCustomMapPath)
16+
17+
t.onTestFinished(async () => {
18+
await fs.rm(tmpDir, { recursive: true, force: true }).catch(() => {})
19+
})
20+
21+
const keyPair = SecretStreamAgent.keyPair()
22+
const receiverKeyPair = SecretStreamAgent.keyPair(Buffer.alloc(32, 1))
23+
const receiverDeviceId = Buffer.from(receiverKeyPair.publicKey).toString(
24+
'hex',
25+
)
26+
27+
const server = createServer({
28+
defaultOnlineStyleUrl: 'https://demotiles.maplibre.org/style.json',
29+
customMapPath: tmpCustomMapPath,
30+
fallbackMapPath: DEMOTILES_Z2,
31+
keyPair,
32+
})
33+
34+
// First listen - use specific ports
35+
const firstResult = await server.listen({
36+
localPort: 0,
37+
remotePort: 0,
38+
})
39+
const firstLocalPort = firstResult.localPort
40+
const firstRemotePort = firstResult.remotePort
41+
42+
// Create a map share to capture the remote port being used
43+
const firstShareResponse = await fetch(
44+
`http://127.0.0.1:${firstLocalPort}/mapShares`,
45+
{
46+
method: 'POST',
47+
headers: { 'Content-Type': 'application/json' },
48+
body: JSON.stringify({
49+
mapId: 'custom',
50+
receiverDeviceId,
51+
}),
52+
},
53+
)
54+
expect(firstShareResponse.status).toBe(201)
55+
const firstShare = await firstShareResponse.json()
56+
57+
// The mapShareUrls should contain the first remote port
58+
expect(firstShare.mapShareUrls).toBeDefined()
59+
expect(firstShare.mapShareUrls.length).toBeGreaterThan(0)
60+
const firstUrl = new URL(firstShare.mapShareUrls[0])
61+
expect(firstUrl.port).toBe(String(firstRemotePort))
62+
63+
// Close the server
64+
await server.close()
65+
66+
// Listen again - ports will be different (since we use 0)
67+
const secondResult = await server.listen({
68+
localPort: 0,
69+
remotePort: 0,
70+
})
71+
const secondLocalPort = secondResult.localPort
72+
const secondRemotePort = secondResult.remotePort
73+
74+
// Ports should be different (very likely with port 0)
75+
// At minimum, the listen result should reflect the new ports
76+
expect(secondResult.localPort).not.toBe(firstLocalPort)
77+
78+
// Create another map share - this should use the NEW remote port
79+
const secondShareResponse = await fetch(
80+
`http://127.0.0.1:${secondLocalPort}/mapShares`,
81+
{
82+
method: 'POST',
83+
headers: { 'Content-Type': 'application/json' },
84+
body: JSON.stringify({
85+
mapId: 'custom',
86+
receiverDeviceId,
87+
}),
88+
},
89+
)
90+
expect(secondShareResponse.status).toBe(201)
91+
const secondShare = await secondShareResponse.json()
92+
93+
// The mapShareUrls should contain the SECOND remote port, not the first
94+
expect(secondShare.mapShareUrls).toBeDefined()
95+
expect(secondShare.mapShareUrls.length).toBeGreaterThan(0)
96+
const secondUrl = new URL(secondShare.mapShareUrls[0])
97+
98+
// This is the key assertion - the URL should have the new port
99+
expect(secondUrl.port).toBe(String(secondRemotePort))
100+
expect(secondUrl.port).not.toBe(String(firstRemotePort))
101+
102+
// Clean up
103+
await server.close()
104+
})
105+
106+
it('should work correctly after multiple restart cycles', async (t) => {
107+
const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'map-server-test-'))
108+
const tmpCustomMapPath = path.join(tmpDir, 'custom-map.smp')
109+
await fs.copyFile(OSM_BRIGHT_Z6, tmpCustomMapPath)
110+
111+
t.onTestFinished(async () => {
112+
await fs.rm(tmpDir, { recursive: true, force: true }).catch(() => {})
113+
})
114+
115+
const server = createServer({
116+
defaultOnlineStyleUrl: 'https://demotiles.maplibre.org/style.json',
117+
customMapPath: tmpCustomMapPath,
118+
fallbackMapPath: DEMOTILES_Z2,
119+
})
120+
121+
const ports: number[] = []
122+
123+
// Restart 3 times
124+
for (let i = 0; i < 3; i++) {
125+
const result = await server.listen({ localPort: 0, remotePort: 0 })
126+
ports.push(result.localPort)
127+
128+
// Verify server is working
129+
const response = await fetch(
130+
`http://127.0.0.1:${result.localPort}/maps/custom/style.json`,
131+
)
132+
expect(response.status).toBe(200)
133+
134+
await server.close()
135+
}
136+
137+
// All ports should be different (extremely likely with port 0)
138+
const uniquePorts = new Set(ports)
139+
expect(uniquePorts.size).toBe(3)
140+
}, 20_000) // On node 18 restarting can be slow.
141+
})

0 commit comments

Comments
 (0)