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

Regression: File.downloaded becomes negative once the first piece is fully downloaded #1515

Closed
corwin-of-amber opened this issue Sep 24, 2018 · 8 comments

Comments

@corwin-of-amber
Copy link
Contributor

@corwin-of-amber corwin-of-amber commented Sep 24, 2018

What version of WebTorrent?
0.102.4

What operating system and Node.js version?
macOS 10.13.6
Node.js 10.0.0

What browser and version? (if using WebTorrent in the browser)

What did you expect to happen?
File.download property should always be a nonnegative integer.

What actually happened?
After the first piece (_startPiece) of the file has finished downloaded, the file's offset is subtracted from the downloaded size, causing it to be a (sometimes quite large) negative.
This occurs in file.js, line 46 (https://github.com/webtorrent/webtorrent/blob/master/lib/file.js#L46), as of revision 6fbef8e.

I noticed the negative values after upgrading from version 0.98.24, which did not have this issue.

@corwin-of-amber

This comment has been minimized.

Copy link
Contributor Author

@corwin-of-amber corwin-of-amber commented Sep 24, 2018

I was able to patch the problem by replacing - this.offset with - (this.offset % pieceLength). I am not 100% sure that this is how pieces work in a torrent so I leave it to your judgement.

@KayleePop

This comment has been minimized.

Copy link
Contributor

@KayleePop KayleePop commented Sep 24, 2018

Is this where it happened? #1479

@corwin-of-amber

This comment has been minimized.

Copy link
Contributor Author

@corwin-of-amber corwin-of-amber commented Sep 25, 2018

@KayleePop Yes, this seems so. I think the author of that push request only considered the case where there is only one large file and that this file starts at the first piece of the torrent. When there are multiple videos, for instance, and the video starts at a high-index piece (e.g. 1000), it does not make sense to initialize the size to pieceLength - this.offset.

@G-Ray

This comment has been minimized.

Copy link

@G-Ray G-Ray commented Dec 9, 2018

I'm affected. @DiegoRBaquero what about you ?

@stale

This comment has been minimized.

Copy link

@stale stale bot commented Mar 9, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale label Mar 9, 2019
@corwin-of-amber

This comment has been minimized.

Copy link
Contributor Author

@corwin-of-amber corwin-of-amber commented Mar 11, 2019

Any news about this? Seems like a pretty simple fix.
It will be so sad if WebTorrent becomes abandoned...

@stale stale bot removed the stale label Mar 11, 2019
@DiegoRBaquero

This comment has been minimized.

Copy link
Member

@DiegoRBaquero DiegoRBaquero commented Mar 11, 2019

I'll review a PR if you a willing to take on this :)

@corwin-of-amber

This comment has been minimized.

Copy link
Contributor Author

@corwin-of-amber corwin-of-amber commented Mar 11, 2019

Sure, you got it.

DiegoRBaquero added a commit that referenced this issue Mar 11, 2019
Fixes #1515, #1552.
@lock lock bot locked as resolved and limited conversation to collaborators Jun 9, 2019
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
4 participants
You can’t perform that action at this time.