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

[transport] Document filter function #31

Closed
alanshaw opened this issue Aug 14, 2018 · 6 comments
Closed

[transport] Document filter function #31

alanshaw opened this issue Aug 14, 2018 · 6 comments

Comments

@alanshaw
Copy link
Member

filter filters the list of multiaddrs that this transport is willing to dial to.

It's a required function but is not documented.

@alanshaw alanshaw changed the title Document filter Document filter function Aug 14, 2018
@gitGksgk
Copy link

gitGksgk commented Oct 23, 2018

Hello, has this been fixed? Using js-ipfs won't automatically produce this issue unless npm install wrtc which means using wrtc for webrtc support. AFAIK without this js-ipfs's peer discovery seems not working by default. However, after installing I got a t.filter is not a function as is known. I am using js-ipfs 0.31.7, but in latest release corresponding code in src\http\index.js seemingly unchanged, this error might persists.
here's the corresponding code :

        if (wrtc || electronWebRTC) {
          const using = wrtc ? 'wrtc' : 'electron-webrtc'
          console.log(`Using ${using} for webrtc support`)
          const wstar = new WStar({ wrtc: (wrtc || electronWebRTC) })
          libp2p.modules.transport = [TCP, WS, wstar]
          libp2p.modules.peerDiscovery = [MulticastDNS, Bootstrap, wstar.discovery]
        }

the issue, as alanshaw has pointed out, is because Transport filter not implemented.
tried 0.32.3, the error persists.
step to reproduce:

  1. get js-ipfs, npm install
  2. same directory, npm install wrtc
  3. node src\cli\bin.js daemon
    it will show:
Initializing daemon...
Using wrtc for webrtc support
J:\js-ipfs-update\js-ipfs\node_modules\libp2p\src\index.js:145
      if (t.filter(multiaddrs).length > 0) {
            ^

TypeError: t.filter is not a function
    at _modules.transport.forEach (J:\js-ipfs-update\js-ipfs\node_modules\libp2p\src\index.js:145:13)
    at Array.forEach (<anonymous>)
    at Node.start (J:\js-ipfs-update\js-ipfs\node_modules\libp2p\src\index.js:136:29)
    at gotConfig (J:\js-ipfs-update\js-ipfs\src\core\components\libp2p.js:95:26)
    at store.get (J:\js-ipfs-update\js-ipfs\node_modules\ipfs-repo\src\config.js:45:9)
    at fs.readFile (J:\js-ipfs-update\js-ipfs\node_modules\datastore-fs\src\index.js:218:7)
    at J:\js-ipfs-update\js-ipfs\node_modules\graceful-fs\graceful-fs.js:78:16
    at FSReqWrap.readFileAfterClose [as oncomplete] (fs.js:511:3)

J:\js-ipfs-update\js-ipfs>node src\cli\bin.js version
js-ipfs version: 0.32.3

@gitGksgk
Copy link

gitGksgk commented Oct 23, 2018

emmm.... I've got it wrong. t.filter is implemented.
This issue is because of inconsistent arguement type in transport

const WStar = require('libp2p-webrtc-star')
const TCP = require('libp2p-tcp')
const WS = require('libp2p-websockets')

if (wrtc || electronWebRTC) {
       const using = wrtc ? 'wrtc' : 'electron-webrtc'
       console.log(`Using ${using} for webrtc support`)
       const wstar = new WStar({ wrtc: (wrtc || electronWebRTC) })
       libp2p.modules.transport = [TCP, WS, wstar]
       libp2p.modules.peerDiscovery = [MulticastDNS, Bootstrap, wstar.discovery]
     }

in libp2p.modules.transport = [TCP, WS, wstar] TCP or WS is a class while wstar is an instance of a
class....
changing it to libp2p.modules.transport = [TCP, WS, WStar] resolves the issue, though it may lead to some unexpected behavior...
same thing goes with libp2p.modules.peerDiscovery. but I have no idea on how to formalize wstar.discovery

@jacobheun
Copy link
Contributor

jacobheun commented Oct 24, 2018

On startup libp2p should check if the transport is an instance of a class/function and handle it appropriately.

In addition to documenting .filter tests need to be added so that the transport modules are being adequately checked.

@pynchmeister
Copy link

Am I correct to assume that this is still being fixed?

@jacobheun
Copy link
Contributor

It still needs to be documented and added, yes, but the core transports do have the appropriate support. This would be a nice thing to roll out with the async/await updates to transports. PRs are welcome and appreciated!

@daviddias daviddias changed the title Document filter function [transport] Document filter function Feb 5, 2020
@daviddias daviddias transferred this issue from libp2p/interface-transport Feb 5, 2020
@jacobheun
Copy link
Contributor

Fixed during the migration to async await, https://github.com/libp2p/js-interfaces/tree/master/src/transport#filtering-addresses.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants