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

[Bug]: Forced reload on reestablished internet connection #1144

Open
farao opened this issue Sep 30, 2024 · 8 comments
Open

[Bug]: Forced reload on reestablished internet connection #1144

farao opened this issue Sep 30, 2024 · 8 comments
Labels
bug Something isn't working

Comments

@farao
Copy link

farao commented Sep 30, 2024

Describe the bug

I've been using Tuba on mobile and there, sometimes the internet goes away for a while. This is not a problem since Tuba at least caches some posts and you can continue scrolling/reading. But when the internet gets reestablished, you can't continue scrolling but it reloads forcefully and drags you on top of your timeline.

My whish would be that you can just continue scrolling and whenever you want to load newer posts, you just do that manually (e.g. by going to the top of the timeline and dragging down).

Steps To Reproduce

  1. Turn off internet
  2. Scroll a bit
  3. Turn on internet

Instance Backend

Mastodon

Operating System

Furios trixie (based on Droidian)

Package

Flatpak

@farao farao added the bug Something isn't working label Sep 30, 2024
@GeopJr
Copy link
Owner

GeopJr commented Sep 30, 2024

Not sure :/

The main issue is that Tuba is unaware of what happened while offline. E.g. when someone edits a post, a websocket event gets emitted and Tuba updates the edited post accordingly. When you go offline, the websocket obviously disconnects, so you never receive any updates. When you go back offline, you will be missing them until you refresh.

That can lead to situations where someone might have posted misinformation and edited or even deleted their post but you might never learn about it because you will receive the updates when you refresh manually 🤷

If the disconnects are for a short period of time, I can maybe set up a check to only refresh if disconnected for over 1 minute or similar.

@farao
Copy link
Author

farao commented Nov 9, 2024

I had a bit a look into this and I found that there was already a reconnect mechanism for the web socket and all that was necessary was just not to unsubscribe the url and not to reload and jump to the start on reconnect. I made a PR #1196 for this, what do you think about it?

@farao
Copy link
Author

farao commented Nov 9, 2024

I see of course the problem with the possible updates in between, but I think this might be better than reloading the whole timeline after an outage? This anyways only concerns the messages that have been already loaded if I understand the problem correctly?

@farao
Copy link
Author

farao commented Nov 9, 2024

If you find this inacceptible, I could imagine getting the timeline that is already displayed again in the background and doing updates and deletes if necessary?

@GeopJr
Copy link
Owner

GeopJr commented Nov 9, 2024

there was already a reconnect mechanism for the web socket

That's only meant for server disconnections. It really shouldn't be used for when the app is offline as it will keep trying every 6 seconds to reconnect forever which could be hours.

Plus it will stop on connection errors. The reconnect mechanism is for when the websocket errors after successfully connecting, with the exception of 'Connection Reset By Peer'.

Code wise, reconnections happen here https://github.com/GeopJr/Tuba/blob/main/src/Services/Network/Streams.vala#L74 (notice that to get there, websocket_connect_async has to have NOT errored out)
while if websocket_connect_async errors with 'Connection Reset By Peer' and only then, it will try to reconnect, otherwise it will just stop https://github.com/GeopJr/Tuba/blob/main/src/Services/Network/Streams.vala#L80

but I think this might be better than reloading the whole timeline after an outage?

If you are using a stable connection, it shouldn't happen often. Actually the main reason network connection monitor was ever added was host hibernation/sleep/suspension, where it would stay offline for hours and when coming back websockets wouldn't work.

