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 3 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 to fail if one server responds with a malformed result.
babolivier marked this conversation as resolved.
Show resolved Hide resolved
42 changes: 27 additions & 15 deletions synapse/handlers/device.py
Original file line number Diff line number Diff line change
Expand Up @@ -704,22 +704,33 @@ 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. 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.
babolivier marked this conversation as resolved.
Show resolved Hide resolved
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 +749,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