feross / simple-peer

📡 Simple WebRTC video, voice, and data channels
MIT License
7.41k stars 970 forks source link

Sending buffers fails over wrtc sometimes #60

Closed paulkernfeld closed 8 years ago

paulkernfeld commented 8 years ago

When I run the unit tests for node.js, they fail:

# data send/receive Buffer
isbuffer true true
ok 24 data is Buffer
not ok 25 got correct message
  ---
    operator: deepEqual
    expected: |-
      <Buffer >
    actual: |-
      <Buffer 76 61 72 20 63 6f 6d 6d 6f 6e 20 3d 20 72 65 71 75 69 72 65 28 27 2e 2f 63 6f 6d 6d 6f 6e 27 29 0a 76 61 72 20 50 65 65 72 20 3d 20 72 65 71 75 69 72 ... >
    at: Peer.<anonymous> (/Users/paul/repos/simple-peer/test/binary.js:71:9)
  ...
isbuffer true true
FATAL ERROR: v8::ArrayBuffer::Externalize ArrayBuffer already externalized
^C

Bizarrely, that Buffer translates to:

var common = require('./common')
var Peer = requir...

I think the reason it's failing is because simple-peer isn't converting Buffers to UInt8Arrays, because isTypedArray returns true for... most Buffers? All Buffers? I'll try to make a pull request that fixes this.

paulkernfeld commented 8 years ago

My pull request is here: feross/simple-peer#61

feross commented 8 years ago

What version of node? Please run node --version. This sounds like a bug in wrtc.

paulkernfeld commented 8 years ago

I'm using node 5.5.0.

Maybe this is just mincing words, but I'm not sure if I would say that this is a bug in wrtc, since it doesn't claim to allow Buffers (see js-platform/node-webrtc#103).

In node 5.5, it looks like Buffers are considered to be typed arrays by is-typedarray. I'm not sure if that's true in other versions of node; I'll check it out.

paul ~/projects/swarmhub master $ node --version
v5.5.0
paul ~/projects/swarmhub master $ node
> require('is-typedarray')(Buffer([1,2]))
true
feross commented 8 years ago

Buffer is a subclass of Uint8Array since iojs v3 and node v4. So, for any Buffer buf, this should be true: buf instanceof Uint8Array. That's why I think it's ultimately a bug in wrtc.

That issue about wrtc not supporting buffers is old, from the time before Buffer was implemented as a subclass of Uint8Array.