From f346048a6e9ac798b742d939a38e0cfe71475f38 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 5 May 2017 10:34:10 +0100 Subject: [PATCH 1/3] Handle exceptions thrown in handling remote device list updates --- synapse/handlers/device.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/synapse/handlers/device.py b/synapse/handlers/device.py index c22f65ce5..72915b85d 100644 --- a/synapse/handlers/device.py +++ b/synapse/handlers/device.py @@ -17,6 +17,7 @@ from synapse.api.constants import EventTypes from synapse.util import stringutils from synapse.util.async import Linearizer from synapse.util.caches.expiringcache import ExpiringCache +from synapse.util.retryutils import NotRetryingDestination from synapse.util.metrics import measure_func from synapse.types import get_domain_from_id, RoomStreamToken from twisted.internet import defer @@ -430,7 +431,21 @@ class DeviceListEduUpdater(object): if resync: # Fetch all devices for the user. origin = get_domain_from_id(user_id) - result = yield self.federation.query_user_devices(origin, user_id) + try: + result = yield self.federation.query_user_devices(origin, user_id) + except NotRetryingDestination: + logger.warn( + "Failed to handle device list update for %s," + " we're not retrying the remote", + user_id, + ) + return + except Exception: + logger.exception( + "Failed to handle device list update for %s", user_id + ) + return + stream_id = result["stream_id"] devices = result["devices"] yield self.store.update_remote_device_list_cache( From b843631d7157f0beb64db62a86c691369aa49b14 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 5 May 2017 10:59:32 +0100 Subject: [PATCH 2/3] Add comment and TODO --- synapse/handlers/device.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/synapse/handlers/device.py b/synapse/handlers/device.py index 72915b85d..187af03fb 100644 --- a/synapse/handlers/device.py +++ b/synapse/handlers/device.py @@ -426,6 +426,8 @@ class DeviceListEduUpdater(object): # This can happen since we batch updates return + # Given a list of updates we check if we need to resync. This + # happens if we've missed updates. resync = yield self._need_to_do_resync(user_id, pending_updates) if resync: @@ -434,6 +436,8 @@ class DeviceListEduUpdater(object): try: result = yield self.federation.query_user_devices(origin, user_id) except NotRetryingDestination: + # TODO: Remember that we are now out of sync and try again + # later logger.warn( "Failed to handle device list update for %s," " we're not retrying the remote", @@ -441,6 +445,8 @@ class DeviceListEduUpdater(object): ) return except Exception: + # TODO: Remember that we are now out of sync and try again + # later logger.exception( "Failed to handle device list update for %s", user_id ) From 653d90c1a529ff553d506ac806ab0403f34955ad Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 5 May 2017 14:01:17 +0100 Subject: [PATCH 3/3] Comment --- synapse/handlers/device.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/synapse/handlers/device.py b/synapse/handlers/device.py index 187af03fb..982cda3ed 100644 --- a/synapse/handlers/device.py +++ b/synapse/handlers/device.py @@ -443,6 +443,12 @@ class DeviceListEduUpdater(object): " we're not retrying the remote", user_id, ) + # We abort on exceptions rather than accepting the update + # as otherwise synapse will 'forget' that its device list + # is out of date. If we bail then we will retry the resync + # next time we get a device list update for this user_id. + # This makes it more likely that the device lists will + # eventually become consistent. return except Exception: # TODO: Remember that we are now out of sync and try again