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

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

Open
jobpaardekooper opened this issue Apr 3, 2024 · 4 comments
Labels
On Hold Blocked for some reason. P2 Important issue.

Comments

@jobpaardekooper
Copy link

jobpaardekooper commented Apr 3, 2024

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
Copy link
Owner

birdofpreyru commented Apr 3, 2024

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.

@birdofpreyru birdofpreyru added the P2 Important issue. label Apr 3, 2024
@jobpaardekooper
Copy link
Author

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

@birdofpreyru
Copy link
Owner

birdofpreyru commented Apr 5, 2024

@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
Copy link
Author

jobpaardekooper commented Apr 5, 2024

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.

@birdofpreyru birdofpreyru added the On Hold Blocked for some reason. label Apr 5, 2024
@birdofpreyru birdofpreyru changed the title Please add some details to the documentation on readTimeout downloadFile(): debug readTimeout option handling, and add related documentation Apr 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
On Hold Blocked for some reason. P2 Important issue.
Projects
None yet
Development

No branches or pull requests

2 participants