Skip to content

Commit fdfb7b4

Browse files
jacobheunjacobheun
authored
fix: not started yet (#297)
* fix: callback when not started rather than throwing asserts * fix: dont remove transports until the switch has stopped * test: update connection check logic * test: fix variable reference * chore: update switch dep * chore: update switch dep
1 parent 15bdb79 commit fdfb7b4

7 files changed

+91
-67
lines changed

‎package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@
5050
"libp2p-connection-manager": "~0.0.2",
5151
"libp2p-floodsub": "~0.15.1",
5252
"libp2p-ping": "~0.8.3",
53-
"libp2p-switch": "~0.41.2",
53+
"libp2p-switch": "~0.41.3",
5454
"libp2p-websockets": "~0.12.0",
5555
"mafmt": "^6.0.2",
5656
"multiaddr": "^5.0.2",

‎src/index.js

+18-13
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@
22

33
const FSM = require('fsm-event')
44
const EventEmitter = require('events').EventEmitter
5-
const assert = require('assert')
65
const debug = require('debug')
76
const log = debug('libp2p')
87
log.error = debug('libp2p:error')
8+
const errCode = require('err-code')
99

1010
const each = require('async/each')
1111
const series = require('async/series')
@@ -24,7 +24,12 @@ const pubsub = require('./pubsub')
2424
const getPeerInfo = require('./get-peer-info')
2525
const validateConfig = require('./config').validate
2626

27-
const NOT_STARTED_ERROR_MESSAGE = 'The libp2p node is not started yet'
27+
const notStarted = (action, state) => {
28+
return errCode(
29+
new Error(`libp2p cannot ${action} when not started; state is ${state}`),
30+
'ERR_NODE_NOT_STARTED'
31+
)
32+
}
2833

2934
/**
3035
* @fires Node#error Emitted when an error occurs
@@ -217,8 +222,6 @@ class Node extends EventEmitter {
217222
* @returns {void}
218223
*/
219224
dial (peer, callback) {
220-
assert(this.isStarted(), NOT_STARTED_ERROR_MESSAGE)
221-
222225
this.dialProtocol(peer, null, callback)
223226
}
224227

@@ -233,7 +236,9 @@ class Node extends EventEmitter {
233236
* @returns {void}
234237
*/
235238
dialProtocol (peer, protocol, callback) {
236-
assert(this.isStarted(), NOT_STARTED_ERROR_MESSAGE)
239+
if (!this.isStarted()) {
240+
return callback(notStarted('dial', this.state._state))
241+
}
237242

238243
if (typeof protocol === 'function') {
239244
callback = protocol
@@ -261,7 +266,9 @@ class Node extends EventEmitter {
261266
* @returns {void}
262267
*/
263268
dialFSM (peer, protocol, callback) {
264-
assert(this.isStarted(), NOT_STARTED_ERROR_MESSAGE)
269+
if (!this.isStarted()) {
270+
return callback(notStarted('dial', this.state._state))
271+
}
265272

266273
if (typeof protocol === 'function') {
267274
callback = protocol
@@ -282,8 +289,6 @@ class Node extends EventEmitter {
282289
}
283290

284291
hangUp (peer, callback) {
285-
assert(this.isStarted(), NOT_STARTED_ERROR_MESSAGE)
286-
287292
this._getPeerInfo(peer, (err, peerInfo) => {
288293
if (err) { return callback(err) }
289294

@@ -293,7 +298,7 @@ class Node extends EventEmitter {
293298

294299
ping (peer, callback) {
295300
if (!this.isStarted()) {
296-
return callback(new Error(NOT_STARTED_ERROR_MESSAGE))
301+
return callback(notStarted('ping', this.state._state))
297302
}
298303

299304
this._getPeerInfo(peer, (err, peerInfo) => {
@@ -463,13 +468,13 @@ class Node extends EventEmitter {
463468
}
464469
cb()
465470
},
466-
(cb) => {
467-
// Ensures idempotency for restarts
468-
this._switch.transport.removeAll(cb)
469-
},
470471
(cb) => {
471472
this.connectionManager.stop()
472473
this._switch.stop(cb)
474+
},
475+
(cb) => {
476+
// Ensures idempotent restarts
477+
this._switch.transport.removeAll(cb)
473478
}
474479
], (err) => {
475480
if (err) {

‎test/fsm.spec.js

+24
Original file line numberDiff line numberDiff line change
@@ -114,5 +114,29 @@ describe('libp2p state machine (fsm)', () => {
114114

115115
node.start()
116116
})
117+
118+
it('should not dial when the node is stopped', (done) => {
119+
node.on('stop', () => {
120+
node.dial(null, (err) => {
121+
expect(err).to.exist()
122+
expect(err.code).to.eql('ERR_NODE_NOT_STARTED')
123+
done()
124+
})
125+
})
126+
127+
node.stop()
128+
})
129+
130+
it('should not dial (fsm) when the node is stopped', (done) => {
131+
node.on('stop', () => {
132+
node.dialFSM(null, null, (err) => {
133+
expect(err).to.exist()
134+
expect(err.code).to.eql('ERR_NODE_NOT_STARTED')
135+
done()
136+
})
137+
})
138+
139+
node.stop()
140+
})
117141
})
118142
})

‎test/stream-muxing.node.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ describe('stream muxing', () => {
215215

216216
nodeA.dial(nodeB.peerInfo, (err) => {
217217
expect(err).to.not.exist()
218-
expect(Object.keys(nodeA._switch.muxedConns)).to.have.length(0)
218+
expect(nodeA._switch.connection.getAll()).to.have.length(0)
219219
cb()
220220
})
221221
},

‎test/transports.browser.js

+14-13
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ describe('transports', () => {
102102
function check () {
103103
const peers = nodeA.peerBook.getAll()
104104
expect(Object.keys(peers)).to.have.length(1)
105-
expect(Object.keys(nodeA._switch.muxedConns)).to.have.length(0)
105+
expect(nodeA._switch.connection.getAll()).to.have.length(0)
106106
done()
107107
}
108108
})
@@ -142,7 +142,7 @@ describe('transports', () => {
142142
const peers = nodeA.peerBook.getAll()
143143
expect(err).to.not.exist()
144144
expect(Object.keys(peers)).to.have.length(1)
145-
expect(Object.keys(nodeA._switch.muxedConns)).to.have.length(0)
145+
expect(nodeA._switch.connection.getAll()).to.have.length(0)
146146
done()
147147
}
148148
})
@@ -153,16 +153,17 @@ describe('transports', () => {
153153
expect(err).to.not.exist()
154154

155155
connFSM.once('muxed', () => {
156-
expect(nodeA._switch.muxedConns).to.have.any.keys(
157-
peerB.id.toB58String()
158-
)
156+
expect(
157+
nodeA._switch.connection.getAllById(peerB.id.toB58String())
158+
).to.have.length(1)
159159

160160
connFSM.once('error', done)
161161
connFSM.once('close', () => {
162162
// ensure the connection is closed
163-
expect(nodeA._switch.muxedConns).to.not.have.any.keys([
164-
peerB.id.toB58String()
165-
])
163+
expect(
164+
nodeA._switch.connection.getAllById(peerB.id.toB58String())
165+
).to.have.length(0)
166+
166167
done()
167168
})
168169

@@ -312,7 +313,7 @@ describe('transports', () => {
312313
function check () {
313314
const peers = node1.peerBook.getAll()
314315
expect(Object.keys(peers)).to.have.length(1)
315-
expect(Object.keys(node1._switch.muxedConns)).to.have.length(0)
316+
expect(node1._switch.connection.getAll()).to.have.length(0)
316317
done()
317318
}
318319
})
@@ -326,7 +327,7 @@ describe('transports', () => {
326327

327328
function check () {
328329
// Verify both nodes are connected to node 3
329-
if (node1._switch.muxedConns[b58Id] && node2._switch.muxedConns[b58Id]) {
330+
if (node1._switch.connection.getAllById(b58Id) && node2._switch.connection.getAllById(b58Id)) {
330331
done()
331332
}
332333
}
@@ -417,7 +418,7 @@ describe('transports', () => {
417418
function check () {
418419
const peers = node1.peerBook.getAll()
419420
expect(Object.keys(peers)).to.have.length(1)
420-
expect(Object.keys(node1._switch.muxedConns)).to.have.length(0)
421+
expect(node1._switch.connection.getAll()).to.have.length(0)
421422
done()
422423
}
423424
})
@@ -430,8 +431,8 @@ describe('transports', () => {
430431

431432
function check () {
432433
if (++counter === 3) {
433-
expect(Object.keys(node1._switch.muxedConns).length).to.equal(1)
434-
expect(Object.keys(node2._switch.muxedConns).length).to.equal(1)
434+
expect(node1._switch.connection.getAll()).to.have.length(1)
435+
expect(node2._switch.connection.getAll()).to.have.length(1)
435436
done()
436437
}
437438
}

0 commit comments

Comments
 (0)