Properly failover for unknown endpoints from Conduit/Dendrite. (#12077)

Before this fix, a legitimate 404 from a federation endpoint (e.g. due
to an unknown room) would be treated as an unknown endpoint. This
could cause unnecessary federation traffic.
This commit is contained in:
Patrick Cloke 2022-02-28 07:52:44 -05:00 committed by GitHub
parent 02d708568b
commit 9e83521af8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 14 additions and 9 deletions

1
changelog.d/12077.bugfix Normal file
View File

@ -0,0 +1 @@
Fix a long-standing bug where Synapse would make additional failing requests over federation for missing data.

View File

@ -615,11 +615,15 @@ class FederationClient(FederationBase):
synapse_error = e.to_synapse_error() synapse_error = e.to_synapse_error()
# There is no good way to detect an "unknown" endpoint. # There is no good way to detect an "unknown" endpoint.
# #
# Dendrite returns a 404 (with no body); synapse returns a 400 # Dendrite returns a 404 (with a body of "404 page not found");
# Conduit returns a 404 (with no body); and Synapse returns a 400
# with M_UNRECOGNISED. # with M_UNRECOGNISED.
return e.code == 404 or ( #
e.code == 400 and synapse_error.errcode == Codes.UNRECOGNIZED # This needs to be rather specific as some endpoints truly do return 404
) # errors.
return (
e.code == 404 and (not e.response or e.response == b"404 page not found")
) or (e.code == 400 and synapse_error.errcode == Codes.UNRECOGNIZED)
async def _try_destination_list( async def _try_destination_list(
self, self,
@ -1002,7 +1006,7 @@ class FederationClient(FederationBase):
) )
except HttpResponseException as e: except HttpResponseException as e:
# If an error is received that is due to an unrecognised endpoint, # If an error is received that is due to an unrecognised endpoint,
# fallback to the v1 endpoint. Otherwise consider it a legitmate error # fallback to the v1 endpoint. Otherwise, consider it a legitimate error
# and raise. # and raise.
if not self._is_unknown_endpoint(e): if not self._is_unknown_endpoint(e):
raise raise
@ -1071,7 +1075,7 @@ class FederationClient(FederationBase):
except HttpResponseException as e: except HttpResponseException as e:
# If an error is received that is due to an unrecognised endpoint, # If an error is received that is due to an unrecognised endpoint,
# fallback to the v1 endpoint if the room uses old-style event IDs. # fallback to the v1 endpoint if the room uses old-style event IDs.
# Otherwise consider it a legitmate error and raise. # Otherwise, consider it a legitimate error and raise.
err = e.to_synapse_error() err = e.to_synapse_error()
if self._is_unknown_endpoint(e, err): if self._is_unknown_endpoint(e, err):
if room_version.event_format != EventFormatVersions.V1: if room_version.event_format != EventFormatVersions.V1:
@ -1132,7 +1136,7 @@ class FederationClient(FederationBase):
) )
except HttpResponseException as e: except HttpResponseException as e:
# If an error is received that is due to an unrecognised endpoint, # If an error is received that is due to an unrecognised endpoint,
# fallback to the v1 endpoint. Otherwise consider it a legitmate error # fallback to the v1 endpoint. Otherwise, consider it a legitimate error
# and raise. # and raise.
if not self._is_unknown_endpoint(e): if not self._is_unknown_endpoint(e):
raise raise
@ -1458,8 +1462,8 @@ class FederationClient(FederationBase):
) )
except HttpResponseException as e: except HttpResponseException as e:
# If an error is received that is due to an unrecognised endpoint, # If an error is received that is due to an unrecognised endpoint,
# fallback to the unstable endpoint. Otherwise consider it a # fallback to the unstable endpoint. Otherwise, consider it a
# legitmate error and raise. # legitimate error and raise.
if not self._is_unknown_endpoint(e): if not self._is_unknown_endpoint(e):
raise raise