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

Download large file with a weak network connection: resolves with status 200 when the file is not fully loaded #336

Closed
ghost opened this issue Mar 28, 2019 · 12 comments

Comments

@ghost
Copy link

ghost commented Mar 28, 2019

Versions

"react": "16.6.1",
"react-native": "0.57.5",
"rn-fetch-blob": "^0.10.13"

Problem
I download the file to the device. When the Internet connection is weak or the connection is broken, the file download stops. But instead of an error, RNFetchBlob's promise successfully resolves with 200 status code and has the path to the broken file.

Actual Behavior: RNFetchBlob's promise successfully resolves with 200 status code when file was not fully downloaded

Expected Behavior: If the connection is broken, RNFetchBlob's promise resolves with an error and I can catch the error in the .catch block.

Code

      return new Promise((resolve, reject) => {
            RNFetchBlob
                .config({
                    // add this option that makes response data to be stored as a file,
                    // this is much more performant.
                    fileCache : true,
                    session : 'init',
                    path : path,
                    trusty : true
                })
                .fetch('get', url, {})
                .progress((received, total) => {
                    if (OfflineMapDownloadService.isDBDownloading) {
                        this.progressCallback(received/total);
                    }
                })
                .then((res) => {
                    let status = res.info().status;
                    if(status === 200) {
                        resolve(res.path());
                    } else {
                        console.log(res);
                        reject(new Error('Invalid RN-blob status ' + status));
                    }
                })
                .catch((errorMessage, statusCode) => {
                    console.log(errorMessage);
                    Sentry.captureException(new Error(errorMessage));
                    reject(errorMessage);
                });
        });
@ghost
Copy link
Author

ghost commented Mar 28, 2019

I found in old repo someone has the same problem (wkh237#673). File was not completely downloaded. but promise resolves with status 200. So main question: how we can check if file was completely downloaded?

@ghost
Copy link
Author

ghost commented Mar 28, 2019

Here is another same problem in the old repo (wkh237#264) . Seems that this not fixed yet in the new repo. =(

@ghost
Copy link
Author

ghost commented Mar 28, 2019

For now I solved this issue by checking the size of the downloaded file and comparing it to the total value from the progress callback. But it is so strange, that in case of bad network connection rn-fetch-blob resolves successfully, and not with the error.

@ghost ghost closed this as completed Mar 31, 2019
@JofBigHealth
Copy link

@NelGarbuzova We are about to implement the same approach too. I also think that's the only way. Thanks for posting your solution. 👍

@schumannd
Copy link

I would suggest you keep the issue open, as it is not properly solved.

@JofBigHealth
Copy link

Note: if I understand @NelGarbuzova's approach correctly it won't always work. The Java code does not always call the .progress callback when the download has completed so the inferring percent complete from the total bytes and downloaded bytes values given to that callback will lead to false negatives. In the end we did a post install patch that replaced RNFetchBlobProgressConfig's shouldReport to always return true. This means the progress callback is called excessively but solves the problem for now. If I get chance I'll do a PR to fix shouldReport to call at least once when 100% complete. The problem is that the ObjC code will need to be fixed too and I'm not so hot on that.

@JofBigHealth
Copy link

JofBigHealth commented Apr 24, 2019

Note also that progress won't always finish with totalBytes === downloadedBytes with iOS either. However, iOS doesn't seem to have this exact problem when attempted to resume; if the connection fails to resume the native (OS) code will timeout and RBFB will report that error.

In conclusion; I don't recommend comparing downloaded to total bytes for either platform without fixing the native code.

@olegmilan
Copy link

olegmilan commented Oct 8, 2019

Any updates on this issue?
I'm still seeing this issue on both platforms (Android and IOS).
To reproduce it, I just need to turn off wifi/4g during download and it can be a small file(not necessary a big file) like image with ~1mb size

      "version": "0.10.16",
      "resolved": "https://registry.npmjs.org/rn-fetch-blob/-/rn-fetch-blob-0.10.16.tgz",
      "integrity": "sha512-hZV+nF0HK4CWmspXGMw7/G8Q8qugpS/wbKiNLsFpdBZR8XYzjFZNvBWgGyC0F5JWQn3sjmK2w/FJjBlwdQWNQg==",
      "requires": {
        "base-64": "0.1.0",
        "glob": "7.0.6"
      },

@skoob13
Copy link

skoob13 commented Oct 8, 2019

I have the similar issue on 0.10.15. On iOS (both simulator and real device) a download promise rejects when device goes to offline. However, there is a different situation on Android: a promise resolves right after a phone goes to offline.

I solved the issue as described by @NeliHarbuzava, but with small improvements. I save the amount of bytes on the first progress callback, then when a promise resolves I check for amount of actual downloaded bytes of cached file:

const { path, size } = await RNFetchBlob.fs.stat(response.path());

So if the size is the same value as total from progress callback, I assume that a download was successful and move the file to other location (we use custom filenames internally). I had tested on 600+ real Android devices for a few days and it looks like it works good.

Nevertheless, this issue has taken too much time to identify and it should be reopened definitely.

@schumannd
Copy link

@skoob13 I agree. IIRC a PR was merged recently that was supposed to fix this but broke something else.

Actually it looks liek a PR was merged 3 days ago which is supposed to fix this:
#449
Looking good so far

@olegmilan
Copy link

Thank you @skoob13, that's exact approach that I was going to implement in order to overcome this issue

Perfect @schumannd, thank you, I hope this will solve those issues

@olegmilan
Copy link

For now I solved this issue by checking the size of the downloaded file and comparing it to the total value from the progress callback. But it is so strange, that in case of bad network connection rn-fetch-blob resolves successfully, and not with the error.

Btw, you can check my solution, that I described in #464 where I retrieve Content Length from res.info().headers["Content-Length"]

Note:

Also I noticed that .progress callback is not always being called for instance when file is small and you will get Content-length: 0 in this case

This issue was closed.
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

No branches or pull requests

4 participants