Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
monicanagent opened this issue Mar 17, 2019 · 5 comments
Closed

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

monicanagent opened this issue Mar 17, 2019 · 5 comments
Assignees
Labels
exp/novice Someone with a little familiarity can pick up kind/bug A bug in existing code (including security flaws) P0 Critical: Tackled by core team ASAP status/ready Ready to be worked

Comments

@monicanagent
Copy link

monicanagent commented Mar 17, 2019

  • 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:

  1. Clone js-libp2p trunk to desktop.
  2. In cloned folder run npm install to install all dependencies.
  3. 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)

  1. 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 jacobheun added kind/bug A bug in existing code (including security flaws) exp/novice Someone with a little familiarity can pick up status/ready Ready to be worked P0 Critical: Tackled by core team ASAP labels Mar 18, 2019
@jacobheun
Copy link
Contributor

It looks like this was introduced in #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
Copy link
Member

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
Copy link
Contributor

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
Copy link
Member

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

@jacobheun jacobheun self-assigned this Mar 18, 2019
jacobheun added a commit that referenced this issue Mar 18, 2019
fix: correct transport config check
jacobheun added a commit that referenced this issue Mar 18, 2019
fix: correct transport config check
jacobheun added a commit that referenced this issue Mar 20, 2019
fix: correct transport config check
@jacobheun
Copy link
Contributor

This is no longer an issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exp/novice Someone with a little familiarity can pick up kind/bug A bug in existing code (including security flaws) P0 Critical: Tackled by core team ASAP status/ready Ready to be worked
Projects
None yet
Development

No branches or pull requests

3 participants