Skip to content

Commit 8bdfff2

Browse files
authored
fix(emulator): reject TCP start() Promise on port binding errors (#277)
Fixes TCP transport start() method to properly reject the Promise when port binding fails or initialization hangs. Previously, errors were only logged to console, causing applications to hang indefinitely on port conflicts. Changes: - Added reject parameter to Promise constructor in start() method - Error events during initialization now reject the Promise instead of just logging - Added 10-second timeout to prevent hanging when server never emits events - Errors after successful initialization continue to be logged (existing behavior) - Added type cast for error event to satisfy TypeScript compiler - Comprehensive test coverage for port binding errors and timeout scenarios Root cause: Promise constructor only had resolve parameter, no reject. When modbus-serial ServerTCP emitted error events during initialization, they were logged but never propagated to the caller, leaving the Promise pending forever. Testing: - New test: Port binding error rejection (EADDRINUSE scenario) - New test: Initialization timeout (hanging server scenario) - All 1607 existing tests pass - Preserves existing error handling behavior for post-initialization errors Follow-up issues: - #274 (HIGH): Event listener cleanup on initialization failure - #275 (HIGH): Race condition when calling stop() during initialization - #276 (MEDIUM): Error messages should include host/port context Closes #254
1 parent 0e277d7 commit 8bdfff2

2 files changed

Lines changed: 94 additions & 6 deletions

File tree

packages/emulator/src/transports/tcp.test.ts

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,77 @@ describe('TcpTransport', () => {
178178
})
179179

180180
describe('error handling', () => {
181+
it('should reject start() on port binding error', async () => {
182+
// Test for issue #254: start() should reject when port binding fails
183+
const { ServerTCP } = await import('modbus-serial')
184+
const bindError = new Error('listen EADDRINUSE: address already in use')
185+
let errorCallback: ((err: Error) => void) | undefined
186+
;(ServerTCP as any).mockImplementationOnce((vector: any, _options: any) => {
187+
capturedServiceVector = vector
188+
initializedListener = undefined
189+
errorListener = undefined
190+
191+
const localMockInstance = {
192+
close: jest.fn((cb: (err: Error | null) => void) => cb(null)),
193+
on: jest.fn((event: string, listener: any) => {
194+
if (event === 'initialized') {
195+
initializedListener = listener
196+
} else if (event === 'error') {
197+
errorListener = listener
198+
errorCallback = listener
199+
}
200+
}),
201+
removeAllListeners: jest.fn(),
202+
socks: new Map(),
203+
}
204+
205+
// Simulate error during initialization (instead of initialized event)
206+
setImmediate(() => {
207+
if (errorCallback) {
208+
errorCallback(bindError)
209+
}
210+
})
211+
212+
return localMockInstance
213+
})
214+
215+
const testTransport = new TcpTransport({ host: 'localhost', port: 8502 })
216+
await expect(testTransport.start()).rejects.toThrow(
217+
'listen EADDRINUSE: address already in use'
218+
)
219+
})
220+
221+
it('should reject start() on initialization timeout', async () => {
222+
// Test for issue #254: start() should timeout if server never emits initialized or error
223+
const { ServerTCP } = await import('modbus-serial')
224+
225+
;(ServerTCP as any).mockImplementationOnce((vector: any, _options: any) => {
226+
capturedServiceVector = vector
227+
initializedListener = undefined
228+
errorListener = undefined
229+
230+
const localMockInstance = {
231+
close: jest.fn((cb: (err: Error | null) => void) => cb(null)),
232+
on: jest.fn((event: string, listener: any) => {
233+
if (event === 'initialized') {
234+
initializedListener = listener
235+
} else if (event === 'error') {
236+
errorListener = listener
237+
}
238+
}),
239+
removeAllListeners: jest.fn(),
240+
socks: new Map(),
241+
}
242+
243+
// Don't emit initialized or error - simulate hanging server
244+
245+
return localMockInstance
246+
})
247+
248+
const testTransport = new TcpTransport({ host: 'localhost', port: 8502 })
249+
await expect(testTransport.start()).rejects.toThrow(/timeout/i)
250+
}, 15000) // Increase test timeout to allow for implementation timeout
251+
181252
it('should reject on stop error', async () => {
182253
const { ServerTCP } = await import('modbus-serial')
183254
const closeError = new Error('Failed to close server')

packages/emulator/src/transports/tcp.ts

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -83,17 +83,34 @@ export class TcpTransport extends BaseTransport {
8383
const server = new ServerTCP(serviceVector, options)
8484
this.server = server
8585

86-
// Wait for initialized event
87-
return new Promise<void>((resolve) => {
86+
// Wait for initialized event with timeout
87+
return new Promise<void>((resolve, reject) => {
88+
let isInitializing = true
89+
90+
const timeout = setTimeout(() => {
91+
isInitializing = false
92+
delete this.server
93+
reject(new Error('TCP server initialization timeout after 10s'))
94+
}, 10000)
95+
8896
server.on('initialized', () => {
97+
clearTimeout(timeout)
98+
isInitializing = false
8999
this.started = true
90100
resolve()
91101
})
92102

93-
// Handle errors
94-
server.on('error', (err) => {
95-
// Log error but don't stop server
96-
console.error('TCP transport error:', err)
103+
// Handle errors - cast to EventEmitter as modbus-serial types don't include 'error' event
104+
;(server as unknown as EventEmitter).on('error', (err: Error) => {
105+
if (isInitializing) {
106+
clearTimeout(timeout)
107+
isInitializing = false
108+
delete this.server
109+
reject(err)
110+
} else {
111+
// Log errors that occur after initialization
112+
console.error('TCP transport error:', err)
113+
}
97114
})
98115
})
99116
}

0 commit comments

Comments
 (0)