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

Bug fix for download interruption promise rejection #396

Merged

Conversation

sonudoo
Copy link

@sonudoo sonudoo commented Jul 7, 2019

Resolving the issue wkh237#264. Current version doesn't throw a promise rejection upon incomplete file download to storage directly, on android. I am providing a fix by comparing the bytes downloaded with the content length.

Tested on android. Throws RNFetchBlob failed. Download interrupted upon interruption.

@jonathangreco
Copy link

This is important.

When my download file has been interrupted (lost connection) the promise is not rejected, no error are catch. So I need this to be merged.

@mifi
Copy link

mifi commented Aug 29, 2019

Same problem here - only Android. Causing very sneaky bugs. What's preventing this from being merged?

@jonathangreco
Copy link

@Traviskn ping :)

@mifi
Copy link

mifi commented Aug 29, 2019

One potential issue i can see is if the server doesn't respond with content length, then what will happen with this code?

@mifi
Copy link

mifi commented Aug 29, 2019

Here's my workaround in JS code:

const res = await RNFetchBlob
  .config({ path: tmpPath })
  .fetch('GET', url);

// TODO hack to remove when https://github.com/joltup/rn-fetch-blob/pull/396/files is merged
if (Platform.OS === 'android') {
  const contentLengthStr = res.info().headers['Content-Length'];
  if (contentLengthStr) {
    const contentLength = parseInt(contentLengthStr, 10);
    if (contentLength) {
      const stat = await RNFetchBlob.fs.stat(tmpPath);
      if (stat.size !== contentLength) throw new Error('Download size was not correct, maybe interrupted');
    }
  }
}

@sonudoo
Copy link
Author

sonudoo commented Aug 31, 2019

I think rn-fetch-blob is no longer being maintained.

@Traviskn
Copy link

Sorry for the delayed review, thank you for this PR!

@Traviskn Traviskn merged commit 4305ef8 into joltup:master Sep 26, 2019
@Traviskn
Copy link

This seems to have caused a regression that breaks Transfer-Encoding: chunked #443

Traviskn added a commit that referenced this pull request Sep 27, 2019
…droid-bugfix"

This reverts commit 4305ef8, reversing
changes made to d35fb73.
@Traviskn
Copy link

I have reverted the merge of this PR. If a new PR can be opened that handles interrupted/incomplete downloads without breaking Transfer-Encoding: chunked I'd be glad to take another look!

@sonudoo
Copy link
Author

sonudoo commented Sep 27, 2019

Sure. I will look into it and raise another PR soon. Thanks.

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

Successfully merging this pull request may close these issues.

4 participants