fix: Handle failed to send messages in low network significantly better

This commit is contained in:
Sorunome 2020-09-10 12:11:13 +02:00
parent bbc1b63695
commit b5ac500136
No known key found for this signature in database
GPG key ID: B19471D07FC9BE9C
5 changed files with 70 additions and 16 deletions

View file

@ -399,7 +399,7 @@ class Database extends _$Database {
// Is the timeline limited? Then all previous messages should be // Is the timeline limited? Then all previous messages should be
// removed from the database! // removed from the database!
if (roomUpdate.limitedTimeline) { if (roomUpdate.limitedTimeline) {
await removeRoomEvents(clientId, roomUpdate.id); await removeSuccessfulRoomEvents(clientId, roomUpdate.id);
await updateRoomSortOrder(0.0, 0.0, clientId, roomUpdate.id); await updateRoomSortOrder(0.0, 0.0, clientId, roomUpdate.id);
await setRoomPrevBatch(roomUpdate.prev_batch, clientId, roomUpdate.id); await setRoomPrevBatch(roomUpdate.prev_batch, clientId, roomUpdate.id);
} }
@ -432,9 +432,10 @@ class Database extends _$Database {
status = eventContent['unsigned'][MessageSendingStatusKey]; status = eventContent['unsigned'][MessageSendingStatusKey];
} }
if (eventContent['status'] is num) status = eventContent['status']; if (eventContent['status'] is num) status = eventContent['status'];
if ((status == 1 || status == -1) && var storeNewEvent = !((status == 1 || status == -1) &&
eventContent['unsigned'] is Map<String, dynamic> && eventContent['unsigned'] is Map<String, dynamic> &&
eventContent['unsigned']['transaction_id'] is String) { eventContent['unsigned']['transaction_id'] is String);
if (!storeNewEvent) {
final allOldEvents = final allOldEvents =
await getEvent(clientId, eventContent['event_id'], chatId).get(); await getEvent(clientId, eventContent['event_id'], chatId).get();
if (allOldEvents.isNotEmpty) { if (allOldEvents.isNotEmpty) {
@ -452,8 +453,15 @@ class Database extends _$Database {
} else { } else {
// status changed and we have an old transaction id --> update event id and stuffs // status changed and we have an old transaction id --> update event id and stuffs
try { try {
await updateEventStatus(status, eventContent['event_id'], clientId, final updated = await updateEventStatus(
eventContent['unsigned']['transaction_id'], chatId); status,
eventContent['event_id'],
clientId,
eventContent['unsigned']['transaction_id'],
chatId);
if (updated == 0) {
storeNewEvent = true;
}
} catch (err) { } catch (err) {
// we could not update the transaction id to the event id....so it already exists // we could not update the transaction id to the event id....so it already exists
// as we just tried to fetch the event previously this is a race condition if the event comes down sync in the mean time // as we just tried to fetch the event previously this is a race condition if the event comes down sync in the mean time
@ -461,7 +469,8 @@ class Database extends _$Database {
// than our status. So, we just ignore this error // than our status. So, we just ignore this error
} }
} }
} else { }
if (storeNewEvent) {
DbEvent oldEvent; DbEvent oldEvent;
if (type == 'history') { if (type == 'history') {
final allOldEvents = final allOldEvents =

View file

@ -6479,9 +6479,9 @@ abstract class _$Database extends GeneratedDatabase {
); );
} }
Future<int> removeRoomEvents(int client_id, String room_id) { Future<int> removeSuccessfulRoomEvents(int client_id, String room_id) {
return customUpdate( return customUpdate(
'DELETE FROM events WHERE client_id = :client_id AND room_id = :room_id', 'DELETE FROM events WHERE client_id = :client_id AND room_id = :room_id AND status <> -1 AND status <> 0',
variables: [Variable.withInt(client_id), Variable.withString(room_id)], variables: [Variable.withInt(client_id), Variable.withString(room_id)],
updates: {events}, updates: {events},
updateKind: UpdateKind.delete, updateKind: UpdateKind.delete,

View file

@ -230,7 +230,7 @@ getRoom: SELECT * FROM rooms WHERE client_id = :client_id AND room_id = :room_id
getEvent: SELECT * FROM events WHERE client_id = :client_id AND event_id = :event_id AND room_id = :room_id; getEvent: SELECT * FROM events WHERE client_id = :client_id AND event_id = :event_id AND room_id = :room_id;
removeEvent: DELETE FROM events WHERE client_id = :client_id AND event_id = :event_id AND room_id = :room_id; removeEvent: DELETE FROM events WHERE client_id = :client_id AND event_id = :event_id AND room_id = :room_id;
removeRoom: DELETE FROM rooms WHERE client_id = :client_id AND room_id = :room_id; removeRoom: DELETE FROM rooms WHERE client_id = :client_id AND room_id = :room_id;
removeRoomEvents: DELETE FROM events WHERE client_id = :client_id AND room_id = :room_id; removeSuccessfulRoomEvents: DELETE FROM events WHERE client_id = :client_id AND room_id = :room_id AND status <> -1 AND status <> 0;
storeFile: INSERT OR REPLACE INTO files (mxc_uri, bytes, saved_at) VALUES (:mxc_uri, :bytes, :time); storeFile: INSERT OR REPLACE INTO files (mxc_uri, bytes, saved_at) VALUES (:mxc_uri, :bytes, :time);
dbGetFile: SELECT * FROM files WHERE mxc_uri = :mxc_uri; dbGetFile: SELECT * FROM files WHERE mxc_uri = :mxc_uri;
markPendingEventsAsError: UPDATE events SET status = -1 WHERE client_id = :client_id AND status = 0; markPendingEventsAsError: UPDATE events SET status = -1 WHERE client_id = :client_id AND status = 0;

View file

@ -99,6 +99,7 @@ class Timeline {
for (final e in events) { for (final e in events) {
addAggregatedEvent(e); addAggregatedEvent(e);
} }
_sort();
} }
/// Don't forget to call this before you dismiss this object! /// Don't forget to call this before you dismiss this object!
@ -274,7 +275,15 @@ class Timeline {
void _sort() { void _sort() {
if (_sortLock || events.length < 2) return; if (_sortLock || events.length < 2) return;
_sortLock = true; _sortLock = true;
events?.sort((a, b) => b.sortOrder - a.sortOrder > 0 ? 1 : -1); events?.sort((a, b) {
if (b.status == -1 && a.status != -1) {
return 1;
}
if (a.status == -1 && b.status != -1) {
return -1;
}
return b.sortOrder - a.sortOrder > 0 ? 1 : -1;
});
_sortLock = false; _sortLock = false;
} }
} }

View file

@ -218,6 +218,7 @@ void main() {
}); });
test('Resend message', () async { test('Resend message', () async {
timeline.events.clear();
client.onEvent.add(EventUpdate( client.onEvent.add(EventUpdate(
type: 'timeline', type: 'timeline',
roomID: roomID, roomID: roomID,
@ -233,6 +234,7 @@ void main() {
}, },
sortOrder: room.newSortOrder)); sortOrder: room.newSortOrder));
await Future.delayed(Duration(milliseconds: 50)); await Future.delayed(Duration(milliseconds: 50));
expect(timeline.events[0].status, -1);
await timeline.events[0].sendAgain(); await timeline.events[0].sendAgain();
await Future.delayed(Duration(milliseconds: 50)); await Future.delayed(Duration(milliseconds: 50));
@ -240,22 +242,23 @@ void main() {
expect(updateCount, 17); expect(updateCount, 17);
expect(insertList, [0, 0, 0, 0, 0, 0, 0, 0]); expect(insertList, [0, 0, 0, 0, 0, 0, 0, 0]);
expect(timeline.events.length, 7); expect(timeline.events.length, 1);
expect(timeline.events[0].status, 1); expect(timeline.events[0].status, 1);
}); });
test('Request history', () async { test('Request history', () async {
timeline.events.clear();
await room.requestHistory(); await room.requestHistory();
await Future.delayed(Duration(milliseconds: 50)); await Future.delayed(Duration(milliseconds: 50));
expect(updateCount, 20); expect(updateCount, 20);
expect(timeline.events.length, 10); expect(timeline.events.length, 3);
expect(timeline.events[7].eventId, '3143273582443PhrSn:example.org'); expect(timeline.events[0].eventId, '3143273582443PhrSn:example.org');
expect(timeline.events[8].eventId, '2143273582443PhrSn:example.org'); expect(timeline.events[1].eventId, '2143273582443PhrSn:example.org');
expect(timeline.events[9].eventId, '1143273582443PhrSn:example.org'); expect(timeline.events[2].eventId, '1143273582443PhrSn:example.org');
expect(room.prev_batch, 't47409-4357353_219380_26003_2265'); expect(room.prev_batch, 't47409-4357353_219380_26003_2265');
await timeline.events[9].redact(reason: 'test', txid: '1234'); await timeline.events[2].redact(reason: 'test', txid: '1234');
}); });
test('Clear cache on limited timeline', () async { test('Clear cache on limited timeline', () async {
@ -271,6 +274,39 @@ void main() {
expect(timeline.events.isEmpty, true); expect(timeline.events.isEmpty, true);
}); });
test('sort errors on top', () async {
timeline.events.clear();
client.onEvent.add(EventUpdate(
type: 'timeline',
roomID: roomID,
eventType: 'm.room.message',
content: {
'type': 'm.room.message',
'content': {'msgtype': 'm.text', 'body': 'Testcase'},
'sender': '@alice:example.com',
'status': -1,
'event_id': 'abc',
'origin_server_ts': testTimeStamp
},
sortOrder: room.newSortOrder));
client.onEvent.add(EventUpdate(
type: 'timeline',
roomID: roomID,
eventType: 'm.room.message',
content: {
'type': 'm.room.message',
'content': {'msgtype': 'm.text', 'body': 'Testcase'},
'sender': '@alice:example.com',
'status': 2,
'event_id': 'def',
'origin_server_ts': testTimeStamp + 5
},
sortOrder: room.newSortOrder));
await Future.delayed(Duration(milliseconds: 50));
expect(timeline.events[0].status, -1);
expect(timeline.events[1].status, 2);
});
test('sending event to failed update', () async { test('sending event to failed update', () async {
timeline.events.clear(); timeline.events.clear();
client.onEvent.add(EventUpdate( client.onEvent.add(EventUpdate(