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

Don't fail all of an iteration of the device list retry loop on error #7609

Merged
merged 5 commits into from
Jun 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/7609.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Prevent an entire iteration of the device list resync loop from failing if one server responds with a malformed result.
36 changes: 21 additions & 15 deletions synapse/handlers/device.py
Original file line number Diff line number Diff line change
Expand Up @@ -704,22 +704,27 @@ def _maybe_retry_device_resync(self):
need_resync = yield self.store.get_user_ids_requiring_device_list_resync()
# Iterate over the set of user IDs.
for user_id in need_resync:
# Try to resync the current user's devices list. Exception handling
# isn't necessary here, since user_device_resync catches all instances
# of "Exception" that might be raised from the federation request. This
# means that if an exception is raised by this function, it must be
# because of a database issue, which means _maybe_retry_device_resync
# probably won't be able to go much further anyway.
result = yield self.user_device_resync(
user_id=user_id, mark_failed_as_stale=False,
)
# user_device_resync only returns a result if it managed to successfully
# resync and update the database. Updating the table of users requiring
# resync isn't necessary here as user_device_resync already does it
# (through self.store.update_remote_device_list_cache).
if result:
try:
# Try to resync the current user's devices list.
result = yield self.user_device_resync(
user_id=user_id, mark_failed_as_stale=False,
)

# user_device_resync only returns a result if it managed to
# successfully resync and update the database. Updating the table
# of users requiring resync isn't necessary here as
# user_device_resync already does it (through
# self.store.update_remote_device_list_cache).
if result:
logger.debug(
"Successfully resynced the device list for %s", user_id,
)
except Exception as e:
# If there was an issue resyncing this user, e.g. if the remote
# server sent a malformed result, just log the error instead of
# aborting all the subsequent resyncs.
logger.debug(
"Successfully resynced the device list for %s" % user_id,
"Could not resync the device list for %s: %s", user_id, e,
)
finally:
# Allow future calls to retry resyncinc out of sync device lists.
Expand All @@ -738,6 +743,7 @@ def user_device_resync(self, user_id, mark_failed_as_stale=True):
request:
https://matrix.org/docs/spec/server_server/r0.1.2#get-matrix-federation-v1-user-devices-userid
"""
logger.debug("Attempting to resync the device list for %s", user_id)
log_kv({"message": "Doing resync to update device list."})
# Fetch all devices for the user.
origin = get_domain_from_id(user_id)
Expand Down