mirrors/synapse
1
0
Fork 0
mirror of https://github.com/element-hq/synapse.git synced 2024-11-28 23:20:09 +03:00
Code Issues Projects Releases Packages Wiki Activity

Pass errors back to the client when trying multiple federation destinations. (#9868)

Browse source 
This ensures that something like an auth error (403) will be
returned to the requester instead of attempting to try more
servers, which will likely result in the same error, and then
passing back a generic 400 error.
This commit is contained in:
Patrick Cloke 2021-04-27 07:30:34 -04:00 committed by GitHub
parent 0ffa5fb935
commit 1350b053da
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 61 additions and 58 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
changelog.d/9868.bugfix Normal file
View file

@ -0,0 +1 @@
Fix a long-standing bug where errors from federation did not propagate to the client.

118
synapse/federation/federation_client.py
View file

@ -451,6 +451,28 @@ class FederationClient(FederationBase):
return signed_auth return signed_auth
def _is_unknown_endpoint(
self, e: HttpResponseException, synapse_error: Optional[SynapseError] = None
) -> bool:
"""
Returns true if the response was due to an endpoint being unimplemented.
Args:
e: The error response received from the remote server.
synapse_error: The above error converted to a SynapseError. This is
automatically generated if not provided.
"""
if synapse_error is None:
synapse_error = e.to_synapse_error()
# There is no good way to detect an "unknown" endpoint.
#
# Dendrite returns a 404 (with no body); synapse returns a 400
# with M_UNRECOGNISED.
return e.code == 404 or (
e.code == 400 and synapse_error.errcode == Codes.UNRECOGNIZED
)
async def _try_destination_list( async def _try_destination_list(
self, self,
description: str, description: str,
@ -468,9 +490,9 @@ class FederationClient(FederationBase):
callback: Function to run for each server. Passed a single callback: Function to run for each server. Passed a single
argument: the server_name to try. argument: the server_name to try.
If the callback raises a CodeMessageException with a 300/400 code, If the callback raises a CodeMessageException with a 300/400 code or
attempts to perform the operation stop immediately and the exception is an UnsupportedRoomVersionError, attempts to perform the operation
reraised. stop immediately and the exception is reraised.
Otherwise, if the callback raises an Exception the error is logged and the Otherwise, if the callback raises an Exception the error is logged and the
next server tried. Normally the stacktrace is logged but this is next server tried. Normally the stacktrace is logged but this is
@ -492,8 +514,7 @@ class FederationClient(FederationBase):
continue continue
try: try:
res = await callback(destination) return await callback(destination)
return res
except InvalidResponseError as e: except InvalidResponseError as e:
logger.warning("Failed to %s via %s: %s", description, destination, e) logger.warning("Failed to %s via %s: %s", description, destination, e)
except UnsupportedRoomVersionError: except UnsupportedRoomVersionError:
@ -502,17 +523,15 @@ class FederationClient(FederationBase):
synapse_error = e.to_synapse_error() synapse_error = e.to_synapse_error()
failover = False failover = False
# Failover on an internal server error, or if the destination
# doesn't implemented the endpoint for some reason.
if 500 <= e.code < 600: if 500 <= e.code < 600:
failover = True failover = True
elif failover_on_unknown_endpoint: elif failover_on_unknown_endpoint and self._is_unknown_endpoint(
# there is no good way to detect an "unknown" endpoint. Dendrite e, synapse_error
# returns a 404 (with no body); synapse returns a 400 ):
# with M_UNRECOGNISED. failover = True
if e.code == 404 or (
e.code == 400 and synapse_error.errcode == Codes.UNRECOGNIZED
):
failover = True
if not failover: if not failover:
raise synapse_error from e raise synapse_error from e
@ -570,9 +589,8 @@ class FederationClient(FederationBase):
UnsupportedRoomVersionError: if remote responds with UnsupportedRoomVersionError: if remote responds with
a room version we don't understand. a room version we don't understand.
SynapseError: if the chosen remote server returns a 300/400 code. SynapseError: if the chosen remote server returns a 300/400 code, or
no servers successfully handle the request.
RuntimeError: if no servers were reachable.
""" """
valid_memberships = {Membership.JOIN, Membership.LEAVE} valid_memberships = {Membership.JOIN, Membership.LEAVE}
if membership not in valid_memberships: if membership not in valid_memberships:
@ -642,9 +660,8 @@ class FederationClient(FederationBase):
``auth_chain``. ``auth_chain``.
Raises: Raises:
SynapseError: if the chosen remote server returns a 300/400 code. SynapseError: if the chosen remote server returns a 300/400 code, or
no servers successfully handle the request.
RuntimeError: if no servers were reachable.
""" """
async def send_request(destination) -> Dict[str, Any]: async def send_request(destination) -> Dict[str, Any]:
@ -673,7 +690,7 @@ class FederationClient(FederationBase):
if create_event is None: if create_event is None:
# If the state doesn't have a create event then the room is # If the state doesn't have a create event then the room is
# invalid, and it would fail auth checks anyway. # invalid, and it would fail auth checks anyway.
raise SynapseError(400, "No create event in state") raise InvalidResponseError("No create event in state")
# the room version should be sane. # the room version should be sane.
create_room_version = create_event.content.get( create_room_version = create_event.content.get(
@ -746,16 +763,11 @@ class FederationClient(FederationBase):
content=pdu.get_pdu_json(time_now), content=pdu.get_pdu_json(time_now),
) )
except HttpResponseException as e: except HttpResponseException as e:
if e.code in [400, 404]: # If an error is received that is due to an unrecognised endpoint,
err = e.to_synapse_error() # fallback to the v1 endpoint. Otherwise consider it a legitmate error
# and raise.
# If we receive an error response that isn't a generic error, or an if not self._is_unknown_endpoint(e):
# unrecognised endpoint error, we assume that the remote understands raise
# the v2 invite API and this is a legitimate error.
if err.errcode not in [Codes.UNKNOWN, Codes.UNRECOGNIZED]:
raise err
else:
raise e.to_synapse_error()
logger.debug("Couldn't send_join with the v2 API, falling back to the v1 API") logger.debug("Couldn't send_join with the v2 API, falling back to the v1 API")
@ -802,6 +814,11 @@ class FederationClient(FederationBase):
Returns: Returns:
The event as a dict as returned by the remote server The event as a dict as returned by the remote server
Raises:
SynapseError: if the remote server returns an error or if the server
only supports the v1 endpoint and a room version other than "1"
or "2" is requested.
""" """
time_now = self._clock.time_msec() time_now = self._clock.time_msec()
@ -817,28 +834,19 @@ class FederationClient(FederationBase):
}, },
) )
except HttpResponseException as e: except HttpResponseException as e:
if e.code in [400, 404]: # If an error is received that is due to an unrecognised endpoint,
err = e.to_synapse_error() # fallback to the v1 endpoint if the room uses old-style event IDs.
# Otherwise consider it a legitmate error and raise.
# If we receive an error response that isn't a generic error, we err = e.to_synapse_error()
# assume that the remote understands the v2 invite API and this if self._is_unknown_endpoint(e, err):
# is a legitimate error.
if err.errcode != Codes.UNKNOWN:
raise err
# Otherwise, we assume that the remote server doesn't understand
# the v2 invite API. That's ok provided the room uses old-style event
# IDs.
if room_version.event_format != EventFormatVersions.V1: if room_version.event_format != EventFormatVersions.V1:
raise SynapseError( raise SynapseError(
400, 400,
"User's homeserver does not support this room version", "User's homeserver does not support this room version",
Codes.UNSUPPORTED_ROOM_VERSION, Codes.UNSUPPORTED_ROOM_VERSION,
) )
elif e.code in (403, 429):
raise e.to_synapse_error()
else: else:
raise raise err
# Didn't work, try v1 API. # Didn't work, try v1 API.
# Note the v1 API returns a tuple of `(200, content)` # Note the v1 API returns a tuple of `(200, content)`
@ -865,9 +873,8 @@ class FederationClient(FederationBase):
pdu: event to be sent pdu: event to be sent
Raises: Raises:
SynapseError if the chosen remote server returns a 300/400 code. SynapseError: if the chosen remote server returns a 300/400 code, or
no servers successfully handle the request.
RuntimeError if no servers were reachable.
""" """
async def send_request(destination: str) -> None: async def send_request(destination: str) -> None:
@ -889,16 +896,11 @@ class FederationClient(FederationBase):
content=pdu.get_pdu_json(time_now), content=pdu.get_pdu_json(time_now),
) )
except HttpResponseException as e: except HttpResponseException as e:
if e.code in [400, 404]: # If an error is received that is due to an unrecognised endpoint,
err = e.to_synapse_error() # fallback to the v1 endpoint. Otherwise consider it a legitmate error
# and raise.
# If we receive an error response that isn't a generic error, or an if not self._is_unknown_endpoint(e):
# unrecognised endpoint error, we assume that the remote understands raise
# the v2 invite API and this is a legitimate error.
if err.errcode not in [Codes.UNKNOWN, Codes.UNRECOGNIZED]:
raise err
else:
raise e.to_synapse_error()
logger.debug("Couldn't send_leave with the v2 API, falling back to the v1 API") logger.debug("Couldn't send_leave with the v2 API, falling back to the v1 API")