birdofpreyru / react-native-fs

File system access for React Native
https://dr.pogodin.studio/docs/react-native-file-system
Other
121 stars 8 forks source link

`downloadFile()`: debug `readTimeout` option handling, and add related documentation #33

Open jobpaardekooper opened 3 months ago

jobpaardekooper commented 3 months ago

The DownloadFileOptionsT includes the option readTimeout. It is a number but not much further information is given about it. Is it in milliseconds, seconds or minutes? From taking a quick look at the code it seems to be milliseconds? Nothing really happens when I set it and make the download hang by turning of the server half way through.

What happens when the timeout is reached? Should it throw an error? Or something else? What exactly would trigger this timeout. Would it be triggered when no bytes are read from the connection for x amount of time?

birdofpreyru commented 3 months ago

Hi @jobpaardekooper , I have no idea — it was inherited from the original RNFS code, which does not give much details on it in its README: image I haven't really looked / tested yet what it does, whether it works, and whether it haven't been broken by my code updates.

jobpaardekooper commented 3 months ago

That is what I had guessed. Thanks for the reply!

birdofpreyru commented 3 months ago

@jobpaardekooper I had a very brief look at the code. I see on Android readTimeout (if its value isn't lost somewhere on the way) is just set to the created connection here, and it is supposed to work as per Android's URLConnection.setReadTimeout(int) docs.

Beyond that, I think, it should be debugged what happens there. I imagine, if the timeout is hit, an exception will be thrown at some point in that download() method, but its body is wrapped with just try / finally, without catch clause... just looking at it, I am not sure it will be handled correctly. Perhaps, it just ends up calling this hook as if the download has completed successfully, and that reports it to TS side as success.

So, if you have interest in it, and time to invest, perhaps you can just run your test project from Android Studio, in Debug mode, setting a bunch of breakpoints in the methods I referred to above, and check what happens there? My current guess, the code should be updated to correctly message thrown exceptions to the TS side.

jobpaardekooper commented 3 months ago

Thanks a lot for digging a bit deeper. I don't have much time to debug it right now.

Also, for now I have switched back to the original RNFS. I know this library will be the future and would like to help improve it once I find some more time. But currently issue #24 is really preventing me from using this. My app requires the user to select one out of a few files to download before they can start using the app. It seems that this bug is also effecting me and it is especially prevalent on first launch. It completely brakes my apps usability sadly so I have gone back for now. I know its not sustainable though.

Thanks for all the effort and I will likely be back using this when that gets fixed or I have more time to investigate that issue.