zulip / zulip-flutter

Upcoming Zulip mobile apps for Android and iOS, using Flutter
Apache License 2.0
196 stars 189 forks source link

msglist: Throttle fetchOlder retries #1050

Open PIG208 opened 2 weeks ago

PIG208 commented 2 weeks ago

This approach is different from how a BackoffMachine is typically used, because the message list doesn't send and retry requests in a loop; its caller retries rapidly on scroll changes, and we want to ignore the excessive requests.

The test drops irrelevant requests with connection.takeRequests without checking, as we are only interested in verifying that no request was sent.

Fixes: #945

chrisbobbe commented 2 weeks ago

(Rerunning CI.)

PIG208 commented 1 week ago

(Working on another update to address a potential race; don't review yet)

PIG208 commented 1 week ago

The PR has been updated with the proposed changes, and additionally, a check for this.generation == generation to avoid potential races after adding _updateEndMarkers and notifyListeners to the backoff callback.

I have also moved the backoff machine variable, renamed to _fetchOlderCooldownBackoffMachine with Cooldown as a noun, to _MessageSequence right next to fetchOlderCoolingDown, because they are relevant to each other. The backoff machine does not have a public getter and we keep it private.

The loading indicator change might be out of scope for this PR, and we should work on that as a follow-up.

chrisbobbe commented 1 week ago

Looks like CI is failing, could you take a look?

PIG208 commented 1 week ago

Updated the PR. Thanks!