fix(mesh): close error-listener race in pilot tunnel reverse-dial connect (#1754)

acceptReverseLocal swapped a pre-connect error handler for permanent
close/error handlers inside the socket's 'connect' callback, briefly
leaving 'error' with the old handler removed and the new ones not yet
attached. A Node EventEmitter 'error' event with zero listeners
throws instead of being swallowed, and CI's runner timing hit this
window intermittently (backend-tests-pilot-tunnel-bridge-reverse-
route-events.test.ts's post-handshake-close test), surfacing as an
unhandled ECONNRESET exception even though every assertion passed.

Replace the swap with a single 'error' listener attached at socket
creation that branches on a connected flag, so the socket is never
without an error listener at any point in its lifecycle.
This commit is contained in:
Anso
2026-08-02 21:08:07 -04:00
committed by GitHub
parent c613010199
commit 48c89d217d
+26 -17
View File
@@ -918,23 +918,33 @@ export class PilotTunnelBridge extends EventEmitter implements MeshTunnelHandle
if (sendClose) this.sendJson({ t: 'tcp_close', s });
};
// Pre-connect failure: ack-fail and drop. The handler is removed in
// 'connect' below so post-connect errors fall through to the
// mid-stream teardown path instead of double-firing.
const onPreConnectError = (err?: Error) => {
if (!this.streams.has(s)) return;
this.streams.delete(s);
meshSvc.logActivity({
source: 'mesh', level: 'error', type: 'route.resolve.fail',
nodeId: this.nodeId,
message: `reverse dial failed pre-connect: ${err?.message ?? 'socket error'}`,
details: { ...baseDetails, reason: 'connect_error' },
});
this.sendJson({ t: 'tcp_open_ack', s, ok: false, err: 'unreachable' });
};
socket.once('error', onPreConnectError);
// One persistent 'error' listener attached at socket creation,
// branching on `connected`, instead of swapping a pre-connect handler
// for a post-connect one inside the 'connect' callback. A swap leaves
// a window (real under some schedulers, e.g. CI runners) where the
// socket briefly has zero 'error' listeners between removing the old
// one and attaching the new one; a Node EventEmitter 'error' with no
// listener throws instead of being swallowed. Keeping a single
// listener for the socket's whole lifetime removes that window
// entirely rather than narrowing it.
let connected = false;
socket.on('error', (err?: Error) => {
if (!connected) {
if (!this.streams.has(s)) return;
this.streams.delete(s);
meshSvc.logActivity({
source: 'mesh', level: 'error', type: 'route.resolve.fail',
nodeId: this.nodeId,
message: `reverse dial failed pre-connect: ${err?.message ?? 'socket error'}`,
details: { ...baseDetails, reason: 'connect_error' },
});
this.sendJson({ t: 'tcp_open_ack', s, ok: false, err: 'unreachable' });
return;
}
teardown(true);
});
socket.once('connect', () => {
socket.off('error', onPreConnectError);
connected = true;
meshSvc.logActivity({
source: 'mesh', level: 'info', type: 'route.resolve.ok',
nodeId: this.nodeId,
@@ -960,7 +970,6 @@ export class PilotTunnelBridge extends EventEmitter implements MeshTunnelHandle
this.refreshIdleTimer(s, cur);
});
socket.on('close', () => teardown(true));
socket.on('error', () => teardown(true));
});
}