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

fix(events): Support of paused client event #411

Merged
merged 3 commits into from
Jun 1, 2022
Merged

fix(events): Support of paused client event #411

merged 3 commits into from
Jun 1, 2022

Conversation

pavloniym
Copy link
Contributor

What is the purpose of this pull request? (put an "X" next to item)

[ ] Documentation update
[x] Bug fix
[x] New feature
[ ] Other, please explain:

What changes did you make? (Give an overview)
Handle paused client request

Which issue (if any) does this pull request address?
Some torrent clients (e.g. Transmission torrent client) receive 'invalid event' on paused request.

Is there anything you'd like reviewers to focus on?
No

@welcome
Copy link

welcome bot commented Feb 5, 2022

🙌 Thanks for opening this pull request! You're awesome.

@pavloniym pavloniym changed the title Added paused client event fix(events): Support of paused client event Feb 6, 2022
@pavloniym
Copy link
Contributor Author

Are the changes okay for merging? @DiegoRBaquero @feross

@DiegoRBaquero
Copy link
Member

is this part of a BEP? why weren't we handling it before?

@github-actions
Copy link

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

@github-actions github-actions bot added the stale label Apr 18, 2022
@DiegoRBaquero
Copy link
Member

@pavloniym ?

@github-actions github-actions bot removed the stale label Apr 19, 2022
@pavloniym
Copy link
Contributor Author

@DiegoRBaquero, so sorry!
Yes, you are completely right!
As I understand, paused event is some kind of custom torrent-client event and is not part of BEP.
So bittorrent-tracker should not support all custom events of all clients, it doesn't make sense...
I am awfully sorry for my misunderstand.

Idea: add special flag to server configuration to ignore such custom events, without error throw

@DiegoRBaquero
Copy link
Member

Where do you see the error thrown? It should only be a warning and ignored

@pavloniym
Copy link
Contributor Author

If I understand you correctly: https://github.com/webtorrent/bittorrent-tracker/blob/master/lib/server/swarm.js#L51

In this place tracker return error as callback on client announce hook.
Transmission provide paused event on torrent download complete, which is not supported (and not in BEP (and I think it is correct)), so show error to user.

I have a private torrent-tracker and more than 10% of users (300+ users) use Transmission. Many of them complaining about this error, I tried to explain them that "this error is normal" but not succeeded :)

To keep server code clean and support only BEP events — my idea is to add special flag to server configuration to ignore such custom events, without error throw

@DiegoRBaquero
Copy link
Member

I see it in BEP21: http://bittorrent.org/beps/bep_0021.html

More ref: https://trac.transmissionbt.com/ticket/5883

I guess we can include it, as long as no official BEP overrides it.

@DiegoRBaquero DiegoRBaquero merged commit ef76b3f into webtorrent:master Jun 1, 2022
@welcome
Copy link

welcome bot commented Jun 1, 2022

🎉 Congrats on getting your first pull request landed!

webtorrent-bot pushed a commit that referenced this pull request Jun 1, 2022
# [9.19.0](v9.18.6...v9.19.0) (2022-06-01)

### Features

* **events:** Support of `paused` client event ([#411](#411)) ([ef76b3f](ef76b3f))
@webtorrent-bot
Copy link

🎉 This PR is included in version 9.19.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants