Expose swarm object on tracker server #218

yciabaud · 2017-03-09T10:39:39Z

This PR allows a server tracker to intercept announce request and responses. I use it to provide extra data to peers.

I don't expect this PR to be reviewed either, I post this in case someone is interested.
Tell me if you don't want PRs like that anymore...

I tried to clarify this in #207 but I did not have any clue after 2 weeks.

feross

Thanks for the PR. I'm not sure this is something that we need to support.

Can you explain the reason that you're providing extra data to peers, beyond what is required in the bittorrent tracker spec? What is your use case?

Also, why is getParams even necessary? If you're trying to provide custom response data, this does not seem necessary.

feross · 2017-03-09T18:52:32Z

lib/client/websocket-tracker.js

-      incomplete: data.incomplete
-    })
+    data.announce = self.announceUrl
+    self.client.emit('update', data)


This change makes the different tracker types behave inconsistently. See https://github.com/feross/bittorrent-tracker/blob/f5a32ff13d99a30a41315bc182297a7a8dfa318d/lib/client/http-tracker.js#L193 for example.

I'm not a fan of this change, as it exposes data.info_hash (and other properties?) which don't match the camelCase naming pattern of everything else. What other properties are on data that are now being exposed?

feross · 2017-03-09T18:53:56Z

server.js

