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

Rework network request cancellation and stale request handling #640

Open
baka-kaba opened this issue Nov 3, 2018 · 3 comments
Open

Rework network request cancellation and stale request handling #640

baka-kaba opened this issue Nov 3, 2018 · 3 comments

Comments

@baka-kaba
Copy link
Contributor

baka-kaba commented Nov 3, 2018

There are some situations where a request should be cancelled when a new one comes in - say you're moving quickly through a bunch of pages by tapping the next arrow, the current request should end as the new one is queued.

This would cut down on unnecessary network traffic (especially if we add a short delay before launching the request, to make sure the user has finished tapping), but it also avoids bugs - it's possible to queue up enough requests that they arrive out of order, so the user ends up looking at the wrong page etc.

There's already code in place to handle this - each AwfulRequest can supply a unique tag, usually as a static member so all the requests of a certain type are grouped together. And NetworkUtils#cancelRequest cancels all queued requests with the supplied tag, so it's possible to interrupt all PostRequests for example (ThreadDisplayFragment does this in #syncThread).

AwfulFragment also has a #queueRequest method that allows you to set a "cancel on destroy" flag, which tags a request with the fragment instead so they can all be cleaned up when it dies. This calls a #cancelNetworkRequests method, which can be overridden to also call NetworkUtils#cancelRequest with a particular request tag to clean up any requests tagged that way too (ThreadDisplayFragment also does this)

if that's all a bit confusing, yep it could do with being reworked

@baka-kaba baka-kaba changed the title Cancel stale network requests Rework network request cancellation and stale request handling Nov 3, 2018
@Sereri
Copy link
Member

Sereri commented Nov 3, 2018

Like, I'm not saying "this has to stay in", especially because of the discoverability, but this partly was in response to people wanting to cache threads for later reading. Go to a thread, quickly queue the pages that you want to read and you're done.
Honestly it would be nicer to have some sort of GUI around it anyway, so feel free to change this so that it cancels old requests

@baka-kaba
Copy link
Contributor Author

Yeah but didn't we already remove that? I can page through a bunch of thread pages, and the logs only show some of the requests, so it sorta works. I'm not sure how Volley handles it once the request has been started, I'd have thought it would outright stop the response handling, but it seems like once the (pretty quick) network request comes back with a response, it carries on handling it and delivering the results. It's pretty inconsistent, and I feel like results arriving out of order will cause problems in the depths of the app

And yeah caching is good, but I'd want to make that a separate process that doesn't hammer the site, something a bit like the forum updater that gradually crawls the forum hierarchy. What's the taxman's policy on downloading for offline reading? Thought I heard someone say he's not keen

@Sereri
Copy link
Member

Sereri commented Nov 3, 2018

Honestly most of Lowtax's "policies" are not really existent and more interpretation. It's like lore or something. As far as I know Lowtax doesn't really care about the apps at all, probably to a fault. However it's better to not piss him off so everyone tries to avoid that. It's kinda like he's old testament god where you don't know how he will react to X, so you try the least offensive version to not make him smite you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants