From 9e83521af860cb33a7459dbe74188ce5ef39f446 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 28 Feb 2022 07:52:44 -0500 Subject: [PATCH] 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. --- changelog.d/12077.bugfix | 1 + synapse/federation/federation_client.py | 22 +++++++++++++--------- 2 files changed, 14 insertions(+), 9 deletions(-) create mode 100644 changelog.d/12077.bugfix diff --git a/changelog.d/12077.bugfix b/changelog.d/12077.bugfix new file mode 100644 index 000000000..1bce82082 --- /dev/null +++ b/changelog.d/12077.bugfix @@ -0,0 +1 @@ +Fix a long-standing bug where Synapse would make additional failing requests over federation for missing data. diff --git a/synapse/federation/federation_client.py b/synapse/federation/federation_client.py index 2121e92e3..a4bae3c4c 100644 --- a/synapse/federation/federation_client.py +++ b/synapse/federation/federation_client.py @@ -615,11 +615,15 @@ class FederationClient(FederationBase): 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 + # 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. - 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( self, @@ -1002,7 +1006,7 @@ class FederationClient(FederationBase): ) except HttpResponseException as e: # 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. if not self._is_unknown_endpoint(e): raise @@ -1071,7 +1075,7 @@ class FederationClient(FederationBase): except HttpResponseException as e: # 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. - # Otherwise consider it a legitmate error and raise. + # Otherwise, consider it a legitimate error and raise. err = e.to_synapse_error() if self._is_unknown_endpoint(e, err): if room_version.event_format != EventFormatVersions.V1: @@ -1132,7 +1136,7 @@ class FederationClient(FederationBase): ) except HttpResponseException as e: # 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. if not self._is_unknown_endpoint(e): raise @@ -1458,8 +1462,8 @@ class FederationClient(FederationBase): ) except HttpResponseException as e: # If an error is received that is due to an unrecognised endpoint, - # fallback to the unstable endpoint. Otherwise consider it a - # legitmate error and raise. + # fallback to the unstable endpoint. Otherwise, consider it a + # legitimate error and raise. if not self._is_unknown_endpoint(e): raise