Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

kadDHT error #1657

Closed
mccoysc opened this issue Oct 23, 2018 · 11 comments
Closed

kadDHT error #1657

mccoysc opened this issue Oct 23, 2018 · 11 comments
Labels
exp/novice Someone with a little familiarity can pick up kind/bug A bug in existing code (including security flaws) P2 Medium: Good to have, but can wait until someone steps up

Comments

@mccoysc
Copy link

mccoysc commented Oct 23, 2018

in browser jsipfs with version 0.33.rc1 and lower,if i enable the dht, error would be caused for in libp2p-browser.js no kadDHT configed and would cause this error.
the modules.dht should be config to kadDHT.

@daviddias daviddias added the kind/bug A bug in existing code (including security flaws) label Oct 24, 2018
@alanshaw
Copy link
Member

Apologies but I don't understand. Could you please rephrase and include the stack trace for the error?

@daviddias daviddias added the status/ready Ready to be worked label Nov 2, 2018
@LiMoMoMo
Copy link

LiMoMoMo commented Nov 7, 2018

i got the same error 😭 .

  • enable the dht :
var options = {
        EXPERIMENTAL: {
          dht: true,
          pubsub: false
        },
        // libp2p: null,
        ...
      }
var node = new IPFS(options)
  • my error info is :
Uncaught Error: "value" is required
    at Object.exports.process
    at _class._validateWithOptions
    at _class.root.validate
    at _class.root.attempt
    at _class.root.assert
    at module.exports.validate
    at new Node
    at new Node
    at defaultBundle
    at gotConfig

@dsmith3210
Copy link

I'm having the same problem. I reproduced it here, the relevant stack trace seems to be:

Error: "value" is required joi-browser.js:4011
./node_modules/joi-browser/dist/joi-browser.js/</</exports.process
joi-browser.js:4011
_validateWithOptions
joi-browser.js:1700
./node_modules/joi-browser/dist/joi-browser.js/</</internals.root/root.validate
joi-browser.js:4552
./node_modules/joi-browser/dist/joi-browser.js/</</internals.root/root.attempt
joi-browser.js:4580
./node_modules/joi-browser/dist/joi-browser.js/</</internals.root/root.assert
joi-browser.js:4575
./node_modules/libp2p/src/config.js/module.exports.validate
config.js:46
Node
index.js:32
Node
libp2p-browser.js:55
defaultBundle
libp2p.js:63
gotConfig
libp2p.js:72
./node_modules/ipfs-repo/src/config.js/module.exports/get/<
config.js:45
./node_modules/datastore-level/src/index.js/</get/<
index.js:76
./node_modules/levelup/lib/levelup.js/</LevelUP.prototype.get/<
levelup.js:170
./node_modules/encoding-down/index.js/DB.prototype._get/<
index.js:57
./node_modules/level-js/index.js/</Level.prototype._get/req.onsuccess
index.js:125

The root of it seems to be this assert:

  // Ensure dht is correct
  if (options.config.EXPERIMENTAL.dht) {
    Joi.assert(options.modules.dht, ModuleSchema.required())
  }

@alanshaw
Copy link
Member

Thanks @dsmith3210

@jacobheun this error is originating in libp2p - are you able to take a look?

@alanshaw alanshaw added exp/novice Someone with a little familiarity can pick up P2 Medium: Good to have, but can wait until someone steps up labels Nov 21, 2018
@jacobheun
Copy link
Contributor

So the reason this is failing is because the DHT is being enabled, but no DHT is being provided. In the nodejs configuration we set the dht to kademlia, https://github.com/ipfs/js-ipfs/blob/v0.33.1/src/core/runtime/libp2p-nodejs.js#L36. The browser config is not setting the dht, https://github.com/ipfs/js-ipfs/blob/v0.33.1/src/core/runtime/libp2p-browser.js#L18-L35. So when libp2p runs it's config validation it gets unhappy about turning the missing dht on.

This can be enabled by passing it into the libp2p options when creating the IPFS node. However, it's not obvious that this needs to be done. I'd recommend we just update the libp2p-browser config to include the dht, that way enabling it in EXPERIMENTAL will just work.

@alanshaw if that sounds good I can submit a PR.

@vasco-santos
Copy link
Member

yeah, we need to add the DHT in the browser. I am adding it in ipfs/js-ipfs#856, but maybe we can add sooner that this PR is merged

@dsmith3210
Copy link

Thank you for the info, @jacobheun. I added DHT to libp2p-browser.js and I'm up and running now.

One other note was that I had to add an empty dht object in the config section of defaults: otherwise I got a fatal error: TypeError: this._config.dht is undefined; can't access its "enabledDiscovery" property

@vasco-santos
Copy link
Member

Thanks for the feedback @dsmith3210 ! I will fix that, in libp2p we should check if we have the dht config, and if not, use the defaults.

@alanshaw
Copy link
Member

alanshaw commented Dec 4, 2018

Lets wait until #856 is ready before we add it to the browser.

@alanshaw alanshaw closed this as completed Dec 4, 2018
@ghost ghost removed the status/ready Ready to be worked label Dec 4, 2018
@mccoysc
Copy link
Author

mccoysc commented Dec 4, 2018

too much cpu usage nearly 1 cpu core

@vasco-santos
Copy link
Member

@mccoysc can you test that using the following ipfs/js-ipfs#856? It would be really valuable for me

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
exp/novice Someone with a little familiarity can pick up kind/bug A bug in existing code (including security flaws) P2 Medium: Good to have, but can wait until someone steps up
Projects
None yet
Development

No branches or pull requests

7 participants