Skip to content

Commit 1e1b2bb

Browse files
committed
fix(transport-webrtc): check initial peer connection state at construction time
When RTCPeerConnectionMultiaddrConnection is constructed, ICE may have already reached a terminal state ('failed' or 'closed') before the onconnectionstatechange handler is registered. Because that handler is a property assignment, it does not fire retroactively for transitions that already occurred, leaving a half-open connection the connection manager never cleans up. Explicitly check the initial connectionState at construction time and call onTransportClosed() / peerConnection.close() when the state is already terminal. Note: 'disconnected' is transient — the ICE agent may recover to 'connected' — so only 'failed' and 'closed' are treated as terminal here. Closes #2646
1 parent 5002872 commit 1e1b2bb

2 files changed

Lines changed: 63 additions & 0 deletions

File tree

packages/transport-webrtc/src/rtcpeerconnection-to-conn.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,18 @@ class RTCPeerConnectionMultiaddrConnection extends AbstractMultiaddrConnection {
2929
this.peerConnection.close()
3030
}
3131
}
32+
33+
// Handle the case where the peerConnection already reached a terminal state
34+
// before this handler was registered (e.g. ICE failed during SDP exchange).
35+
// Since onconnectionstatechange is a property assignment it won't fire for
36+
// past state transitions, so we need to check the current state explicitly.
37+
// Note: 'disconnected' is transient and may recover to 'connected', so only
38+
// 'failed' and 'closed' are treated as terminal states here.
39+
if (initialState === 'failed' || initialState === 'closed') {
40+
this.log.trace('peer connection already in terminal state %s at construction time', initialState)
41+
this.onTransportClosed()
42+
this.peerConnection.close()
43+
}
3244
}
3345

3446
sendData (data: Uint8ArrayList): SendResult {

packages/transport-webrtc/test/maconn.spec.ts

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import { defaultLogger } from '@libp2p/logger'
44
import { multiaddr } from '@multiformats/multiaddr'
55
import { expect } from 'aegir/chai'
6+
import delay from 'delay'
67
import { stubObject } from 'sinon-ts'
78
import { toMultiaddrConnection } from '../src/rtcpeerconnection-to-conn.ts'
89
import { RTCPeerConnection } from '../src/webrtc/index.js'
@@ -33,4 +34,54 @@ describe('Multiaddr Connection', () => {
3334
expect(maConn.timeline.close).to.not.be.undefined
3435
expect(metrics.increment.calledWith({ close: true })).to.be.true
3536
})
37+
38+
it('closes immediately when peer connection is already in failed state at construction time', async () => {
39+
const peerConnection = {
40+
connectionState: 'failed' as RTCPeerConnectionState,
41+
onconnectionstatechange: null as any,
42+
close: () => {}
43+
}
44+
45+
const remoteAddr = multiaddr('/ip4/1.2.3.4/udp/1234/webrtc/certhash/uEiAUqV7kzvM1wI5DYDc1RbcekYVmXli_Qprlw3IkiEg6tQ')
46+
47+
const maConn = toMultiaddrConnection({
48+
// @ts-expect-error - intentional mock
49+
peerConnection,
50+
remoteAddr,
51+
direction: 'outbound',
52+
log: defaultLogger().forComponent('libp2p:webrtc:connection')
53+
})
54+
55+
// Give any microtasks or synchronous operations a chance to complete
56+
await delay(0)
57+
58+
expect(maConn.timeline.close).to.not.be.undefined
59+
})
60+
61+
it('closes when peer connection transitions to failed state after construction', async () => {
62+
const peerConnection: any = {
63+
connectionState: 'connected' as RTCPeerConnectionState,
64+
onconnectionstatechange: null as any,
65+
close: () => {}
66+
}
67+
68+
const remoteAddr = multiaddr('/ip4/1.2.3.4/udp/1234/webrtc/certhash/uEiAUqV7kzvM1wI5DYDc1RbcekYVmXli_Qprlw3IkiEg6tQ')
69+
70+
const maConn = toMultiaddrConnection({
71+
peerConnection,
72+
remoteAddr,
73+
direction: 'outbound',
74+
log: defaultLogger().forComponent('libp2p:webrtc:connection')
75+
})
76+
77+
expect(maConn.timeline.close).to.be.undefined
78+
79+
// Simulate peerConnection going to 'failed' after construction
80+
peerConnection.connectionState = 'failed'
81+
peerConnection.onconnectionstatechange?.()
82+
83+
await delay(0)
84+
85+
expect(maConn.timeline.close).to.not.be.undefined
86+
})
3687
})

0 commit comments

Comments
 (0)