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

error handling w/ duplicate torrents #348

Closed
madd512 opened this issue Jun 7, 2015 · 4 comments
Closed

error handling w/ duplicate torrents #348

madd512 opened this issue Jun 7, 2015 · 4 comments

Comments

@madd512
Copy link

@madd512 madd512 commented Jun 7, 2015

var WebTorrent = require('webtorrent-hybrid');

var client = new WebTorrent();

client.seed('./www', torrent => {
  console.log('seed worked');
  try {
    client.add(torrent.infoHash, () => console.log('add worked'))
  } catch (err) {
    console.error('got error', err)
  }
})

Expected output:

$ node test.js 
seed worked
add worked

Or, less preferably:

$ node test.js 
seed worked
got error [Error: There is already a swarm with info hash 55f65714bb41dcdf19fd3ccceb89fdfded970de2 listening on port 48839]

Actual output:

$ node test.js 
seed worked
events.js:85
      throw er; // Unhandled 'error' event
            ^
Error: There is already a swarm with info hash ... listening on port 48839
    at /.../node_modules/webtorrent-hybrid/node_modules/webtorrent/node_modules/bittorrent-swarm/lib/tcp-pool.js:101:22
    at process._tickCallback (node.js:355:11)

Might it be worth adding an err argument to the callbacks, or just returning an existing torrent when it's already been added/seeded?

@feross

This comment has been minimized.

Copy link
Member

@feross feross commented Jun 7, 2015

You don't need to manually add the torrent to the client when you're seeding - it's handled for you. client.seed actually calls client.add internally :-)

Can't add an err argument to the callback because it's actually just a shorthand for listening to the 'torrent' event. Events don't have err arguments.

I like the idea of returning the existing torrent instead of failing with an error. Let's do that.

@madd512

This comment has been minimized.

Copy link
Author

@madd512 madd512 commented Jun 7, 2015

I like the idea of returning the existing torrent instead of failing with an error. Let's do that.

I was hoping you'd say that :)

I'm looking through the code. The error is thrown here, in bittorrent-swarm. I believe it can be caught and handled here.

I could intercept this error and emit the torrent event again, but that may cause unexpected behavior for anybody expecting that event to be emitted once per torrent, and that would sort of leak behavior out in a weird way which I don't like.

Instead, I think I'll iterate through self.torrents, running clientOnTorrent on each one, assuming ontorrent is has been defined. Since clientOnTorrent only runs ontorrent when the infoHash matches, I think that should do it... maybe...

madd512 pushed a commit to madd512/webtorrent that referenced this issue Jun 7, 2015
@madd512

This comment has been minimized.

Copy link
Author

@madd512 madd512 commented Jun 7, 2015

PR here #351

@feross

This comment has been minimized.

Copy link
Member

@feross feross commented Jun 11, 2015

Released as 0.49.0.

@feross feross closed this Jun 11, 2015
feross added a commit that referenced this issue Jun 11, 2015
feross added a commit that referenced this issue Jun 11, 2015
Return the existing torrent instead of failing with an error.

Fixes #348
@lock lock bot locked as resolved and limited conversation to collaborators May 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.