+  if (!self._reqHandler.getParams) {
+    self._reqHandler.getParams = function (params) {
+      return params
+    }


I'm concerned about the performance impact of calling a function that closes over a bunch of variables every time, when 99% of the time, this option will go unused.

It would be better to just check if it exists before calling it.

feross · 2017-03-09T18:54:39Z

server.js

+  if (!self._reqHandler.getResponse) {
+    self._reqHandler.getResponse = function (params, cb) {
+      return cb
+    }


This function seems awkward. Why is it called getResponse if it returns a callback function and not a response?

feross · 2017-03-09T18:55:31Z

server.js

  self.udp6 = null
  self.ws = null

+  self._reqHandler = opts.requestHandler || {}


These two options are separate and ought to be exposed separately on the options object, not bundled into one object IMO

feross · 2017-03-09T19:06:19Z

test/request-handler.js

@@ -0,0 +1,68 @@
+var Buffer = require('safe-buffer').Buffer


Thanks for adding tests.

yciabaud · 2017-03-09T19:53:49Z

Thank you @feross for the review, you right this might be bad designed and this is working only with websockets.

In my context, I have some peers that I want to seed their files via HTTP to avoid some WebRTC issues (WebRTC not supported, resources ...). I need the other peers to add them as webseeds and not as webrtc peers.

To achieve this, I add an http port number to the announce fields in the client and my tracker must send the data back to the swarm.

I am using this interceptor to push the webseed urls in the swarm on the ws channels, and my client listens to the response to add the urls as webseeds.

How do you think this could be handled? Those are peers too so I believe I should track them in the torrent swarm but I cannot make it fit the bittorrent protocol (just like ws peers have different fields).
One bittorrent solution would be to make updatable torrents BEP0039 and spread webseeds in it but I am not sure this would be efficient.

Maybe we could support webseeds peers like WebRTC peers because it can make sense in the browser, but I feel like it should be implemented as an extension in the websocket server.

What do you think ? How should I handle this? Should bittorrent-tracker allow users to extends the websocket protocol ? Is this webseed tracking thing worth to be added to webtorrent ? Should I work on BEP0039 ?

feross · 2017-03-09T22:54:43Z

Thanks for explaining. I'm not sure I understand 100% why this is necessary, so let me ask some additional questions:

Are you building an app where some of your peers can actually run HTTP servers and act as web seeds in the p2p network? I assume those peers are running in Node.js. Is this correct?
Is it possible to figure out which peers will be web seeds in advance? Because if it is possible, then you could simply embed the webseed URLs in the magnet link or torrent file. Then, no changes to the tracker server are necessary. This is the easiest way to use webseeds.
If you require the ability for a peer to dynamically add themselves to this "webseed list", then this more complicated. Consider: Is it necessary to implement this as an extension to the tracker protocol? If you only intend to use this feature for your own app, then it could be implemented any way you like outside of the tracker server. For example, you could add an HTTP endpoint GET /webseeds that would return a list of webseed URLs. And peers could POST /webseeds to add themselves to the "webseed list".

It's still possible to change the tracker to support these options, but I want to make sure you've considered these other options, which seem easier and cleaner to me.

yciabaud · 2017-03-10T15:07:47Z

Are you building an app where some of your peers can actually run HTTP servers and act as web seeds in the p2p network? I assume those peers are running in Node.js. Is this correct?

You're right, I do have a Node.js client running a webserver and acting as seeds.

Is it possible to figure out which peers will be web seeds in advance? Because if it is possible, then you could simply embed the webseed URLs in the magnet link or torrent file. Then, no changes to the tracker server are necessary. This is the easiest way to use webseeds.

Those peers are not running on servers, which means some files can be deleted or the peers can suddenly go offline. I would like to add/remove new peers dynamically.

If you require the ability for a peer to dynamically add themselves to this "webseed list", then this more complicated. Consider: Is it necessary to implement this as an extension to the tracker protocol? If you only intend to use this feature for your own app, then it could be implemented any way you like outside of the tracker server. For example, you could add an HTTP endpoint GET /webseeds that would return a list of webseed URLs. And peers could POST /webseeds to add themselves to the "webseed list".

I don't know if this is has to be an extension to the protocol, that is why my PR only tries to let an app using bittorent-tracker like mine to extends the websocket protocol for its personal needs.

My point is that I have an app communicating with clients for tracking purpose using websockets and I would like to use this connection to provide extra tracking features.
Do you think we can propose another way to allow server users to communicate with their peers?
Otherwise I guess I can find the websocket in the swarm and push the messages on my own...

feross · 2017-03-10T21:42:01Z

I think that we already have an official way to support "intercepting" announces, though it's not documented. We added it back when BitTorrent, Inc. wanted to use this library as the tracker for BitTorrent Bundles.

Basically, Server.prototype.createSwarm and Server.prototype.getSwarm are designed to be overwritten, so you can specify your own Swarm implementation. With ES6 classes, it's even easier to just extend the Swarm class and leave createSwarm and getSwarm untouched.

So, you could just overwrite the announce() method. If we expose the Swarm implementation on Server.Swarm, you could do:

class Swarm extends Server.Swarm {
  announce (params, cb) {
    super.announce(params, function (err, response) {
      if (err) return cb(response)
      response.complete = 246
      response.extraData = 'hi'
      cb(null, response)
    })
  }
}

Server.Swarm = Swarm

var myServer = new Server(/* ... */)

It's a bit lower-level, but I think this works. I'll send a PR to update this PR with these changes.

feross · 2017-03-10T21:46:30Z

PR to this PR is here: yciabaud#1

Let me know if it looks good. Once that's merged, then this PR will be updated.

Replace the request handler option by exposing the Swarm

yciabaud · 2017-03-12T08:14:11Z

This is a cleaner way to intercept requests, we need to overwrite things but I believe it is acceptable in the context of extending the protocol for personal needs

Thank you for taking the time to help me out with this. It is really appreciated.

feross

Awesome, glad we got this figured out!

feross · 2017-03-13T22:22:05Z

Released as 9.1.0.

yciabaud added 3 commits March 9, 2017 11:19

Extension point on requests and responses

146d4d4

Remove debug log

1efb832

Add extra data on ws annouce event

8482c3a

feross reviewed Mar 9, 2017

View reviewed changes

feross added 5 commits March 10, 2017 13:38

do not filter out extra keys from 'update' events

f65983e

remove requestHandler option

ee9ebc5

expose Swarm object for easy overriding

992cd77

test: remove UDP request handler test -- not possible

51b57f6

test: change request handler test to new approach

0c683df

Merge pull request #1 from feross/feross/request-handler-changes

a77873a

Replace the request handler option by exposing the Swarm

yciabaud changed the title ~~Announce interceptor on tracker~~ Expose swarm object on tracker server Mar 12, 2017

feross approved these changes Mar 13, 2017

View reviewed changes

feross merged commit bb0a278 into webtorrent:master Mar 13, 2017

yciabaud deleted the request-handler3 branch March 13, 2017 22:53

Uh oh!

Expose swarm object on tracker server #218

Expose swarm object on tracker server #218

Uh oh!

Conversation

yciabaud commented Mar 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

feross left a comment

Choose a reason for hiding this comment

Uh oh!

feross Mar 9, 2017

Choose a reason for hiding this comment

Uh oh!

feross Mar 9, 2017

Choose a reason for hiding this comment

Uh oh!

feross Mar 9, 2017

Choose a reason for hiding this comment

Uh oh!

feross Mar 9, 2017

Choose a reason for hiding this comment

Uh oh!

feross Mar 9, 2017

Choose a reason for hiding this comment

Uh oh!

yciabaud commented Mar 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

feross commented Mar 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yciabaud commented Mar 10, 2017

Uh oh!

feross commented Mar 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

feross commented Mar 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yciabaud commented Mar 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

feross left a comment

Choose a reason for hiding this comment

Uh oh!

feross commented Mar 13, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

yciabaud commented Mar 9, 2017 •

edited

Loading

yciabaud commented Mar 9, 2017 •

edited

Loading

feross commented Mar 9, 2017 •

edited

Loading

feross commented Mar 10, 2017 •

edited

Loading

feross commented Mar 10, 2017 •

edited

Loading

yciabaud commented Mar 12, 2017 •

edited

Loading