joltup / rn-fetch-blob

A project committed to making file access and data transfer easier, efficient for React Native developers.
MIT License
2.8k stars 770 forks source link

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

Closed ghost closed 5 years ago

ghost commented 5 years ago

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 commented 5 years ago

I found in old repo someone has the same problem (https://github.com/wkh237/react-native-fetch-blob/issues/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 commented 5 years ago

Here is another same problem in the old repo (https://github.com/wkh237/react-native-fetch-blob/issues/264) . Seems that this not fixed yet in the new repo. =(

ghost commented 5 years ago

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.

JofBigHealth commented 5 years ago

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

schumannd commented 5 years ago

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

JofBigHealth commented 5 years ago

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 commented 5 years ago

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 commented 4 years ago

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 commented 4 years ago

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 sizeis 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 commented 4 years ago

@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: https://github.com/joltup/rn-fetch-blob/pull/449 Looking good so far

olegmilan commented 4 years ago

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 commented 4 years ago

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