Skip to content
This repository was archived by the owner on Mar 10, 2020. It is now read-only.

Can't connect to individual nodes with many instances #84

Closed
victorb opened this issue Oct 24, 2015 · 8 comments
Closed

Can't connect to individual nodes with many instances #84

victorb opened this issue Oct 24, 2015 · 8 comments

Comments

@victorb
Copy link
Contributor

victorb commented Oct 24, 2015

So, I'm trying to have a couple of IPFS machines to mirror some content. So I've setup two boxes to test, and assign them to different variables and I expected that I would be able to call them separately but there is apparently some sharing in between them...

Example:

var ipfsAPI = require('ipfs-api')

var d1 = ipfsAPI('/ip4/127.0.0.1/tcp/5001')
var d2 = ipfsAPI('/ip4/127.0.0.2/tcp/5001')

d1.id(function(err, res) {
    console.log(res.ID) // => id for d2
})
d2.id(function(err, res) {
    console.log(res.ID) // => id for d2
})

The last one is the one being used.

@victorb
Copy link
Contributor Author

victorb commented Oct 24, 2015

Progress so far in figuring out why: Tried inlining config, if there was any troubles there. Nothing.
Second, tried out PR #83, in case there was something weird with the request-api module. Nothing either.
Thirdly, I found what is the problem. It was indeed the request-api module. Apparently, it always uses the default config! There seems to be a mixup between config the API method against IPFS, config the object we configure the connection to the daemon and config that is the default config.

Made a "sample" commit for showing what the problem is: https://github.com/VictorBjelkholm/node-ipfs-api/commit/d2418e9eef36b591de17d3bcea02b6eb0e28d576 Try it with the test code above

@dignifiedquire
Copy link
Contributor

I would suggest changing the api to sth like this to avoid these issues in the future:

// index.js

function API(url) {
  this.config = makeConfig(url)
  this.request = requestApi.bind(null, this.config)
}

API.prototype.id = function () {
  this.request('id')
}

@dignifiedquire
Copy link
Contributor

@victorbjelkholm could you make a PR with a failing test for this please, that would be easier to ensure any fix works as advertised

@dignifiedquire
Copy link
Contributor

@victorbjelkholm created a test and something I thought would fix it, but the test is still failing, take a look: dignifiedquire@c1f1833

@daviddias
Copy link
Contributor

@victorbjelkholm I'm still trying to nail down and reproduce this error, printing the config obj from inside the request-api always gives me the host + port of the disposable daemon (which changes at each run).

config : { 'api-path': '/api/v0/',
  'user-agent': '/node-ipfs-api/2.4.1/',
  host: '127.0.0.1',
  port: '50935' }

It would be very odd if it were always using the default config, because we have been using disposable daemons (in random ports) for a long time. Could you try it again, with the solid-testsbranch and add more tests there?

@dignifiedquire
Copy link
Contributor

@diasdavid the issue is notthat it always uses the default config, the issue is that it shares the config between multiple instances because the config object is in a module. So if you instantiate the first api and then a second one with a different config you end up with the first one using the config of the second because it got overriden.

daviddias added a commit that referenced this issue Oct 25, 2015
@victorb
Copy link
Contributor Author

victorb commented Oct 25, 2015

@diasdavid just tried the solid-tests branch and it's working perfectly! Thanks a lot!
f97c0b3 closes this issue.

@daviddias
Copy link
Contributor

sweet! Glad to know that :D thank you for finding that bug!

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

3 participants