Make read timeout configurable

[kitlith]

Related to #419.

Alternately, we could try setting a timeout for the entire connection rather than "going 10 seconds without recieving any data", or provide more timeout options, etc. This is just my first stab at it.

Also not quite sure about the wording I used for the new setting, so feel free to have me tweak that.

I've tested to ensure it works in the Android Emulator by connecting to a server that will never respond (nc -l 8000, add feed at http://<host-ip>:8000 by adding another random feed and editing the URL)

(btw this is my first contribution to an Android App, so i recommend you make sure I didn't miss something)

[FredJul]

Hi, thank you for your PR. However, I am a bit skeptical about it at this moment. Flym already has quite a lot of options, and I'm not sure a new one for that is really necessary. What use case are you trying to solve with that ? Poor connection ?

[kitlith]

I'm generating an RSS feed on the fly via web scraping. (i'm literally hosting the web scraping feed generation on my phone, too). A lot of the time, it works just fine, but sometimes I just couldn't get any of the feeds to finish generating before the 10 second read timeout was up. So I implemented a frankly terrible keepalive kludge for it, and then thought "hey, maybe i should make a PR to Flym so that I can stop kludging it and other people could also get use out of it". Especially since there's been an open feature request asking for it almost 2 years.

The alternative is to just boost the timeout to a higher value. One of the RSS readers I found on github today had it set to something as high as an hour, so maybe that's a more appealing option.

[FredJul]

Ok, I understand your point. I would be OK to increase it to 20s, but I believe it's too rare to worth an option. IMO a server which does not answer in less that 20s seems to be a server issue, whatever it is doing. Putting an hour does not seems acceptable for me as well (the loader will be too long to disappears).

[kitlith]

Well, if you just want to bump that timeout to 20s, you don't need a PR from me to do that. Close this PR when you do that if you're going to do that.

a server which does not answer in less that 20s seems to be a server issue, whatever it is doing.

The only ways to send a response before the the full response is prepared in the web frameworks I've investigated/am using are... hacky, unless i've missed something completely. I'd like to send a 100 Continue or a 102 Processing (from webdav extensions, its rule of thumb is to send a 102 if it's going to take longer than 20 seconds for the full reply) except i have no way to do so, and I still have no idea if that would actually reset the read timeout for this, so I'm stuck putting <!-- keepalive --> in the body and streaming the response. I'd kinda like to get rid of this, but i guess it's going to stay for now, just because I want to be careful and have things 'always work' as much as possible.

(i suppose the other possible fix on my end would be to just make it a stateful server and cache stuff/implement its own polling period, etc, but I kinda like that it's stateless and the polling period is determined by whoever is talking to the feed generator)

so then i figured maybe i should just fix the issue here, at the source, by making the timeout configurable so that I (and a few others, such as the person who opened the issue i referenced) can just put the timeout as high as they want, and everyone else can just leave it where it is.

Is there something we can do to resolve the feeling that there's too many options?

[FredJul]

FYI I just have increased the default readTime to 20s. I'm not sure there is an easy solution to simplify the settings, but even if I do, I'm sorry but I'm generally not willing to add a setting for a very rare use case to avoid complexifying the code.

[ildar]

May i suggest a compromise? Put the value into an app database then those who eagerly need it can tune it with advanced tools (sqlite3)

[FredJul]

Hi, to be honest I believe almost nobody will use such a feature. You will need to have a rooted phone, the need for an increased read time and the knowledge to do it. I guess almost nobody will meet these requirements.

[ildar]

Allright, let OP give his voice.

[kitlith]

I delegate to the author on what they want to do in their software. I admit that my preferences are a bit different than theirs, and that I am looking at other RSS readers, but I was doing that before I opened the PR, too.

If it were my software, I would probably err on the side of adding more settings, and if I thought a set of settings would be rarely used, perhaps hiding it under a separate menu of "tuning options" or something, as both a place where I could experiment with them and tune to a decent set of defaults prior to shipping, and where users can go to modify things that don't quite suit them. Something like Android's developer options. Most users won't want or need to touch it, but the few that do will be grateful for it.

I don't like the idea of dumping it in a sqlite database. I can't clearly articulate my reasoning, but it's just too much, for both the user and the app, especially if the app isn't already using sqlite for other settings.

And really, this isn't my software, and it isn't even anywhere near the type of programming i typically do (command line applications) so once again, i delegate to the author. I'll probably just keep my ugly <!-- keepalive --> kludge for 'just in case' and call it a day.

I wasn't even planning on replying to this thread again, but since you want to "hear my voice", ildar, here you go. I'm leaving my branch up in case the author wants to revisit this later (or if someone really wants to maintain a fork w/ this setting added for some reason) but it's really not worth arguing over. I'd rather have effort put into #398 than in continuing to discuss/bikeshed whether or not the read timeout should be configurable, or otherwise what its value should be.