Fix: show spinner again while recovering from connection error #702
Conversation
|
I wonder if we should also show the spinner while in RECONNECTING state. If you sleep your laptop and open it again 3 days later and go straight back to riot, you might not have 3 sync fails to go from RECONNECTING to ERROR. If the catchup sync then takes long (Matthew had one that took 5 minutes, I guess also because of retry logic after 80 seconds), it'll be very confusing to not have a spinner. Downside would be that if a connection drop on a sync request, we'll show a spinner briefly... maybe that's better than the catchup-sync-without-spinner confusion? Wdyt @dbkr? |
|
FTR, closed matrix-org/matrix-react-sdk#2138 in favor of this approach. |
There was a problem hiding this comment.
Yeah, it makes sense to represent somehow the state where we know our current state is severely out of date (ie. when you've opened your laptop after a day). RECONNECTING isn't that though - PREPARED seems closer to the truth, although should we be adding another edge in that diagram from 'SYNCING' to 'PREPARED'?
src/sync.js
Outdated
| @@ -780,6 +780,13 @@ SyncApi.prototype._onSyncError = function(err, syncOptions) { | |||
| // instead, so that clients can observe this state | |||
| // if they wish. | |||
| this._startKeepAlives().then(() => { | |||
| if (this.getSyncState() == 'ERROR') { | |||
This is according to the state diagram in client.js. This will show a spinner at the bottom of a room again while the catchup sync is in progress, which seems to have broken at some point.
bwindels
force-pushed
the
bwindels/fixreconnectspinner
branch
from
beccb8f
to
0d23d04
Compare
|
Now needs matrix-org/matrix-react-sdk#2143 on react-sdk side to fix the spinner. |
|
@dbkr Yeah, good point, this might be a breaking change, but these changes would be released together with the already breaking changes for lazy loading (e.g. |
|
ah, yes - I'd forgotten about that one. Might be an idea to stick a 'breaking change' heading at the top of the changelog, or note them down somewhere, otherwise I'll forget. |
Pass through PREPARED state after error, when keepalive returns succes.
This is according to the state diagram in client.js.
This will show a spinner at the bottom of a room again
while the catchup sync is in progress,
which seems to have broken at some point.
Addresses item 6 (We no longer show a spinner when recovering after losing connectivity) of element-hq/element-web#7182