Skip to content
This repository has been archived by the owner on Jun 19, 2023. It is now read-only.

fix: added peer connection state listener to emit closed events #134

Merged
merged 5 commits into from
Apr 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/maconn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,9 @@ export class WebRTCMultiaddrConnection implements MultiaddrConnection {
log.error('error closing connection', err)
}

this.timeline.close = new Date().getTime()
log.trace('closing connection')

this.timeline.close = Date.now()
this.peerConnection.close()
}
}
2 changes: 1 addition & 1 deletion src/peer_transport/transport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ export class WebRTCTransport implements Transport, Startable {
const result = await options.upgrader.upgradeOutbound(
new WebRTCMultiaddrConnection({
peerConnection: pc,
timeline: { open: (new Date()).getTime() },
timeline: { open: Date.now() },
remoteAddr: webrtcMultiaddr
}),
{
Expand Down
2 changes: 1 addition & 1 deletion src/peer_transport/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import * as pb from './pb/index.js'
import { detect } from 'detect-browser'

const browser = detect()
const isFirefox = ((browser != null) && browser.name === 'firefox')
export const isFirefox = ((browser != null) && browser.name === 'firefox')

interface MessageStream {
read: () => Promise<pb.Message>
Expand Down
25 changes: 24 additions & 1 deletion src/transport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import * as sdp from './sdp.js'
import { WebRTCStream } from './stream.js'
import { genUfrag } from './util.js'
import { protocols } from '@multiformats/multiaddr'
import { isFirefox } from './peer_transport/util.js'

const log = logger('libp2p:webrtc:transport')

Expand Down Expand Up @@ -97,6 +98,9 @@ export class WebRTCDirectTransport implements Transport {
* Connect to a peer using a multiaddr
*/
async _connect (ma: Multiaddr, options: WebRTCDialOptions): Promise<Connection> {
const controller = new AbortController()
const signal = controller.signal

const remotePeerString = ma.getPeerId()
if (remotePeerString === null) {
throw inappropriateMultiaddr("we need to have the remote's PeerId")
Expand Down Expand Up @@ -183,13 +187,32 @@ export class WebRTCDirectTransport implements Transport {
}
}

const eventListeningName = isFirefox ? 'iceconnectionstatechange' : 'connectionstatechange'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally it's better to check if a capability is present or not. But that being said, I'm not sure off the top of my head how I would check for this. So I think it's okay.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, well iceconnectionstatechange event has been supported since Firefox 24 which is about a decade old, and connectionstatechange has been supported for at least two years in all other browsers, so I would say this is low risk.


peerConnection.addEventListener(eventListeningName, () => {
switch (peerConnection.connectionState) {
case 'failed':
case 'disconnected':
case 'closed':
maConn.close().catch((err) => {
maschad marked this conversation as resolved.
Show resolved Hide resolved
log.error('error closing connection', err)
}).finally(() => {
// Remove the event listener once the connection is closed
controller.abort()
})
break
default:
break
}
}, { signal })

// Creating the connection before completion of the noise
// handshake ensures that the stream opening callback is set up
const maConn = new WebRTCMultiaddrConnection({
peerConnection,
remoteAddr: ma,
timeline: {
open: (new Date()).getTime()
open: Date.now()
}
})

Expand Down