If you are using an unstable connection, IMO the only ways forward are either monitoring how long tuba was offline and only refresh the timeline if it was 10+ minutes or manually disabling network monitoring (there's a dconf option for that, not in the UI)

This anyways only concerns the messages that have been already loaded if I understand the problem correctly?
You will also be missing any newer posts and notifications. New posts come from the websocket, so you'll have to refresh eventually. If you don't refresh, it will look like this:

  • New messages from the new websocket connection
  • [posts posts the old websocket connection you never received won't be here]
  • Old posts that loaded prior to the disconnection

I could imagine getting the timeline that is already displayed again in the background and doing updates and deletes if necessary?

Not possible in a reasonable way. If you have loaded, let's say 5000 posts, it would take ages to get all of them 40 or so at a time and will get rate limited sooner.


My solutions are:

  • Disabling refreshing the timelines altogether and accept the consequences. Websockets will keep the same behavior, reconnecting them manually is really not that big of an issue.
  • Only refresh timelines if the app was offline for 10+ minutes.
  • Increase or make the network monitor decision delay configurable. Currently, if you go offline, tuba will wait 5 seconds iirc before marking you as such, so connections that go offline for 1-2 seconds and come back won't trigger a network change.

@farao
Copy link
Author

farao commented Nov 10, 2024

there was already a reconnect mechanism for the web socket

That's only meant for server disconnections. It really shouldn't be used for when the app is offline as it will keep trying every 6 seconds to reconnect forever which could be hours.

I see. Would it be a possibility to have at the end of the currently loaded timeline a "hurdle" that you need to scroll over (don't know how to describe this best, android apps have that quite often that you scroll over the end and a spinner icon or so appears) and only then it tries to reconnect?

Code wise, reconnections happen here https://github.com/GeopJr/Tuba/blob/main/src/Services/Network/Streams.vala#L74 (notice that to get there, websocket_connect_async has to have NOT errored out) while if websocket_connect_async errors with 'Connection Reset By Peer' and only then, it will try to reconnect, otherwise it will just stop https://github.com/GeopJr/Tuba/blob/main/src/Services/Network/Streams.vala#L80

but I think this might be better than reloading the whole timeline after an outage?

If you are using a stable connection, it shouldn't happen often. Actually the main reason network connection monitor was ever added was host hibernation/sleep/suspension, where it would stay offline for hours and when coming back websockets wouldn't work.

Yeah, unfortunately this calculation is e.g. without considering linux on mobile on german trains (just as an example ;)). It's really annoying if you scrolled for a while - can't continue for the network being gone (which is ok, you can't change that) - but then on reconnect it pulls you out of the point until which you scrolled to reload. Here just reconnecting after 10min+ does not necessarily help because you might be longer without internet. But surely would help in some situations.

If you are using an unstable connection, IMO the only ways forward are either monitoring how long tuba was offline and only refresh the timeline if it was 10+ minutes or manually disabling network monitoring (there's a dconf option for that, not in the UI)

I just realized that I didn't consider the "streaming approach" at all until now, so that's why the PR just worked for me and I did not consider the consequences with the streaming approach. When starting to use Tuba, I switched streaming of timelines off in the settings because there was no way to jump up after a bit of scrolling to the newest message that I still knew. I really liked how Twidere (android app, amongst others for mastodon) did this: when scrolling up when being on the latest toot that was loaded, it showed a spinner and loaded the new statuses but did not change your position in the timeline but allowed you to scroll upwards from that point.

This anyways only concerns the messages that have been already loaded if I understand the problem correctly?
You will also be missing any newer posts and notifications. New posts come from the websocket, so you'll have to refresh eventually. If you don't refresh, it will look like this:

* New messages from the new websocket connection

* [posts posts the old websocket connection you never received won't be here]

* Old posts that loaded prior to the disconnection

Yes, as written above, I unfortunately did not put the streaming approach into consideration. I guess there's no way in the mastodon API (at least I did not find one) to only get all updates that happened for the last x minutes (if we know how many minutes the app has been offline), right?

I could imagine getting the timeline that is already displayed again in the background and doing updates and deletes if necessary?

Not possible in a reasonable way. If you have loaded, let's say 5000 posts, it would take ages to get all of them 40 or so at a time and will get rate limited sooner.

True!

My solutions are:

* Disabling refreshing the timelines altogether and accept the consequences. Websockets will keep the same behavior, reconnecting them manually is really not that big of an issue.

* Only refresh timelines if the app was offline for 10+ minutes.

* Increase or make the network monitor decision delay configurable. Currently, if you go offline, tuba will wait 5 seconds iirc before marking you as such, so connections that go offline for 1-2 seconds and come back won't trigger a network change.

After considering everything above, I would like to propose two other approaches:

  • keep the behavior as it is for the streaming mode (or maybe add a small delay to get you over minimal outages) and do the trying for reconnect way only for the non-streaming mode? The reconnects could be as described above even only user-triggered by scrolling "over" the end of the timeline?
  • always only load a "frame" of posts around where the users current scroll position, e.g. you could load three api-pages at a time and when you enter the third (does not matter if scrolling upwards or downwards), load the next one in that direction and remove all statuses from the last one "behind" you. That way, you could always keep scrolling but when reconnecting you don't need to go over 5000 posts but at most over three api-pages

What do you think about these? I know that both change the behavior of the app somehow and at least the second would need quite some code changes, but maybe you like one of the ideas?

@GeopJr
Copy link
Owner

GeopJr commented Nov 10, 2024

I see. Would it be a possibility to have at the end of the currently loaded timeline a "hurdle" that you need to scroll over (don't know how to describe this best, android apps have that quite often that you scroll over the end and a spinner icon or so appears) and only then it tries to reconnect?

That's how pagination works currently, when you scroll near the bottom, it will try to fetch the next timeline page and append it.

I will try not to derail this issue too much while responding to some of the above since they are very lengthy.

For starters, streaming aka websocket connections are 100% required when using Tuba. Notifications? Websockets. New posts? Websockets. Post updates/deletions? Websockets. A websocket connection is required at all times while online, that's why it will keep trying to connect if closed/errored. However, if it errors out while attempting to connect (aka not a websocket error), and it's not due to 'connection reset by peer' (common on busy servers like mastodon.social), then it will stop. No point in spamming a server every x seconds if we can't even connect to it for whatever reason.

The streaming option in settings is about the federated and local timelines only. Since those can be extremely busy; mastodon.social's federated timeline can get 10+ new posts per second sometimes.

I really liked how Twidere (android app, amongst others for mastodon) did this: when scrolling up when being on the latest toot that was loaded, it showed a spinner and loaded the new statuses but did not change your position in the timeline but allowed you to scroll upwards from that point.

always only load a "frame" of posts around where the users current scroll position, e.g. you could load three api-pages at a time and when you enter the third (does not matter if scrolling upwards or downwards), load the next one in that direction and remove all statuses from the last one "behind" you. That way, you could always keep scrolling but when reconnecting you don't need to go over 5000 posts but at most over three api-pages

This is basically ListBox vs ListView. It's an extremely loooooooooong issue (#500 #504 #410 #179 and many more) that's been eating my brain for ages. I'll try to be as short as possible. ListBox, what we use currently, is a list of widgets. It's inefficient, it holds all widgets in memory, it's not tied to a scrolled window, so when a new post gets added or one gets deleted, its height changes and causes the issue you described, while the scroll position stays the same.

ListView on the other hand fixes all the above. It holds a list of objects and generates and destroys widgets based on that as you scroll. Buuuuuuuuuut that had other problems (yes I did implement it like a year ago but ended up reverting it). Due to the way it works, the objects and the widgets are separated. ListView widget building works in 4 stages, setup, bind, unbind, teardown.

On setup, it creates empty widgets, so if you had a listview of labels, it would create empty labels. On bind, it binds the objects that are currently visible to the widgets it created prior, so it would now set the label content here. The other two stages do the reverse.

The benefit of recycler views like ListView is that the widgets created in the setup stage get re-used by unbinding and re-binding them to objects (you can read more here https://blog.gtk.org/2020/09/05/a-primer-on-gtklistview/). The main problem here is that you can only have 1 widget type. So only labels in the above example.

That makes it extremely limiting for us. E.g. the notifications tab can have 'post' widgets, 'account' widgets (on follow requests), 'label' widgets on admin notifications etc. Those are 3 completely different widgets.

So my next thought when I implemented it a year ago was to create the actual widget on the bind stage (and destroy it on unbind). And it worked! Until... you reach the point where it has to recycle widgets, then it destroys and creates hundreds of widgets per scroll, making tuba freeze. And therefore it was reverted ://

(And the other issue with ListViews was accessibility; keyboard navigation was impossible, it would only go up to the first or second widget and then move to the next post, so even if ListView didn't have the other issues this would be a showstopper)

Your second suggestion is basically re-implementing the way ListView works but for ListBox, but it will fall under the exact same issue as above, creating and destroying way too many widgets at the same time (and manually deciding the scrolled position which can be messy as post widgets can have different heights).


To fix 'it showed a spinner and loaded the new statuses but did not change your position in the timeline but allowed you to scroll upwards from that point' on ListBox, I made the timelines use a queue. Currently, if you are not close to the top of the timeline, new posts are added to an internal queue and get prepended when you scroll close to the top, fixing the timeline constantly moving up-and-down. This is the reason you don't notice that behavior anymore and not because 'streaming' is disabled, because it's not.


Ideally, what we need is a button between the 'old posts' and 'post from the new websocket connection' that will attempt to load the in-betweens by fetching one timeline page between them and re-show the button based on that buuuuuuuuuut... I'm hesitant to introduce anything like that because, with the above knowledge, when we move to ListViews at some point, we will probably have to get rid of them as they are a different widget than the posts.

I'm kinda cornered here lol


I do not have a linux phone so I am not aware of what behaviors are considered annoying on them compared to desktop. After your insights I can clearly see that refreshing can be very annoying. Maybe I should reconsider the button solution above

@farao
Copy link
Author

farao commented Nov 13, 2024

Wow, thanks a lot for the detailled explanation! I'll read up a bit on ListView and have a look at the respective issues!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants