PubSub example ends with TypeError on Windows 10 (64-bit)

monicanagent commented 5 years ago

Version: 0.25.0-rc.4
Platform: Node.js for Windows 10 / Ubuntu 16.04 LTS (on Windows 10) - 64bit
Subsystem: pubsub

Type: Bug

Severity: Low

Description: The PubSub example ends with a TypeError:

C:\Users\Patrick\Desktop\libp2p\node_modules\superstruct\lib\index.js:1181
        throw new StructError(error);
        ^

TypeError: Expected a value of type `function | object` but received `undefined`.
    at Function.Struct.assert.value [as assert] (C:\Users\Patrick\Desktop\libp2p\node_modules\superstruct\lib\index.js:1181:15)
    at Struct (C:\Users\Patrick\Desktop\libp2p\node_modules\superstruct\lib\index.js:1165:21)
    at module.exports.validate (C:\Users\Patrick\Desktop\libp2p\src\config.js:73:27)
    at new Node (C:\Users\Patrick\Desktop\libp2p\src\index.js:48:16)
    at new MyBundle (C:\Users\Patrick\Desktop\libp2p\examples\pubsub\1.js:37:5)
    at waterfall (C:\Users\Patrick\Desktop\libp2p\examples\pubsub\1.js:48:14)
    at nextTask (C:\Users\Patrick\Desktop\libp2p\node_modules\async\waterfall.js:16:14)
    at next (C:\Users\Patrick\Desktop\libp2p\node_modules\async\waterfall.js:23:9)
    at C:\Users\Patrick\Desktop\libp2p\node_modules\async\internal\onlyOnce.js:12:16
    at PeerId.create (C:\Users\Patrick\Desktop\libp2p\node_modules\peer-info\src\index.js:47:7)

Steps to reproduce the error:

Clone js-libp2p trunk to desktop.
In cloned folder run npm install to install all dependencies.

Run npm audit fix to attempt to fix 3 low-priority vulnerabilities. Result:

npm WARN acorn-dynamic-import@4.0.0 requires a peer of acorn@^6.0.0 but none is installed. You must install peer dependencies yourself.
npm WARN acorn-jsx@5.0.1 requires a peer of acorn@^6.0.0 but none is installed. You must install peer dependencies yourself.
npm WARN wrtc@0.3.5 requires a peer of domexception@^1.0.1 but none is installed. You must install peer dependencies yourself.
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: fsevents@1.2.7 (node_modules/fsevents):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for fsevents@1.2.7: wanted {"os":"darwin","arch":"any"} (current: {"os":"linux","arch":"x64"})
fixed 0 of 3 vulnerabilities in 44953 scanned packages
3 vulnerabilities required manual review and could not be updated

(I can update this issue with details on the reported vulnerabilities--please let me know if it's relevant)

In {trunk}\examples\pubsub run node 1.js (nodejs 1.js on Ubuntu) -- after a few moments the error is reported and example terminates.

jacobheun commented 5 years ago

It looks like this was introduced in https://github.com/libp2p/js-libp2p/pull/313. The issue with enabling the dht by default is that we don't package a dht module with libp2p. So if the dht is not configured, libp2p errors on startup, which is the case in the pubsub example.

The dht change should probably be reverted. Perhaps an alternative route to enabling it by default would be to add some pre-configured bundles to libp2p with clear peerDependencies that users should add to the package.json file. @vasco-santos thoughts?

vasco-santos commented 5 years ago

I think we should keep it enabled by default, taking into consideration that we want people to start using it, in order to allow the implementation to be exposed to a bigger network.

In this way, I think that we should go with the pre-configured bundles to libp2p. Do you agree?

jacobheun commented 5 years ago

My thought is to revert it for the 0.25 release, so that it doesn't block the js-ipfs release, as I'd prefer not to rush the bundle work. With it being enabled by default in the upcoming js-ipfs release, if we target the bundles and having it on by default for the next libp2p 0.26 release, we'd get a little more time to talk through the bundle options and still get more dht exposure via ipfs.

vasco-santos commented 5 years ago

All right, that's fine for me! We can go that way then :)

jacobheun commented 3 years ago

This is no longer an issue.

libp2p / js-libp2p