peers / peerjs-server

Server for PeerJS
https://peerjs.com
MIT License
4.37k stars 1.09k forks source link

_generateClientId is generating 14 character id #41

Closed floatdrop closed 10 years ago

floatdrop commented 10 years ago

While testing express migration I got this error:

1) PeerServer #_generateClientId should generate a 16-character ID:
     Error: expected 14 to be within 15..16
      at Assertion.assert (/Users/floatdrop/peerjs-server/node_modules/expect.js/index.js:96:13)
      at Assertion.within (/Users/floatdrop/peerjs-server/node_modules/expect.js/index.js:248:10)
      at Function.within (/Users/floatdrop/peerjs-server/node_modules/expect.js/index.js:499:17)
      at Context.<anonymous> (/Users/floatdrop/peerjs-server/test/server.js:202:64)
      at callFn (/Users/floatdrop/peerjs-server/node_modules/mocha/lib/runnable.js:223:21)
      at Test.Runnable.run (/Users/floatdrop/peerjs-server/node_modules/mocha/lib/runnable.js:216:7)
      at Runner.runTest (/Users/floatdrop/peerjs-server/node_modules/mocha/lib/runner.js:374:10)
      at /Users/floatdrop/peerjs-server/node_modules/mocha/lib/runner.js:452:12
      at next (/Users/floatdrop/peerjs-server/node_modules/mocha/lib/runner.js:299:14)
      at /Users/floatdrop/peerjs-server/node_modules/mocha/lib/runner.js:309:7
      at next (/Users/floatdrop/peerjs-server/node_modules/mocha/lib/runner.js:247:23)
      at Object._onImmediate (/Users/floatdrop/peerjs-server/node_modules/mocha/lib/runner.js:276:5)
      at processImmediate [as _immediateCallback] (timers.js:330:15)

On second run it disappears - this function should be tested more strictly.

floatdrop commented 10 years ago

Here is snippet, that can generate id's:

var l = 17;
while (true) {
    var id = Math.random().toString(36).substr(2);
    if (id.length < l) {
        console.log(id.length + ': ' + id);
        l = id.length;
    }
}

I got even id with length '5':

16: bf8d3b86h78vkj4i
14: nc8ah4w2iysyvi
13: 5tkdofn06yldi
12: p0zm3nomvx6r
11: 1dtitn5qaor
8: 65dvlsor
7: 7h3q5mi
6: 63whfr
5: rms4i
michelle commented 10 years ago

Ah, that's embarrassing. We'll fix it.

jure commented 10 years ago

This one bit me too. Can we get an updated npm package, please? :)