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

Add websocket trackers directly in webtorrent #672

Closed
rom1504 opened this issue Mar 14, 2016 · 11 comments
Closed

Add websocket trackers directly in webtorrent #672

rom1504 opened this issue Mar 14, 2016 · 11 comments

Comments

@rom1504
Copy link
Member

@rom1504 rom1504 commented Mar 14, 2016

websocket trackers are added to existing torrents in all the webtorrent dependencies that support webrtc:

It would probably make sense to put that directly in webtorrent, if there is webrtc support.

Also see that discussion webtorrent/webtorrent-hybrid#22 (comment)

@rom1504

This comment has been minimized.

Copy link
Member Author

@rom1504 rom1504 commented Mar 14, 2016

That global is handled in torrent.js

WebRtc support is available through simple-peer

rom1504 added a commit to rom1504/webtorrent that referenced this issue Mar 14, 2016
@DiegoRBaquero

This comment has been minimized.

Copy link
Member

@DiegoRBaquero DiegoRBaquero commented Mar 14, 2016

WebRTC Support is also available on the client. No need to require simple-peer :)

@rom1504

This comment has been minimized.

Copy link
Member Author

@rom1504 rom1504 commented Mar 14, 2016

@DiegoRBaquero what do you mean by "the client" exactly ? Is .WEBRTC_SUPPORT available anywhere else ?

@DiegoRBaquero

This comment has been minimized.

@rom1504

This comment has been minimized.

Copy link
Member Author

@rom1504 rom1504 commented Mar 14, 2016

Yes but I can't require the index.js from torrent.js, see #673

@DiegoRBaquero

This comment has been minimized.

Copy link
Member

@DiegoRBaquero DiegoRBaquero commented Mar 14, 2016

You don't need to require it, a Torrent has the reference to the client. You can move the code to the constructor.

@rom1504

This comment has been minimized.

Copy link
Member Author

@rom1504 rom1504 commented Mar 14, 2016

Well it's all global anyway, I could put it anywhere. But I think it belongs to torrent.js since that's where it's used.

@DiegoRBaquero

This comment has been minimized.

Copy link
Member

@DiegoRBaquero DiegoRBaquero commented Mar 15, 2016

But, and anyone correct me if I'm wrong, wouldn't it be better inside the constructor after this line? torrent.js#L76

@feross

This comment has been minimized.

Copy link
Member

@feross feross commented Mar 15, 2016

It would probably make sense to put that directly in webtorrent, if there is webrtc support.

It's actually not a very good idea to make the webtorrent library automatically connect to third party servers without user consent. Because automatically adding servers to .torrent files is basically doing that. I actually made a change very recently to remove the behavior of adding trackers by default to added torrents here: bc69413

Instant.io is a file sending service, so it's fine for it to add it's own and other trackers by default.

WebTorrent.app and webtorrent-hybrid should probably stop adding trackers to added files as well (client.add). When the user wants to seed files (client.seed), on the other hand, I think it's fine to use some reasonable defaults when the user doesn't specify their own trackers.

Going forward, it's safest to ensure that magnet links or .torrent files explicitly contain the trackers to be used. client.seed and create-torrent will continue to do the right thing, so this shouldn't be very hard.

@feross feross closed this Mar 15, 2016
@rom1504

This comment has been minimized.

Copy link
Member Author

@rom1504 rom1504 commented Mar 15, 2016

I agree.
However we got to keep in mind that means if you download a (not created with webtorrent) torrent with webtorrent-app, it won't be made available to instant.io.

@lock

This comment has been minimized.

Copy link

@lock lock bot commented May 4, 2018

This thread has been automatically locked because it has not had recent activity. To discuss futher, please open a new issue.

@lock lock bot locked as resolved and limited conversation to collaborators May 4, 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.

3 participants
You can’t perform that action at this time.