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

Emit `metadata` before `ready` and `done` #1737

Merged
merged 1 commit into from Sep 6, 2019

Conversation

@alxhotel
Copy link
Member

alxhotel commented Sep 6, 2019

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

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

What changes did you make? (Give an overview)

Regarding events, I think it is important the order in which they are emitted.

If I'm correct, the usual order in which torrent events are emitted should be:

infoHash
metadata
ready
done

But right now, the metadata event is emitted the last one. This PR fixes the order of this event. And maybe fixes webtorrent-desktop#1429. I'm not sure since I was not able to reproduce the bug with the exact same steps. But, at least, the order is now (as I believe) correct.

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

They should focus on staying awesome ;)

@feross
feross approved these changes Sep 6, 2019
Copy link
Member

feross left a comment

This seems like a good change to me. I guess this got moved into the wrong place after the PRs changing the verification process?

Would be nice to add a test for this too, if you have the time.

@feross feross merged commit eeb48ba into webtorrent:master Sep 6, 2019
3 checks passed
3 checks passed
LGTM analysis: JavaScript No new or fixed alerts
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@feross

This comment has been minimized.

Copy link
Member

feross commented Sep 6, 2019

Released as 0.107.8

@feross

This comment has been minimized.

Copy link
Member

feross commented Sep 7, 2019

This PR broke webtorrent-cli (webtorrent/webtorrent-cli#112) because it destroys the torrent in response to the 'metadata' event, so I just added an extra check in ef6879a

Released as 0.107.9

alxhotel added a commit to alxhotel/webtorrent that referenced this pull request Sep 7, 2019
alxhotel added a commit to alxhotel/webtorrent that referenced this pull request Sep 7, 2019
alxhotel added a commit to alxhotel/webtorrent that referenced this pull request Sep 7, 2019
@alxhotel alxhotel deleted the alxhotel:fix-metadata-event branch Sep 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

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