Skip to content
Closed
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
2 changes: 2 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,5 @@ node_js:
- "node"
- "4"
- "0.12"
before_script:
- export DISPLAY=:99.0; sh -e /etc/init.d/xvfb start
34 changes: 26 additions & 8 deletions lib/client/websocket-tracker.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,18 @@ WebSocketTracker.prototype.destroy = function (cb) {
if (self.destroyed) return cb(null)
self.destroyed = true
clearInterval(self.interval)
clearTimeout(self.reconnectTimer)
Copy link
Member

Choose a reason for hiding this comment

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

This is a good idea!


delete socketPool[self.announceUrl]
// Destroy peers
for (var peerId in self.peers) {
var peer = self.peers[peerId]
clearTimeout(peer.trackerTimeout)
peer.destroy()
}
delete self.peers
Copy link
Member

Choose a reason for hiding this comment

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

self.peers = null is better. delete is for removing a key from a plain object. self is an actual instance of a class, so delete is a worse choice. It will still work but it causes a de-optimization in most JS engines.


// Close socked
Copy link
Member

Choose a reason for hiding this comment

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

typo?

if (socketPool[self.announceUrl]) socketPool[self.announceUrl].consumers--
Copy link
Member

Choose a reason for hiding this comment

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

I prefer -= 1 to --


self.socket.removeListener('connect', self._onSocketConnectBound)
self.socket.removeListener('data', self._onSocketDataBound)
Expand All @@ -100,11 +110,16 @@ WebSocketTracker.prototype.destroy = function (cb) {
self._onSocketDataBound = null
self._onSocketCloseBound = null

self.socket.on('error', noop) // ignore all future errors
try {
self.socket.destroy(cb)
} catch (err) {
cb(null)
if (socketPool[self.announceUrl].consumers === 0) {
delete socketPool[self.announceUrl]

self.socket.on('error', noop) // ignore all future errors

try {
self.socket.destroy(cb)
} catch (err) {
if (cb) cb()
Copy link
Member

Choose a reason for hiding this comment

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

cb is always defined, so you can get rid of the if statement. Also, cb(null) to be explicit.

}
}

self.socket = null
Expand All @@ -122,7 +137,10 @@ WebSocketTracker.prototype._openSocket = function () {
self.socket = socketPool[self.announceUrl]
if (!self.socket) {
self.socket = socketPool[self.announceUrl] = new Socket(self.announceUrl)
self.socket.consumers = 1
self.socket.on('connect', self._onSocketConnectBound)
} else {
socketPool[self.announceUrl].consumers++
Copy link
Member

Choose a reason for hiding this comment

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

Reverse the conditional. Now that there's another case, it's best to remove ! to avoid a double negative.

}

self.socket.on('data', self._onSocketDataBound)
Expand Down Expand Up @@ -291,11 +309,11 @@ WebSocketTracker.prototype._startReconnectTimer = function () {
var ms = Math.floor(Math.random() * RECONNECT_VARIANCE) + Math.min(Math.pow(2, self.retries) * RECONNECT_MINIMUM, RECONNECT_MAXIMUM)

self.reconnecting = true
var reconnectTimer = setTimeout(function () {
self.reconnectTimer = setTimeout(function () {
Copy link
Member

Choose a reason for hiding this comment

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

Add self.reconnectTimer = null to the constructor, as a form of "documentation".

self.retries++
self._openSocket()
}, ms)
if (reconnectTimer.unref) reconnectTimer.unref()
if (self.reconnectTimer.unref) self.reconnectTimer.unref()

debug('reconnecting socket in %s ms', ms)
}
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
"xtend": "^4.0.0"
},
"devDependencies": {
"electron-webrtc": "^0.1.0",
"magnet-uri": "^5.0.0",
"parse-torrent": "^5.0.0",
"standard": "^6.0.4",
Expand Down
1 change: 1 addition & 0 deletions server.js
Original file line number Diff line number Diff line change
Expand Up @@ -599,6 +599,7 @@ Server.prototype._onWebSocketClose = function (socket) {
var swarm = self.torrents[infoHash]
if (swarm) {
swarm.announce({
type: 'ws',
Copy link
Member

Choose a reason for hiding this comment

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

What is this for?

event: 'stopped',
numwant: 0,
peer_id: socket.peerId
Expand Down
43 changes: 32 additions & 11 deletions test/server.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
var Client = require('../')
var common = require('./common')
var test = require('tape')
var wrtc

var infoHash = '4cb67059ed6bd08362da625b3ae77f6f4a075705'
var peerId = new Buffer('01234567890123456789')
Expand All @@ -25,7 +26,7 @@ function serverTest (t, serverType, serverFamily) {
infoHash: infoHash,
length: torrentLength,
announce: [ announceUrl ]
})
}, { wrtc: wrtc })

client1.start()

Expand All @@ -45,14 +46,19 @@ function serverTest (t, serverType, serverFamily) {
t.equal(swarm.complete, 0)
t.equal(swarm.incomplete, 1)
t.equal(Object.keys(swarm.peers).length, 1)
t.deepEqual(swarm.peers[hostname + ':6881'], {
type: serverType,
ip: clientIp,
port: 6881,
peerId: peerId.toString('hex'),
complete: false,
socket: undefined
})

if (serverType !== 'ws') {
t.deepEqual(swarm.peers[hostname + ':6881'], {
type: serverType,
ip: clientIp,
port: 6881,
peerId: peerId.toString('hex'),
complete: false,
socket: undefined
})
} else {
t.equal(swarm.peers[peerId.toString('hex')].complete, false)
Copy link
Member

Choose a reason for hiding this comment

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

Why not test the rest of the properties on the swarm?

}

client1.complete()

Expand All @@ -73,7 +79,7 @@ function serverTest (t, serverType, serverFamily) {
infoHash: infoHash,
length: torrentLength,
announce: [ announceUrl ]
})
}, { wrtc: wrtc })

client2.start()

Expand All @@ -82,7 +88,7 @@ function serverTest (t, serverType, serverFamily) {
})

client2.once('peer', function (addr) {
t.ok(addr === hostname + ':6881' || addr === hostname + ':6882')
t.ok(addr === hostname + ':6881' || addr === hostname + ':6882' || addr.id === peerId.toString('hex'))

client2.stop()
client2.once('update', function (data) {
Expand All @@ -109,6 +115,21 @@ function serverTest (t, serverType, serverFamily) {
})
}

test('create daemon', function (t) {
wrtc = require('electron-webrtc')()
wrtc.electronDaemon.once('ready', t.end)
})

test('websocket server', function (t) {
serverTest(t, 'ws', 'inet')
})

// cleanup
test('cleanup electron-eval daemon', function (t) {
wrtc.close()
t.end()
})

Copy link
Member

Choose a reason for hiding this comment

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

Would prefer to merge these into one test block. It's confusing to have state shared across blocks.

test('http ipv4 server', function (t) {
serverTest(t, 'http', 'inet')
})
Expand Down