Skip to content
This repository was archived by the owner on Apr 26, 2024. It is now read-only.

Commit c1bdd4f

Browse files
authored
Don't fail all of an iteration of the device list retry loop on error (#7609)
Without this patch, if an error happens which isn't caught by `user_device_resync`, then `_maybe_retry_device_resync` would fail, without retrying the next users in the iteration. This patch fixes this so that it now only logs an error in this case.
1 parent 2dc430d commit c1bdd4f

File tree

2 files changed

+22
-15
lines changed

2 files changed

+22
-15
lines changed

changelog.d/7609.bugfix

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Prevent an entire iteration of the device list resync loop from failing if one server responds with a malformed result.

synapse/handlers/device.py

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -704,22 +704,27 @@ def _maybe_retry_device_resync(self):
704704
need_resync = yield self.store.get_user_ids_requiring_device_list_resync()
705705
# Iterate over the set of user IDs.
706706
for user_id in need_resync:
707-
# Try to resync the current user's devices list. Exception handling
708-
# isn't necessary here, since user_device_resync catches all instances
709-
# of "Exception" that might be raised from the federation request. This
710-
# means that if an exception is raised by this function, it must be
711-
# because of a database issue, which means _maybe_retry_device_resync
712-
# probably won't be able to go much further anyway.
713-
result = yield self.user_device_resync(
714-
user_id=user_id, mark_failed_as_stale=False,
715-
)
716-
# user_device_resync only returns a result if it managed to successfully
717-
# resync and update the database. Updating the table of users requiring
718-
# resync isn't necessary here as user_device_resync already does it
719-
# (through self.store.update_remote_device_list_cache).
720-
if result:
707+
try:
708+
# Try to resync the current user's devices list.
709+
result = yield self.user_device_resync(
710+
user_id=user_id, mark_failed_as_stale=False,
711+
)
712+
713+
# user_device_resync only returns a result if it managed to
714+
# successfully resync and update the database. Updating the table
715+
# of users requiring resync isn't necessary here as
716+
# user_device_resync already does it (through
717+
# self.store.update_remote_device_list_cache).
718+
if result:
719+
logger.debug(
720+
"Successfully resynced the device list for %s", user_id,
721+
)
722+
except Exception as e:
723+
# If there was an issue resyncing this user, e.g. if the remote
724+
# server sent a malformed result, just log the error instead of
725+
# aborting all the subsequent resyncs.
721726
logger.debug(
722-
"Successfully resynced the device list for %s" % user_id,
727+
"Could not resync the device list for %s: %s", user_id, e,
723728
)
724729
finally:
725730
# Allow future calls to retry resyncinc out of sync device lists.
@@ -738,6 +743,7 @@ def user_device_resync(self, user_id, mark_failed_as_stale=True):
738743
request:
739744
https://matrix.org/docs/spec/server_server/r0.1.2#get-matrix-federation-v1-user-devices-userid
740745
"""
746+
logger.debug("Attempting to resync the device list for %s", user_id)
741747
log_kv({"message": "Doing resync to update device list."})
742748
# Fetch all devices for the user.
743749
origin = get_domain_from_id(user_id)

0 commit comments

Comments
 (0)