From 64ff11019e11a712278fdb4fbc0a5f8307f83ddb Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 8 Mar 2019 18:22:47 +0000 Subject: Retry certain federation requests on 404 --- synapse/http/matrixfederationclient.py | 45 ++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) (limited to 'synapse/http/matrixfederationclient.py') diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index 1682c9af13..8776639d6a 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -643,6 +643,51 @@ class MatrixFederationHttpClient(object): ) defer.returnValue(body) + @defer.inlineCallbacks + def get_json_with_trailing_slashes_on_404(self, args={}): + """Runs client.get_json under the hood, but if receiving a 404, tries + the request again with a trailing slash. This is a result of removing + trailing slashes from some federation endpoints and in an effort to + remain backwards compatible with older versions of Synapse, we try + again if a server requires a trailing slash. + + Args: + args (dict): A dictionary of arguments matching those provided by put_json. + Returns: + Deferred[dict|list]: Succeeds when we get a 2xx HTTP response. The + result will be the decoded JSON body. + """ + response = yield self.get_json(**args) + + # Retry with a trailing slash if we received a 404 + if response.code == 404: + args["path"] += "/" + response = yield self.get_json(**args) + + defer.returnValue(response) + + @defer.inlineCallbacks + def put_json_with_trailing_slashes_on_404(self, args={}): + """Runs client.put_json under the hood, but if receiving a 404, tries + the request again with a trailing slash. + + See get_json_with_trailing_slashes_on_404 for more details. + + Args: + args (dict): A dictionary of arguments matching those provided by put_json. + Returns: + Deferred[dict|list]: Succeeds when we get a 2xx HTTP response. The + result will be the decoded JSON body. + """ + response = yield self.put_json(**args) + + # Retry with a trailing slash if we received a 404 + if response.code == 404: + args["path"] += "/" + response = yield self.put_json(**args) + + defer.returnValue(response) + @defer.inlineCallbacks def delete_json(self, destination, path, long_retries=False, timeout=None, ignore_backoff=False, args={}): -- cgit 1.4.1 From 0ea8582f8bd83fc9f4cba80c0462f44583e059a3 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 12 Mar 2019 14:11:11 +0000 Subject: Cleaner way of implementing trailing slashes --- synapse/federation/transport/client.py | 15 +++-- synapse/http/matrixfederationclient.py | 115 +++++++++++++++------------------ tests/handlers/test_typing.py | 6 +- 3 files changed, 66 insertions(+), 70 deletions(-) (limited to 'synapse/http/matrixfederationclient.py') diff --git a/synapse/federation/transport/client.py b/synapse/federation/transport/client.py index 68808e9203..37f37b3d41 100644 --- a/synapse/federation/transport/client.py +++ b/synapse/federation/transport/client.py @@ -52,8 +52,9 @@ class TransportLayerClient(object): destination, room_id) path = _create_v1_path("/state/%s", room_id) - return self.client.get_json_with_trailing_slashes_on_404( + return self.client.get_json( destination, path=path, args={"event_id": event_id}, + try_trailing_slash_on_404=True, ) @log_function @@ -74,8 +75,9 @@ class TransportLayerClient(object): destination, room_id) path = _create_v1_path("/state_ids/%s", room_id) - return self.client.get_json_with_trailing_slashes_on_404( + return self.client.get_json( destination, path=path, args={"event_id": event_id}, + try_trailing_slash_on_404=True, ) @log_function @@ -96,8 +98,9 @@ class TransportLayerClient(object): destination, event_id) path = _create_v1_path("/event/%s", event_id) - return self.client.get_json_with_trailing_slashes_on_404( + return self.client.get_json( destination, path=path, timeout=timeout, + try_trailing_slash_on_404=True, ) @log_function @@ -130,10 +133,11 @@ class TransportLayerClient(object): "limit": [str(limit)], } - return self.client.get_json_with_trailing_slashes_on_404( + return self.client.get_json( destination, path=path, args=args, + try_trailing_slash_on_404=True, ) @defer.inlineCallbacks @@ -171,13 +175,14 @@ class TransportLayerClient(object): path = _create_v1_path("/send/%s", transaction.transaction_id) - response = yield self.client.put_json_with_trailing_slashes_on_404( + response = yield self.client.put_json( transaction.destination, path=path, data=json_data, json_data_callback=json_data_callback, long_retries=True, backoff_on_404=True, # If we get a 404 the other side has gone + try_trailing_slash_on_404=True, ) defer.returnValue(response) diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index 8776639d6a..fec7f5f882 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -196,7 +196,8 @@ class MatrixFederationHttpClient(object): timeout=None, long_retries=False, ignore_backoff=False, - backoff_on_404=False + backoff_on_404=False, + try_trailing_slash_on_404=False, ): """ Sends a request to the given server. @@ -212,6 +213,11 @@ class MatrixFederationHttpClient(object): backoff_on_404 (bool): Back off if we get a 404 + try_trailing_slash_on_404 (bool): True if on a 404 response we + should try appending a trailing slash to the end of the + request. This will be attempted before backing off if backing + off has been enabled. + Returns: Deferred[twisted.web.client.Response]: resolves with the HTTP response object on success. @@ -473,7 +479,8 @@ class MatrixFederationHttpClient(object): json_data_callback=None, long_retries=False, timeout=None, ignore_backoff=False, - backoff_on_404=False): + backoff_on_404=False, + try_trailing_slash_on_404=False): """ Sends the specifed json data using PUT Args: @@ -493,7 +500,11 @@ class MatrixFederationHttpClient(object): and try the request anyway. backoff_on_404 (bool): True if we should count a 404 response as a failure of the server (and should therefore back off future - requests) + requests). + try_trailing_slash_on_404 (bool): True if on a 404 response we + should try appending a trailing slash to the end of the + request. This will be attempted before backing off if backing + off has been enabled. Returns: Deferred[dict|list]: Succeeds when we get a 2xx HTTP response. The @@ -509,7 +520,6 @@ class MatrixFederationHttpClient(object): RequestSendFailed: If there were problems connecting to the remote, due to e.g. DNS failures, connection timeouts etc. """ - request = MatrixFederationRequest( method="PUT", destination=destination, @@ -519,13 +529,26 @@ class MatrixFederationHttpClient(object): json=data, ) - response = yield self._send_request( - request, - long_retries=long_retries, - timeout=timeout, - ignore_backoff=ignore_backoff, - backoff_on_404=backoff_on_404, - ) + send_request_args = { + "request": request, + "long_retries": long_retries, + "timeout": timeout, + "ignore_backoff": ignore_backoff, + # Do not backoff on the initial request if we're trying with trailing slashes + # Otherwise we may end up waiting to contact a server that is actually up + "backoff_on_404": False if try_trailing_slash_on_404 else backoff_on_404, + } + + response = yield self._send_request(**send_request_args) + + # If enabled, retry with a trailing slash if we received a 404 + if try_trailing_slash_on_404 and response.code == 404: + args["path"] += "/" + + # Re-enable backoff if enabled + send_request_args["backoff_on_404"] = backoff_on_404 + + response = yield self.get_json(**send_request_args) body = yield _handle_json_response( self.hs.get_reactor(), self.default_timeout, request, response, @@ -592,7 +615,8 @@ class MatrixFederationHttpClient(object): @defer.inlineCallbacks def get_json(self, destination, path, args=None, retry_on_dns_fail=True, - timeout=None, ignore_backoff=False): + timeout=None, ignore_backoff=False, + try_trailing_slash_on_404=False): """ GETs some json from the given host homeserver and path Args: @@ -606,6 +630,9 @@ class MatrixFederationHttpClient(object): be retried. ignore_backoff (bool): true to ignore the historical backoff data and try the request anyway. + try_trailing_slash_on_404 (bool): True if on a 404 response we + should try appending a trailing slash to the end of the + request. Returns: Deferred[dict|list]: Succeeds when we get a 2xx HTTP response. The result will be the decoded JSON body. @@ -631,63 +658,25 @@ class MatrixFederationHttpClient(object): query=args, ) - response = yield self._send_request( - request, - retry_on_dns_fail=retry_on_dns_fail, - timeout=timeout, - ignore_backoff=ignore_backoff, - ) + send_request_args = { + "request": request, + "retry_on_dns_fail": retry_on_dns_fail, + "timeout": timeout, + "ignore_backoff": ignore_backoff, + } + + response = yield self._send_request(**send_request_args) + + # If enabled, retry with a trailing slash if we received a 404 + if try_trailing_slash_on_404 and response.code == 404: + args["path"] += "/" + response = yield self._send_request(**send_request_args) body = yield _handle_json_response( self.hs.get_reactor(), self.default_timeout, request, response, ) defer.returnValue(body) - @defer.inlineCallbacks - def get_json_with_trailing_slashes_on_404(self, args={}): - """Runs client.get_json under the hood, but if receiving a 404, tries - the request again with a trailing slash. This is a result of removing - trailing slashes from some federation endpoints and in an effort to - remain backwards compatible with older versions of Synapse, we try - again if a server requires a trailing slash. - - Args: - args (dict): A dictionary of arguments matching those provided by put_json. - Returns: - Deferred[dict|list]: Succeeds when we get a 2xx HTTP response. The - result will be the decoded JSON body. - """ - response = yield self.get_json(**args) - - # Retry with a trailing slash if we received a 404 - if response.code == 404: - args["path"] += "/" - response = yield self.get_json(**args) - - defer.returnValue(response) - - @defer.inlineCallbacks - def put_json_with_trailing_slashes_on_404(self, args={}): - """Runs client.put_json under the hood, but if receiving a 404, tries - the request again with a trailing slash. - - See get_json_with_trailing_slashes_on_404 for more details. - - Args: - args (dict): A dictionary of arguments matching those provided by put_json. - Returns: - Deferred[dict|list]: Succeeds when we get a 2xx HTTP response. The - result will be the decoded JSON body. - """ - response = yield self.put_json(**args) - - # Retry with a trailing slash if we received a 404 - if response.code == 404: - args["path"] += "/" - response = yield self.put_json(**args) - - defer.returnValue(response) - @defer.inlineCallbacks def delete_json(self, destination, path, long_retries=False, timeout=None, ignore_backoff=False, args={}): diff --git a/tests/handlers/test_typing.py b/tests/handlers/test_typing.py index 5a3670c939..e17abf2fb4 100644 --- a/tests/handlers/test_typing.py +++ b/tests/handlers/test_typing.py @@ -177,7 +177,7 @@ class TypingNotificationsTestCase(unittest.HomeserverTestCase): timeout=20000, )) - put_json = self.hs.get_http_client().put_json_with_trailing_slashes_on_404 + put_json = self.hs.get_http_client().put_json put_json.assert_called_once_with( "farm", path="/_matrix/federation/v1/send/1000000", @@ -192,6 +192,7 @@ class TypingNotificationsTestCase(unittest.HomeserverTestCase): json_data_callback=ANY, long_retries=True, backoff_on_404=True, + trailing_slashes_on_404=True, ) def test_started_typing_remote_recv(self): @@ -254,7 +255,7 @@ class TypingNotificationsTestCase(unittest.HomeserverTestCase): [call('typing_key', 1, rooms=[ROOM_ID])] ) - put_json = self.hs.get_http_client().put_json_with_trailing_slashes_on_404 + put_json = self.hs.get_http_client().put_json put_json.assert_called_once_with( "farm", path="/_matrix/federation/v1/send/1000000", @@ -269,6 +270,7 @@ class TypingNotificationsTestCase(unittest.HomeserverTestCase): json_data_callback=ANY, long_retries=True, backoff_on_404=True, + trailing_slashes_on_404=True, ) self.assertEquals(self.event_source.get_current_key(), 1) -- cgit 1.4.1 From cf301e37d873fcc00a9fb2d5efffd8ff7ad1b372 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 13 Mar 2019 11:14:43 +0000 Subject: Add workaround note --- synapse/http/matrixfederationclient.py | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) (limited to 'synapse/http/matrixfederationclient.py') diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index fec7f5f882..d22051a47d 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -197,7 +197,6 @@ class MatrixFederationHttpClient(object): long_retries=False, ignore_backoff=False, backoff_on_404=False, - try_trailing_slash_on_404=False, ): """ Sends a request to the given server. @@ -213,11 +212,6 @@ class MatrixFederationHttpClient(object): backoff_on_404 (bool): Back off if we get a 404 - try_trailing_slash_on_404 (bool): True if on a 404 response we - should try appending a trailing slash to the end of the - request. This will be attempted before backing off if backing - off has been enabled. - Returns: Deferred[twisted.web.client.Response]: resolves with the HTTP response object on success. @@ -502,9 +496,9 @@ class MatrixFederationHttpClient(object): a failure of the server (and should therefore back off future requests). try_trailing_slash_on_404 (bool): True if on a 404 response we - should try appending a trailing slash to the end of the - request. This will be attempted before backing off if backing - off has been enabled. + should try appending a trailing slash to the end of the request. + Workaround for #3622 in Synapse <0.99.2. This will be attempted + before backing off if backing off has been enabled. Returns: Deferred[dict|list]: Succeeds when we get a 2xx HTTP response. The @@ -632,7 +626,7 @@ class MatrixFederationHttpClient(object): and try the request anyway. try_trailing_slash_on_404 (bool): True if on a 404 response we should try appending a trailing slash to the end of the - request. + request. Workaround for #3622 in Synapse <0.99.2. Returns: Deferred[dict|list]: Succeeds when we get a 2xx HTTP response. The result will be the decoded JSON body. -- cgit 1.4.1 From 7e75d9644bfb5d687c3b789ad2279da58e09ed7b Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 13 Mar 2019 11:15:23 +0000 Subject: Fix paranthesis indent --- synapse/http/matrixfederationclient.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'synapse/http/matrixfederationclient.py') diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index d22051a47d..7efa7b7572 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -474,7 +474,8 @@ class MatrixFederationHttpClient(object): long_retries=False, timeout=None, ignore_backoff=False, backoff_on_404=False, - try_trailing_slash_on_404=False): + try_trailing_slash_on_404=False, + ): """ Sends the specifed json data using PUT Args: -- cgit 1.4.1 From 7d053cfe10c7be4327b774c6552d21594dcdd2be Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 13 Mar 2019 12:10:33 +0000 Subject: Retry on 400:M_UNRECOGNIZED --- synapse/http/matrixfederationclient.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) (limited to 'synapse/http/matrixfederationclient.py') diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index 7efa7b7572..fc8cf92067 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -474,8 +474,7 @@ class MatrixFederationHttpClient(object): long_retries=False, timeout=None, ignore_backoff=False, backoff_on_404=False, - try_trailing_slash_on_404=False, - ): + try_trailing_slash_on_404=False): """ Sends the specifed json data using PUT Args: @@ -662,14 +661,19 @@ class MatrixFederationHttpClient(object): response = yield self._send_request(**send_request_args) + body = yield _handle_json_response( + self.hs.get_reactor(), self.default_timeout, request, response, + ) + # If enabled, retry with a trailing slash if we received a 404 - if try_trailing_slash_on_404 and response.code == 404: + # or if a 400 with "M_UNRECOGNIZED" which some endpoints return + if (try_trailing_slash_on_404 and + (response.code == 404 + or (response.code == 400 + and body.get("errcode") == "M_UNRECOGNIZED"))): args["path"] += "/" response = yield self._send_request(**send_request_args) - body = yield _handle_json_response( - self.hs.get_reactor(), self.default_timeout, request, response, - ) defer.returnValue(body) @defer.inlineCallbacks -- cgit 1.4.1 From 09626bfd39615e25c735ae5d17ad650aca5fac84 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 13 Mar 2019 18:26:06 +0000 Subject: Switch to wrapper function around _send_request --- synapse/federation/transport/client.py | 10 ++-- synapse/http/matrixfederationclient.py | 103 +++++++++++++++++++++++---------- tests/handlers/test_typing.py | 4 +- 3 files changed, 78 insertions(+), 39 deletions(-) (limited to 'synapse/http/matrixfederationclient.py') diff --git a/synapse/federation/transport/client.py b/synapse/federation/transport/client.py index 37f37b3d41..e424c40fdf 100644 --- a/synapse/federation/transport/client.py +++ b/synapse/federation/transport/client.py @@ -54,7 +54,7 @@ class TransportLayerClient(object): path = _create_v1_path("/state/%s", room_id) return self.client.get_json( destination, path=path, args={"event_id": event_id}, - try_trailing_slash_on_404=True, + try_trailing_slash_on_400=True, ) @log_function @@ -77,7 +77,7 @@ class TransportLayerClient(object): path = _create_v1_path("/state_ids/%s", room_id) return self.client.get_json( destination, path=path, args={"event_id": event_id}, - try_trailing_slash_on_404=True, + try_trailing_slash_on_400=True, ) @log_function @@ -100,7 +100,7 @@ class TransportLayerClient(object): path = _create_v1_path("/event/%s", event_id) return self.client.get_json( destination, path=path, timeout=timeout, - try_trailing_slash_on_404=True, + try_trailing_slash_on_400=True, ) @log_function @@ -137,7 +137,7 @@ class TransportLayerClient(object): destination, path=path, args=args, - try_trailing_slash_on_404=True, + try_trailing_slash_on_400=True, ) @defer.inlineCallbacks @@ -182,7 +182,7 @@ class TransportLayerClient(object): json_data_callback=json_data_callback, long_retries=True, backoff_on_404=True, # If we get a 404 the other side has gone - try_trailing_slash_on_404=True, + try_trailing_slash_on_400=True, ) defer.returnValue(response) diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index fc8cf92067..9019c8791a 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -188,6 +188,55 @@ class MatrixFederationHttpClient(object): self._cooperator = Cooperator(scheduler=schedule) + @defer.inlineCallbacks + def _send_request_with_optional_trailing_slash( + request, + try_trailing_slash_on_400=False, + backoff_on_404=False, + **kwargs, + ): + """Wrapper for _send_request which can optionally retry the request + upon receiving a combination of a 400 HTTP response code and a + 'M_UNRECOGNIZED' errcode. This is a workaround for Synapse <=v0.99.2 + due to #3622. + + Args: + request + try_trailing_slash_on_400 (bool): Whether on receiving a 400 + 'M_UNRECOGNIZED' from the server to retry the request with a + trailing slash appended to the request path. + backoff_on_404 (bool): Whether to backoff on 404 when making a + request with a trailing slash (only affects request if + try_trailing_slash_on_400 is True). + kwargs (Dict): A dictionary of arguments to pass to + `_send_request()`. + + Returns: + Deferred[twisted.web.client.Response]: resolves with the HTTP + response object on success. + """ + response = self._send_request(**kwargs) + + if not try_trailing_slash_on_400: + defer.returnValue(response) + + # Check if it's necessary to retry with a trailing slash + body = yield _handle_json_response( + self.hs.get_reactor(), self.default_timeout, request, response, + ) + + # Retry with a trailing slash if we received a 400 with + # 'M_UNRECOGNIZED' which some endpoints can return when omitting a + # trailing slash on Synapse <=v0.99.2. + if (response.code == 400 and body.get("errcode") == "M_UNRECOGNIZED"): + # Enable backoff if initially disabled + kwargs["backoff_on_404"] = backoff_on_404 + + kwargs["path"] += "/" + response = yield self._send_request(**kwargs) + + defer.returnValue(response) + @defer.inlineCallbacks def _send_request( self, @@ -474,7 +523,7 @@ class MatrixFederationHttpClient(object): long_retries=False, timeout=None, ignore_backoff=False, backoff_on_404=False, - try_trailing_slash_on_404=False): + try_trailing_slash_on_400=False): """ Sends the specifed json data using PUT Args: @@ -495,10 +544,11 @@ class MatrixFederationHttpClient(object): backoff_on_404 (bool): True if we should count a 404 response as a failure of the server (and should therefore back off future requests). - try_trailing_slash_on_404 (bool): True if on a 404 response we - should try appending a trailing slash to the end of the request. - Workaround for #3622 in Synapse <0.99.2. This will be attempted - before backing off if backing off has been enabled. + try_trailing_slash_on_400 (bool): True if on a 400 M_UNRECOGNIZED + response we should try appending a trailing slash to the end + of the request. Workaround for #3622 in Synapse <=v0.99.2. This + will be attempted before backing off if backing off has been + enabled. Returns: Deferred[dict|list]: Succeeds when we get a 2xx HTTP response. The @@ -528,21 +578,21 @@ class MatrixFederationHttpClient(object): "long_retries": long_retries, "timeout": timeout, "ignore_backoff": ignore_backoff, - # Do not backoff on the initial request if we're trying with trailing slashes - # Otherwise we may end up waiting to contact a server that is actually up - "backoff_on_404": False if try_trailing_slash_on_404 else backoff_on_404, + # Do not backoff on the initial request if we're trying again with + # trailing slashes Otherwise we may end up waiting to contact a + # server that is actually up + "backoff_on_404": False if try_trailing_slash_on_400 else backoff_on_404, } - response = yield self._send_request(**send_request_args) + response = yield self._send_request_with_optional_trailing_slash( + request, try_trailing_slash_on_400, backoff_on_404, **send_request_args, + ) - # If enabled, retry with a trailing slash if we received a 404 - if try_trailing_slash_on_404 and response.code == 404: + # If enabled, retry with a trailing slash if we received a 400 + if try_trailing_slash_on_400 and response.code == 400: args["path"] += "/" - # Re-enable backoff if enabled - send_request_args["backoff_on_404"] = backoff_on_404 - - response = yield self.get_json(**send_request_args) + response = yield self._send_request(**send_request_args) body = yield _handle_json_response( self.hs.get_reactor(), self.default_timeout, request, response, @@ -610,7 +660,7 @@ class MatrixFederationHttpClient(object): @defer.inlineCallbacks def get_json(self, destination, path, args=None, retry_on_dns_fail=True, timeout=None, ignore_backoff=False, - try_trailing_slash_on_404=False): + try_trailing_slash_on_400=False): """ GETs some json from the given host homeserver and path Args: @@ -624,9 +674,9 @@ class MatrixFederationHttpClient(object): be retried. ignore_backoff (bool): true to ignore the historical backoff data and try the request anyway. - try_trailing_slash_on_404 (bool): True if on a 404 response we - should try appending a trailing slash to the end of the - request. Workaround for #3622 in Synapse <0.99.2. + try_trailing_slash_on_400 (bool): True if on a 400 M_UNRECOGNIZED + response we should try appending a trailing slash to the end of + the request. Workaround for #3622 in Synapse <=v0.99.2. Returns: Deferred[dict|list]: Succeeds when we get a 2xx HTTP response. The result will be the decoded JSON body. @@ -659,21 +709,10 @@ class MatrixFederationHttpClient(object): "ignore_backoff": ignore_backoff, } - response = yield self._send_request(**send_request_args) - - body = yield _handle_json_response( - self.hs.get_reactor(), self.default_timeout, request, response, + response = yield self._send_request_with_optional_trailing_slash( + request, try_trailing_slash_on_400, False, **send_request_args, ) - # If enabled, retry with a trailing slash if we received a 404 - # or if a 400 with "M_UNRECOGNIZED" which some endpoints return - if (try_trailing_slash_on_404 and - (response.code == 404 - or (response.code == 400 - and body.get("errcode") == "M_UNRECOGNIZED"))): - args["path"] += "/" - response = yield self._send_request(**send_request_args) - defer.returnValue(body) @defer.inlineCallbacks diff --git a/tests/handlers/test_typing.py b/tests/handlers/test_typing.py index 244a0bc80c..6460cbc708 100644 --- a/tests/handlers/test_typing.py +++ b/tests/handlers/test_typing.py @@ -192,7 +192,7 @@ class TypingNotificationsTestCase(unittest.HomeserverTestCase): json_data_callback=ANY, long_retries=True, backoff_on_404=True, - try_trailing_slash_on_404=True, + try_trailing_slash_on_400=True, ) def test_started_typing_remote_recv(self): @@ -270,7 +270,7 @@ class TypingNotificationsTestCase(unittest.HomeserverTestCase): json_data_callback=ANY, long_retries=True, backoff_on_404=True, - try_trailing_slash_on_404=True, + try_trailing_slash_on_400=True, ) self.assertEquals(self.event_source.get_current_key(), 1) -- cgit 1.4.1 From 5526b054aaf4ca4c0b459a19019d41a0cd3a1978 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 13 Mar 2019 18:35:21 +0000 Subject: Fix syntax issues --- synapse/http/matrixfederationclient.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) (limited to 'synapse/http/matrixfederationclient.py') diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index 9019c8791a..5b89a2e05c 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -190,6 +190,7 @@ class MatrixFederationHttpClient(object): @defer.inlineCallbacks def _send_request_with_optional_trailing_slash( + self, request, try_trailing_slash_on_400=False, backoff_on_404=False, @@ -215,7 +216,7 @@ class MatrixFederationHttpClient(object): Deferred[twisted.web.client.Response]: resolves with the HTTP response object on success. """ - response = self._send_request(**kwargs) + response = yield self._send_request(**kwargs) if not try_trailing_slash_on_400: defer.returnValue(response) @@ -225,6 +226,9 @@ class MatrixFederationHttpClient(object): self.hs.get_reactor(), self.default_timeout, request, response, ) + logger.info(" *** BODY IS *** ") + logger.info(body) + # Retry with a trailing slash if we received a 400 with # 'M_UNRECOGNIZED' which some endpoints can return when omitting a # trailing slash on Synapse <=v0.99.2. @@ -588,15 +592,10 @@ class MatrixFederationHttpClient(object): request, try_trailing_slash_on_400, backoff_on_404, **send_request_args, ) - # If enabled, retry with a trailing slash if we received a 400 - if try_trailing_slash_on_400 and response.code == 400: - args["path"] += "/" - - response = yield self._send_request(**send_request_args) - body = yield _handle_json_response( self.hs.get_reactor(), self.default_timeout, request, response, ) + defer.returnValue(body) @defer.inlineCallbacks @@ -713,6 +712,10 @@ class MatrixFederationHttpClient(object): request, try_trailing_slash_on_400, False, **send_request_args, ) + body = yield _handle_json_response( + self.hs.get_reactor(), self.default_timeout, request, response, + ) + defer.returnValue(body) @defer.inlineCallbacks -- cgit 1.4.1 From 660b77f3626583fcc49a76e5c2b3d1143677799b Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 13 Mar 2019 18:38:16 +0000 Subject: Add missing docstring detail --- synapse/http/matrixfederationclient.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'synapse/http/matrixfederationclient.py') diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index 5b89a2e05c..fca6e242be 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -202,7 +202,7 @@ class MatrixFederationHttpClient(object): due to #3622. Args: - request + request (MatrixFederationRequest): details of request to be sent try_trailing_slash_on_400 (bool): Whether on receiving a 400 'M_UNRECOGNIZED' from the server to retry the request with a trailing slash appended to the request path. -- cgit 1.4.1 From 220607a6183a60a62cfa3fbcfcd30fadc0bdff4b Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 13 Mar 2019 18:43:40 +0000 Subject: Remove testing code --- synapse/http/matrixfederationclient.py | 3 --- 1 file changed, 3 deletions(-) (limited to 'synapse/http/matrixfederationclient.py') diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index fca6e242be..380bf294e5 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -226,9 +226,6 @@ class MatrixFederationHttpClient(object): self.hs.get_reactor(), self.default_timeout, request, response, ) - logger.info(" *** BODY IS *** ") - logger.info(body) - # Retry with a trailing slash if we received a 400 with # 'M_UNRECOGNIZED' which some endpoints can return when omitting a # trailing slash on Synapse <=v0.99.2. -- cgit 1.4.1 From 9dd0e34679f711116f39a35466e7dc956b10ab42 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 13 Mar 2019 18:45:17 +0000 Subject: Syntax test --- synapse/http/matrixfederationclient.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'synapse/http/matrixfederationclient.py') diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index 380bf294e5..cdeb3792b2 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -190,12 +190,11 @@ class MatrixFederationHttpClient(object): @defer.inlineCallbacks def _send_request_with_optional_trailing_slash( - self, - request, - try_trailing_slash_on_400=False, - backoff_on_404=False, - **kwargs, - ): + self, + request, + try_trailing_slash_on_400=False, + backoff_on_404=False, + **kwargs): """Wrapper for _send_request which can optionally retry the request upon receiving a combination of a 400 HTTP response code and a 'M_UNRECOGNIZED' errcode. This is a workaround for Synapse <=v0.99.2 -- cgit 1.4.1 From ee8ba397e89d61ec7beeb549391b9ffeaf7e484a Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 13 Mar 2019 18:48:31 +0000 Subject: Are you happy now --- synapse/http/matrixfederationclient.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'synapse/http/matrixfederationclient.py') diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index cdeb3792b2..f554d5e21c 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -585,7 +585,7 @@ class MatrixFederationHttpClient(object): } response = yield self._send_request_with_optional_trailing_slash( - request, try_trailing_slash_on_400, backoff_on_404, **send_request_args, + request, try_trailing_slash_on_400, backoff_on_404, send_request_args, ) body = yield _handle_json_response( @@ -705,7 +705,7 @@ class MatrixFederationHttpClient(object): } response = yield self._send_request_with_optional_trailing_slash( - request, try_trailing_slash_on_400, False, **send_request_args, + request, try_trailing_slash_on_400, False, send_request_args, ) body = yield _handle_json_response( -- cgit 1.4.1 From c2d848b80d8b83646b1a0413057efad910a3df12 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 13 Mar 2019 19:04:43 +0000 Subject: Destructure again --- synapse/http/matrixfederationclient.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) (limited to 'synapse/http/matrixfederationclient.py') diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index f554d5e21c..380bf294e5 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -190,11 +190,12 @@ class MatrixFederationHttpClient(object): @defer.inlineCallbacks def _send_request_with_optional_trailing_slash( - self, - request, - try_trailing_slash_on_400=False, - backoff_on_404=False, - **kwargs): + self, + request, + try_trailing_slash_on_400=False, + backoff_on_404=False, + **kwargs, + ): """Wrapper for _send_request which can optionally retry the request upon receiving a combination of a 400 HTTP response code and a 'M_UNRECOGNIZED' errcode. This is a workaround for Synapse <=v0.99.2 @@ -585,7 +586,7 @@ class MatrixFederationHttpClient(object): } response = yield self._send_request_with_optional_trailing_slash( - request, try_trailing_slash_on_400, backoff_on_404, send_request_args, + request, try_trailing_slash_on_400, backoff_on_404, **send_request_args, ) body = yield _handle_json_response( @@ -705,7 +706,7 @@ class MatrixFederationHttpClient(object): } response = yield self._send_request_with_optional_trailing_slash( - request, try_trailing_slash_on_400, False, send_request_args, + request, try_trailing_slash_on_400, False, **send_request_args, ) body = yield _handle_json_response( -- cgit 1.4.1 From c991e7aec78d7cb299c4f9c16f3b80f03c887bfb Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 13 Mar 2019 19:08:08 +0000 Subject: Syntax checker is bork --- synapse/http/matrixfederationclient.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'synapse/http/matrixfederationclient.py') diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index 380bf294e5..cdeb3792b2 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -190,12 +190,11 @@ class MatrixFederationHttpClient(object): @defer.inlineCallbacks def _send_request_with_optional_trailing_slash( - self, - request, - try_trailing_slash_on_400=False, - backoff_on_404=False, - **kwargs, - ): + self, + request, + try_trailing_slash_on_400=False, + backoff_on_404=False, + **kwargs): """Wrapper for _send_request which can optionally retry the request upon receiving a combination of a 400 HTTP response code and a 'M_UNRECOGNIZED' errcode. This is a workaround for Synapse <=v0.99.2 -- cgit 1.4.1 From bec313818c03f89b501fb8b07efa577f47efee8e Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 13 Mar 2019 19:10:56 +0000 Subject: go home python, you're drunk --- synapse/http/matrixfederationclient.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'synapse/http/matrixfederationclient.py') diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index cdeb3792b2..de6bc4de00 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -585,8 +585,7 @@ class MatrixFederationHttpClient(object): } response = yield self._send_request_with_optional_trailing_slash( - request, try_trailing_slash_on_400, backoff_on_404, **send_request_args, - ) + request, try_trailing_slash_on_400, backoff_on_404, **send_request_args) body = yield _handle_json_response( self.hs.get_reactor(), self.default_timeout, request, response, @@ -705,8 +704,7 @@ class MatrixFederationHttpClient(object): } response = yield self._send_request_with_optional_trailing_slash( - request, try_trailing_slash_on_400, False, **send_request_args, - ) + request, try_trailing_slash_on_400, False, **send_request_args) body = yield _handle_json_response( self.hs.get_reactor(), self.default_timeout, request, response, -- cgit 1.4.1 From 66cdb840a6fdf24431cce50e776d65290c419c86 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 13 Mar 2019 19:18:25 +0000 Subject: Or perhaps I was the one who was drunk --- synapse/http/matrixfederationclient.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) (limited to 'synapse/http/matrixfederationclient.py') diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index de6bc4de00..f596ccfdf4 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -193,7 +193,6 @@ class MatrixFederationHttpClient(object): self, request, try_trailing_slash_on_400=False, - backoff_on_404=False, **kwargs): """Wrapper for _send_request which can optionally retry the request upon receiving a combination of a 400 HTTP response code and a @@ -205,9 +204,6 @@ class MatrixFederationHttpClient(object): try_trailing_slash_on_400 (bool): Whether on receiving a 400 'M_UNRECOGNIZED' from the server to retry the request with a trailing slash appended to the request path. - backoff_on_404 (bool): Whether to backoff on 404 when making a - request with a trailing slash (only affects request if - try_trailing_slash_on_400 is True). kwargs (Dict): A dictionary of arguments to pass to `_send_request()`. @@ -585,7 +581,7 @@ class MatrixFederationHttpClient(object): } response = yield self._send_request_with_optional_trailing_slash( - request, try_trailing_slash_on_400, backoff_on_404, **send_request_args) + request, try_trailing_slash_on_400, **send_request_args) body = yield _handle_json_response( self.hs.get_reactor(), self.default_timeout, request, response, @@ -701,10 +697,11 @@ class MatrixFederationHttpClient(object): "retry_on_dns_fail": retry_on_dns_fail, "timeout": timeout, "ignore_backoff": ignore_backoff, + "backoff_on_404": False, } response = yield self._send_request_with_optional_trailing_slash( - request, try_trailing_slash_on_400, False, **send_request_args) + request, try_trailing_slash_on_400, **send_request_args) body = yield _handle_json_response( self.hs.get_reactor(), self.default_timeout, request, response, -- cgit 1.4.1 From 7c0295f13c00ab1cc613584214af1bfc98041edd Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 13 Mar 2019 19:27:10 +0000 Subject: no kwargs today --- synapse/http/matrixfederationclient.py | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) (limited to 'synapse/http/matrixfederationclient.py') diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index f596ccfdf4..47c7aedfff 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -193,7 +193,9 @@ class MatrixFederationHttpClient(object): self, request, try_trailing_slash_on_400=False, - **kwargs): + backoff_on_404=False, + send_request_args={}, + ): """Wrapper for _send_request which can optionally retry the request upon receiving a combination of a 400 HTTP response code and a 'M_UNRECOGNIZED' errcode. This is a workaround for Synapse <=v0.99.2 @@ -204,14 +206,17 @@ class MatrixFederationHttpClient(object): try_trailing_slash_on_400 (bool): Whether on receiving a 400 'M_UNRECOGNIZED' from the server to retry the request with a trailing slash appended to the request path. - kwargs (Dict): A dictionary of arguments to pass to + 404_backoff (bool): Whether to backoff on 404 when making a + request with a trailing slash (only affects request if + try_trailing_slash_on_400 is True). + send_request_args (Dict): A dictionary of arguments to pass to `_send_request()`. Returns: Deferred[twisted.web.client.Response]: resolves with the HTTP response object on success. """ - response = yield self._send_request(**kwargs) + response = yield self._send_request(**send_request_args) if not try_trailing_slash_on_400: defer.returnValue(response) @@ -226,10 +231,10 @@ class MatrixFederationHttpClient(object): # trailing slash on Synapse <=v0.99.2. if (response.code == 400 and body.get("errcode") == "M_UNRECOGNIZED"): # Enable backoff if initially disabled - kwargs["backoff_on_404"] = backoff_on_404 + send_request_args["backoff_on_404"] = backoff_on_404 - kwargs["path"] += "/" - response = yield self._send_request(**kwargs) + send_request_args["path"] += "/" + response = yield self._send_request(**send_request_args) defer.returnValue(response) @@ -581,7 +586,7 @@ class MatrixFederationHttpClient(object): } response = yield self._send_request_with_optional_trailing_slash( - request, try_trailing_slash_on_400, **send_request_args) + request, try_trailing_slash_on_400, backoff_on_404, send_request_args) body = yield _handle_json_response( self.hs.get_reactor(), self.default_timeout, request, response, -- cgit 1.4.1 From 5ca857ad849784ce72660fce8d8a94bb4aa55d5f Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 13 Mar 2019 19:35:23 +0000 Subject: as above --- synapse/http/matrixfederationclient.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'synapse/http/matrixfederationclient.py') diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index 47c7aedfff..502ff15125 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -586,7 +586,8 @@ class MatrixFederationHttpClient(object): } response = yield self._send_request_with_optional_trailing_slash( - request, try_trailing_slash_on_400, backoff_on_404, send_request_args) + request, try_trailing_slash_on_400, backoff_on_404, send_request_args, + ) body = yield _handle_json_response( self.hs.get_reactor(), self.default_timeout, request, response, @@ -706,7 +707,8 @@ class MatrixFederationHttpClient(object): } response = yield self._send_request_with_optional_trailing_slash( - request, try_trailing_slash_on_400, **send_request_args) + request, try_trailing_slash_on_400, send_request_args, + ) body = yield _handle_json_response( self.hs.get_reactor(), self.default_timeout, request, response, -- cgit 1.4.1 From 26f8e2d099e59a20abf56895b7958f5af853db32 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 13 Mar 2019 19:49:08 +0000 Subject: there comes a time when you should give up. but you dont --- synapse/http/matrixfederationclient.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'synapse/http/matrixfederationclient.py') diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index 502ff15125..269cf21488 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -707,7 +707,7 @@ class MatrixFederationHttpClient(object): } response = yield self._send_request_with_optional_trailing_slash( - request, try_trailing_slash_on_400, send_request_args, + request, try_trailing_slash_on_400, backoff_on_404, send_request_args, ) body = yield _handle_json_response( -- cgit 1.4.1 From 8d16ffaf7a1ab9ae17e59b2d80b82aa65294bf95 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 13 Mar 2019 20:03:10 +0000 Subject: i should have given up --- synapse/http/matrixfederationclient.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'synapse/http/matrixfederationclient.py') diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index 269cf21488..8ad28d1e21 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -707,7 +707,7 @@ class MatrixFederationHttpClient(object): } response = yield self._send_request_with_optional_trailing_slash( - request, try_trailing_slash_on_400, backoff_on_404, send_request_args, + request, try_trailing_slash_on_400, False, send_request_args, ) body = yield _handle_json_response( -- cgit 1.4.1 From 45524f2f5e9dc7eca5c439e4a8bff938abc17ee8 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 13 Mar 2019 20:17:39 +0000 Subject: i should have given up x2 --- synapse/http/matrixfederationclient.py | 31 ++++++++++++--------------- tests/http/test_fedclient.py | 39 ++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 18 deletions(-) (limited to 'synapse/http/matrixfederationclient.py') diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index 8ad28d1e21..74ea6bcf8e 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -190,11 +190,11 @@ class MatrixFederationHttpClient(object): @defer.inlineCallbacks def _send_request_with_optional_trailing_slash( - self, - request, - try_trailing_slash_on_400=False, - backoff_on_404=False, - send_request_args={}, + self, + request, + try_trailing_slash_on_400=False, + backoff_on_404=False, + send_request_args={}, ): """Wrapper for _send_request which can optionally retry the request upon receiving a combination of a 400 HTTP response code and a @@ -213,8 +213,7 @@ class MatrixFederationHttpClient(object): `_send_request()`. Returns: - Deferred[twisted.web.client.Response]: resolves with the HTTP - response object on success. + Deferred[Dict]: Parsed JSON response body. """ response = yield self._send_request(**send_request_args) @@ -236,7 +235,11 @@ class MatrixFederationHttpClient(object): send_request_args["path"] += "/" response = yield self._send_request(**send_request_args) - defer.returnValue(response) + body = yield _handle_json_response( + self.hs.get_reactor(), self.default_timeout, request, response, + ) + + defer.returnValue(body) @defer.inlineCallbacks def _send_request( @@ -585,14 +588,10 @@ class MatrixFederationHttpClient(object): "backoff_on_404": False if try_trailing_slash_on_400 else backoff_on_404, } - response = yield self._send_request_with_optional_trailing_slash( + body = yield self._send_request_with_optional_trailing_slash( request, try_trailing_slash_on_400, backoff_on_404, send_request_args, ) - body = yield _handle_json_response( - self.hs.get_reactor(), self.default_timeout, request, response, - ) - defer.returnValue(body) @defer.inlineCallbacks @@ -706,14 +705,10 @@ class MatrixFederationHttpClient(object): "backoff_on_404": False, } - response = yield self._send_request_with_optional_trailing_slash( + body = yield self._send_request_with_optional_trailing_slash( request, try_trailing_slash_on_400, False, send_request_args, ) - body = yield _handle_json_response( - self.hs.get_reactor(), self.default_timeout, request, response, - ) - defer.returnValue(body) @defer.inlineCallbacks diff --git a/tests/http/test_fedclient.py b/tests/http/test_fedclient.py index b03b37affe..0d0161d13d 100644 --- a/tests/http/test_fedclient.py +++ b/tests/http/test_fedclient.py @@ -268,6 +268,45 @@ class FederationClientTests(HomeserverTestCase): self.assertIsInstance(f.value, TimeoutError) + def test_client_requires_trailing_slashes(self): + """ + If a connection is made to a client but the client rejects it due to + requiring a trailing slash. We need to retry the request with a + trailing slash. Workaround for Synapse <=v0.99.2, explained in #3622. + """ + d = self.cl.get_json( + "testserv:8008", "foo/bar", try_trailing_slash_on_400=True, + ) + + self.pump() + + # there should have been a call to connectTCP + clients = self.reactor.tcpClients + self.assertEqual(len(clients), 1) + (_host, _port, factory, _timeout, _bindAddress) = clients[0] + + # complete the connection and wire it up to a fake transport + client = factory.buildProtocol(None) + conn = StringTransport() + client.makeConnection(conn) + + # that should have made it send the request to the connection + self.assertRegex(conn.value(), b"^GET /foo/bar") + + # Send the HTTP response + client.dataReceived( + b"HTTP/1.1 400 Bad Request\r\n" + b"Content-Type: application/json\r\n" + b"Content-Length: 59\r\n" + b"\r\n" + b'{"errcode":"M_UNRECOGNIZED","error":"Unrecognized request"}' + ) + + # We should get a successful response + r = self.successResultOf(d) + self.assertEqual(r.code, 400) + self.assertEqual(r, {}) + def test_client_sends_body(self): self.cl.post_json( "testserv:8008", "foo/bar", timeout=10000, -- cgit 1.4.1 From 9a2e22fd41845b6fa7f30c66cf89a368af269147 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 13 Mar 2019 20:29:38 +0000 Subject: is this what purgatory feels like --- synapse/http/matrixfederationclient.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'synapse/http/matrixfederationclient.py') diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index 74ea6bcf8e..59e758fefb 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -217,14 +217,14 @@ class MatrixFederationHttpClient(object): """ response = yield self._send_request(**send_request_args) - if not try_trailing_slash_on_400: - defer.returnValue(response) - # Check if it's necessary to retry with a trailing slash body = yield _handle_json_response( self.hs.get_reactor(), self.default_timeout, request, response, ) + if not try_trailing_slash_on_400: + defer.returnValue(body) + # Retry with a trailing slash if we received a 400 with # 'M_UNRECOGNIZED' which some endpoints can return when omitting a # trailing slash on Synapse <=v0.99.2. -- cgit 1.4.1 From b2df0e8e2cb72a2147e6d962a0e8c5ce35233ed4 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 13 Mar 2019 21:08:10 +0000 Subject: receiving a 400 caused an exception. handle it --- synapse/http/matrixfederationclient.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) (limited to 'synapse/http/matrixfederationclient.py') diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index 59e758fefb..6b41639741 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -212,27 +212,34 @@ class MatrixFederationHttpClient(object): send_request_args (Dict): A dictionary of arguments to pass to `_send_request()`. + Raises: + HttpResponseException: If we get an HTTP response code >= 300 + (except 429). + Returns: Deferred[Dict]: Parsed JSON response body. """ - response = yield self._send_request(**send_request_args) + try: + response = yield self._send_request(**send_request_args) + except HttpResponseException as e: + # Received a 400. Raise unless we're retrying + if not try_trailing_slash_on_400: + raise e # Check if it's necessary to retry with a trailing slash body = yield _handle_json_response( self.hs.get_reactor(), self.default_timeout, request, response, ) - if not try_trailing_slash_on_400: - defer.returnValue(body) - # Retry with a trailing slash if we received a 400 with # 'M_UNRECOGNIZED' which some endpoints can return when omitting a # trailing slash on Synapse <=v0.99.2. - if (response.code == 400 and body.get("errcode") == "M_UNRECOGNIZED"): + if not (response.code == 400 and body.get("errcode") == "M_UNRECOGNIZED"): # Enable backoff if initially disabled send_request_args["backoff_on_404"] = backoff_on_404 send_request_args["path"] += "/" + response = yield self._send_request(**send_request_args) body = yield _handle_json_response( -- cgit 1.4.1 From ecea5af491cd69933afe1dbea665d85c84d1d94e Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 13 Mar 2019 21:21:03 +0000 Subject: Correct var name --- synapse/http/matrixfederationclient.py | 4 +-- tests/http/test_fedclient.py | 54 ++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 2 deletions(-) (limited to 'synapse/http/matrixfederationclient.py') diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index 6b41639741..b27c4c1c38 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -238,10 +238,10 @@ class MatrixFederationHttpClient(object): # Enable backoff if initially disabled send_request_args["backoff_on_404"] = backoff_on_404 - send_request_args["path"] += "/" + # Add trailing slash + send_request_args["request"].path += "/" response = yield self._send_request(**send_request_args) - body = yield _handle_json_response( self.hs.get_reactor(), self.default_timeout, request, response, ) diff --git a/tests/http/test_fedclient.py b/tests/http/test_fedclient.py index b03b37affe..b45eee3a82 100644 --- a/tests/http/test_fedclient.py +++ b/tests/http/test_fedclient.py @@ -268,6 +268,60 @@ class FederationClientTests(HomeserverTestCase): self.assertIsInstance(f.value, TimeoutError) + def test_client_requires_trailing_slashes(self): + """ + If a connection is made to a client but the client rejects it due to + requiring a trailing slash. We need to retry the request with a + trailing slash. Workaround for Synapse <=v0.99.2, explained in #3622. + """ + d = self.cl.get_json( + "testserv:8008", "foo/bar", try_trailing_slash_on_400=True, + ) + + self.pump() + + # there should have been a call to connectTCP + clients = self.reactor.tcpClients + self.assertEqual(len(clients), 1) + (_host, _port, factory, _timeout, _bindAddress) = clients[0] + + # complete the connection and wire it up to a fake transport + client = factory.buildProtocol(None) + conn = StringTransport() + client.makeConnection(conn) + + # that should have made it send the request to the connection + self.assertRegex(conn.value(), b"^GET /foo/bar") + + # Send the HTTP response + client.dataReceived( + b"HTTP/1.1 400 Bad Request\r\n" + b"Content-Type: application/json\r\n" + b"Content-Length: 59\r\n" + b"\r\n" + b'{"errcode":"M_UNRECOGNIZED","error":"Unrecognized request"}' + ) + + # We should get a 400 response, then try again + self.pump() + + # We should get another request wiht a trailing slash + self.assertRegex(conn.value(), b"^GET /foo/bar/") + + # Send a happy response this time + client.dataReceived( + b"HTTP/1.1 200 OK\r\n" + b"Content-Type: application/json\r\n" + b"Content-Length: 2\r\n" + b"\r\n" + b'{}' + ) + + # We should get a successful response + r = self.successResultOf(d) + self.assertEqual(r.code, 200) + self.assertEqual(r, {}) + def test_client_sends_body(self): self.cl.post_json( "testserv:8008", "foo/bar", timeout=10000, -- cgit 1.4.1 From 621e7f37f1a7f32ff5046060f17a1da825f9ff8b Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 18 Mar 2019 17:45:54 +0000 Subject: Better exception handling --- synapse/http/matrixfederationclient.py | 33 +++++++++++++++++---------------- tests/http/test_fedclient.py | 5 +---- 2 files changed, 18 insertions(+), 20 deletions(-) (limited to 'synapse/http/matrixfederationclient.py') diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index b27c4c1c38..3c27686a89 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -221,30 +221,31 @@ class MatrixFederationHttpClient(object): """ try: response = yield self._send_request(**send_request_args) + + # Check if it's necessary to retry with a trailing slash + body = yield _handle_json_response( + self.hs.get_reactor(), self.default_timeout, request, response, + ) except HttpResponseException as e: - # Received a 400. Raise unless we're retrying if not try_trailing_slash_on_400: + # Received an error >= 300. Raise unless we're retrying raise e - - # Check if it's necessary to retry with a trailing slash - body = yield _handle_json_response( - self.hs.get_reactor(), self.default_timeout, request, response, - ) + except: + raise e # Retry with a trailing slash if we received a 400 with # 'M_UNRECOGNIZED' which some endpoints can return when omitting a - # trailing slash on Synapse <=v0.99.2. - if not (response.code == 400 and body.get("errcode") == "M_UNRECOGNIZED"): - # Enable backoff if initially disabled - send_request_args["backoff_on_404"] = backoff_on_404 + # trailing slash on Synapse <= v0.99.2. + # Enable backoff if initially disabled + send_request_args["backoff_on_404"] = backoff_on_404 - # Add trailing slash - send_request_args["request"].path += "/" + # Add trailing slash + send_request_args["request"].path += "/" - response = yield self._send_request(**send_request_args) - body = yield _handle_json_response( - self.hs.get_reactor(), self.default_timeout, request, response, - ) + response = yield self._send_request(**send_request_args) + body = yield _handle_json_response( + self.hs.get_reactor(), self.default_timeout, request, response, + ) defer.returnValue(body) diff --git a/tests/http/test_fedclient.py b/tests/http/test_fedclient.py index b45eee3a82..84216db44f 100644 --- a/tests/http/test_fedclient.py +++ b/tests/http/test_fedclient.py @@ -272,7 +272,7 @@ class FederationClientTests(HomeserverTestCase): """ If a connection is made to a client but the client rejects it due to requiring a trailing slash. We need to retry the request with a - trailing slash. Workaround for Synapse <=v0.99.2, explained in #3622. + trailing slash. Workaround for Synapse <= v0.99.2, explained in #3622. """ d = self.cl.get_json( "testserv:8008", "foo/bar", try_trailing_slash_on_400=True, @@ -302,9 +302,6 @@ class FederationClientTests(HomeserverTestCase): b'{"errcode":"M_UNRECOGNIZED","error":"Unrecognized request"}' ) - # We should get a 400 response, then try again - self.pump() - # We should get another request wiht a trailing slash self.assertRegex(conn.value(), b"^GET /foo/bar/") -- cgit 1.4.1 From a8ad39eec74ff973f6bfe8d84dfd8d2d3ca32913 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 18 Mar 2019 17:47:39 +0000 Subject: lint --- synapse/http/matrixfederationclient.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'synapse/http/matrixfederationclient.py') diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index 3c27686a89..2be0244870 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -230,7 +230,7 @@ class MatrixFederationHttpClient(object): if not try_trailing_slash_on_400: # Received an error >= 300. Raise unless we're retrying raise e - except: + except Exception as e: raise e # Retry with a trailing slash if we received a 400 with -- cgit 1.4.1 From 551ea1155966c023d7d2eb41454598bf14cb4be2 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 20 Mar 2019 11:07:36 +0000 Subject: Just return if not doing any trailing slash shennanigans --- synapse/http/matrixfederationclient.py | 2 ++ 1 file changed, 2 insertions(+) (limited to 'synapse/http/matrixfederationclient.py') diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index 2be0244870..1307508e5a 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -226,6 +226,8 @@ class MatrixFederationHttpClient(object): body = yield _handle_json_response( self.hs.get_reactor(), self.default_timeout, request, response, ) + + defer.returnValue(body) except HttpResponseException as e: if not try_trailing_slash_on_400: # Received an error >= 300. Raise unless we're retrying -- cgit 1.4.1 From c69df5d5d37652b98d670a25c8c4291002af9242 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 20 Mar 2019 11:27:18 +0000 Subject: Fix comments. v0.99.2 -> v0.99.3 --- synapse/http/matrixfederationclient.py | 10 +++++----- tests/http/test_fedclient.py | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) (limited to 'synapse/http/matrixfederationclient.py') diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index 1307508e5a..8312e4fc3f 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -198,7 +198,7 @@ class MatrixFederationHttpClient(object): ): """Wrapper for _send_request which can optionally retry the request upon receiving a combination of a 400 HTTP response code and a - 'M_UNRECOGNIZED' errcode. This is a workaround for Synapse <=v0.99.2 + 'M_UNRECOGNIZED' errcode. This is a workaround for Synapse <= v0.99.3 due to #3622. Args: @@ -237,7 +237,7 @@ class MatrixFederationHttpClient(object): # Retry with a trailing slash if we received a 400 with # 'M_UNRECOGNIZED' which some endpoints can return when omitting a - # trailing slash on Synapse <= v0.99.2. + # trailing slash on Synapse <= v0.99.3. # Enable backoff if initially disabled send_request_args["backoff_on_404"] = backoff_on_404 @@ -560,7 +560,7 @@ class MatrixFederationHttpClient(object): requests). try_trailing_slash_on_400 (bool): True if on a 400 M_UNRECOGNIZED response we should try appending a trailing slash to the end - of the request. Workaround for #3622 in Synapse <=v0.99.2. This + of the request. Workaround for #3622 in Synapse <= v0.99.3. This will be attempted before backing off if backing off has been enabled. @@ -593,7 +593,7 @@ class MatrixFederationHttpClient(object): "timeout": timeout, "ignore_backoff": ignore_backoff, # Do not backoff on the initial request if we're trying again with - # trailing slashes Otherwise we may end up waiting to contact a + # trailing slashes. Otherwise we may end up waiting to contact a # server that is actually up "backoff_on_404": False if try_trailing_slash_on_400 else backoff_on_404, } @@ -681,7 +681,7 @@ class MatrixFederationHttpClient(object): and try the request anyway. try_trailing_slash_on_400 (bool): True if on a 400 M_UNRECOGNIZED response we should try appending a trailing slash to the end of - the request. Workaround for #3622 in Synapse <=v0.99.2. + the request. Workaround for #3622 in Synapse <= v0.99.3. Returns: Deferred[dict|list]: Succeeds when we get a 2xx HTTP response. The result will be the decoded JSON body. diff --git a/tests/http/test_fedclient.py b/tests/http/test_fedclient.py index b1b3a025ef..de5da1694a 100644 --- a/tests/http/test_fedclient.py +++ b/tests/http/test_fedclient.py @@ -272,7 +272,7 @@ class FederationClientTests(HomeserverTestCase): """ If a connection is made to a client but the client rejects it due to requiring a trailing slash. We need to retry the request with a - trailing slash. Workaround for Synapse <= v0.99.2, explained in #3622. + trailing slash. Workaround for Synapse <= v0.99.3, explained in #3622. """ d = self.cl.get_json( "testserv:8008", "foo/bar", try_trailing_slash_on_400=True, -- cgit 1.4.1 From cd36a1283b6036505b77f51a0cc860e8e5a0878d Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 20 Mar 2019 14:00:39 +0000 Subject: New test, fix issues --- synapse/http/matrixfederationclient.py | 77 +++++++++++++--------------------- tests/http/test_fedclient.py | 45 ++++++++++++++++++++ 2 files changed, 73 insertions(+), 49 deletions(-) (limited to 'synapse/http/matrixfederationclient.py') diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index 8312e4fc3f..dd358536f1 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -194,7 +194,7 @@ class MatrixFederationHttpClient(object): request, try_trailing_slash_on_400=False, backoff_on_404=False, - send_request_args={}, + **send_request_args ): """Wrapper for _send_request which can optionally retry the request upon receiving a combination of a 400 HTTP response code and a @@ -206,9 +206,8 @@ class MatrixFederationHttpClient(object): try_trailing_slash_on_400 (bool): Whether on receiving a 400 'M_UNRECOGNIZED' from the server to retry the request with a trailing slash appended to the request path. - 404_backoff (bool): Whether to backoff on 404 when making a - request with a trailing slash (only affects request if - try_trailing_slash_on_400 is True). + backoff_on_404 (bool): Whether to backoff on 404 when making a + request with a trailing slash. send_request_args (Dict): A dictionary of arguments to pass to `_send_request()`. @@ -220,36 +219,24 @@ class MatrixFederationHttpClient(object): Deferred[Dict]: Parsed JSON response body. """ try: - response = yield self._send_request(**send_request_args) - - # Check if it's necessary to retry with a trailing slash - body = yield _handle_json_response( - self.hs.get_reactor(), self.default_timeout, request, response, - ) - - defer.returnValue(body) + response = yield self._send_request(request, backoff_on_404=backoff_on_404, **send_request_args) except HttpResponseException as e: + # Received an HTTP error > 300. Check if it meets the requirements + # to retry with a trailing slash if not try_trailing_slash_on_400: - # Received an error >= 300. Raise unless we're retrying - raise e - except Exception as e: - raise e + raise - # Retry with a trailing slash if we received a 400 with - # 'M_UNRECOGNIZED' which some endpoints can return when omitting a - # trailing slash on Synapse <= v0.99.3. - # Enable backoff if initially disabled - send_request_args["backoff_on_404"] = backoff_on_404 + if e.code != 400 or e.to_synapse_error().errcode != "M_UNRECOGNIZED": + raise - # Add trailing slash - send_request_args["request"].path += "/" + # Retry with a trailing slash if we received a 400 with + # 'M_UNRECOGNIZED' which some endpoints can return when omitting a + # trailing slash on Synapse <= v0.99.3. + request.path += "/" - response = yield self._send_request(**send_request_args) - body = yield _handle_json_response( - self.hs.get_reactor(), self.default_timeout, request, response, - ) + response = yield self._send_request(request, backoff_on_404=backoff_on_404, **send_request_args) - defer.returnValue(body) + defer.returnValue(response) @defer.inlineCallbacks def _send_request( @@ -587,19 +574,13 @@ class MatrixFederationHttpClient(object): json=data, ) - send_request_args = { - "request": request, - "long_retries": long_retries, - "timeout": timeout, - "ignore_backoff": ignore_backoff, - # Do not backoff on the initial request if we're trying again with - # trailing slashes. Otherwise we may end up waiting to contact a - # server that is actually up - "backoff_on_404": False if try_trailing_slash_on_400 else backoff_on_404, - } + response = yield self._send_request_with_optional_trailing_slash( + request, try_trailing_slash_on_400, backoff_on_404, + long_retries=long_retries, timeout=timeout, ignore_backoff=ignore_backoff, + ) - body = yield self._send_request_with_optional_trailing_slash( - request, try_trailing_slash_on_400, backoff_on_404, send_request_args, + body = yield _handle_json_response( + self.hs.get_reactor(), self.default_timeout, request, response, ) defer.returnValue(body) @@ -707,16 +688,14 @@ class MatrixFederationHttpClient(object): query=args, ) - send_request_args = { - "request": request, - "retry_on_dns_fail": retry_on_dns_fail, - "timeout": timeout, - "ignore_backoff": ignore_backoff, - "backoff_on_404": False, - } + response = yield self._send_request_with_optional_trailing_slash( + request, try_trailing_slash_on_400, False, + retry_on_dns_fail=retry_on_dns_fail, timeout=timeout, + ignore_backoff=ignore_backoff, + ) - body = yield self._send_request_with_optional_trailing_slash( - request, try_trailing_slash_on_400, False, send_request_args, + body = yield _handle_json_response( + self.hs.get_reactor(), self.default_timeout, request, response, ) defer.returnValue(body) diff --git a/tests/http/test_fedclient.py b/tests/http/test_fedclient.py index de5da1694a..fbe87d4d0b 100644 --- a/tests/http/test_fedclient.py +++ b/tests/http/test_fedclient.py @@ -322,6 +322,51 @@ class FederationClientTests(HomeserverTestCase): r = self.successResultOf(d) self.assertEqual(r, {}) + def test_client_does_not_retry_on_400_plus(self): + """ + Another test for trailing slashes but now test that we don't retry on + trailing slashes on a non-400/M_UNRECOGNIZED response. + + See test_client_requires_trailing_slashes() for context. + """ + d = self.cl.get_json( + "testserv:8008", "foo/bar", try_trailing_slash_on_400=True, + ) + + # Send the request + self.pump() + + # there should have been a call to connectTCP + clients = self.reactor.tcpClients + self.assertEqual(len(clients), 1) + (_host, _port, factory, _timeout, _bindAddress) = clients[0] + + # complete the connection and wire it up to a fake transport + client = factory.buildProtocol(None) + conn = StringTransport() + client.makeConnection(conn) + + # that should have made it send the request to the connection + self.assertRegex(conn.value(), b"^GET /foo/bar") + + # Clear the original request data before sending a response + conn.clear() + + # Send the HTTP response + client.dataReceived( + b"HTTP/1.1 404 Not Found\r\n" + b"Content-Type: application/json\r\n" + b"Content-Length: 2\r\n" + b"\r\n" + b"{}" + ) + + # We should not get another request + self.assertEqual(conn.value(), b"") + + # We should get a 404 failure response + r = self.failureResultOf(d) + def test_client_sends_body(self): self.cl.post_json( "testserv:8008", "foo/bar", timeout=10000, -- cgit 1.4.1 From bb52a2e65308f10285d5d6290786e889d203f3db Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 20 Mar 2019 14:08:57 +0000 Subject: lint --- synapse/http/matrixfederationclient.py | 8 ++++++-- tests/http/test_fedclient.py | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) (limited to 'synapse/http/matrixfederationclient.py') diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index dd358536f1..2c1bcf66fd 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -219,7 +219,9 @@ class MatrixFederationHttpClient(object): Deferred[Dict]: Parsed JSON response body. """ try: - response = yield self._send_request(request, backoff_on_404=backoff_on_404, **send_request_args) + response = yield self._send_request( + request, backoff_on_404=backoff_on_404, **send_request_args, + ) except HttpResponseException as e: # Received an HTTP error > 300. Check if it meets the requirements # to retry with a trailing slash @@ -234,7 +236,9 @@ class MatrixFederationHttpClient(object): # trailing slash on Synapse <= v0.99.3. request.path += "/" - response = yield self._send_request(request, backoff_on_404=backoff_on_404, **send_request_args) + response = yield self._send_request( + request, backoff_on_404=backoff_on_404, **send_request_args, + ) defer.returnValue(response) diff --git a/tests/http/test_fedclient.py b/tests/http/test_fedclient.py index fbe87d4d0b..cd8e086f86 100644 --- a/tests/http/test_fedclient.py +++ b/tests/http/test_fedclient.py @@ -365,7 +365,7 @@ class FederationClientTests(HomeserverTestCase): self.assertEqual(conn.value(), b"") # We should get a 404 failure response - r = self.failureResultOf(d) + self.failureResultOf(d) def test_client_sends_body(self): self.cl.post_json( -- cgit 1.4.1 From 2150151abe919884671fe2d080e5145d9face5fa Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 20 Mar 2019 14:13:32 +0000 Subject: kwargs doesn't like commas on calling funcs either. TIL --- synapse/http/matrixfederationclient.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'synapse/http/matrixfederationclient.py') diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index 2c1bcf66fd..c654c1cf12 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -220,7 +220,7 @@ class MatrixFederationHttpClient(object): """ try: response = yield self._send_request( - request, backoff_on_404=backoff_on_404, **send_request_args, + request, backoff_on_404=backoff_on_404, **send_request_args ) except HttpResponseException as e: # Received an HTTP error > 300. Check if it meets the requirements @@ -237,7 +237,7 @@ class MatrixFederationHttpClient(object): request.path += "/" response = yield self._send_request( - request, backoff_on_404=backoff_on_404, **send_request_args, + request, backoff_on_404=backoff_on_404, **send_request_args ) defer.returnValue(response) -- cgit 1.4.1 From b41c2eaadc1b9d6150ac0f7979e877e5ee9ab80e Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 21 Mar 2019 14:32:47 +0000 Subject: Clean up backoff_on_404 and metehod calls --- synapse/http/matrixfederationclient.py | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) (limited to 'synapse/http/matrixfederationclient.py') diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index c654c1cf12..8e855d13d6 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -193,7 +193,6 @@ class MatrixFederationHttpClient(object): self, request, try_trailing_slash_on_400=False, - backoff_on_404=False, **send_request_args ): """Wrapper for _send_request which can optionally retry the request @@ -206,8 +205,6 @@ class MatrixFederationHttpClient(object): try_trailing_slash_on_400 (bool): Whether on receiving a 400 'M_UNRECOGNIZED' from the server to retry the request with a trailing slash appended to the request path. - backoff_on_404 (bool): Whether to backoff on 404 when making a - request with a trailing slash. send_request_args (Dict): A dictionary of arguments to pass to `_send_request()`. @@ -220,7 +217,7 @@ class MatrixFederationHttpClient(object): """ try: response = yield self._send_request( - request, backoff_on_404=backoff_on_404, **send_request_args + request, **send_request_args ) except HttpResponseException as e: # Received an HTTP error > 300. Check if it meets the requirements @@ -237,7 +234,7 @@ class MatrixFederationHttpClient(object): request.path += "/" response = yield self._send_request( - request, backoff_on_404=backoff_on_404, **send_request_args + request, **send_request_args ) defer.returnValue(response) @@ -579,8 +576,12 @@ class MatrixFederationHttpClient(object): ) response = yield self._send_request_with_optional_trailing_slash( - request, try_trailing_slash_on_400, backoff_on_404, - long_retries=long_retries, timeout=timeout, ignore_backoff=ignore_backoff, + request, + try_trailing_slash_on_400, + backoff_on_404=backoff_on_404, + ignore_backoff=ignore_backoff, + long_retries=long_retries, + timeout=timeout, ) body = yield _handle_json_response( @@ -693,9 +694,12 @@ class MatrixFederationHttpClient(object): ) response = yield self._send_request_with_optional_trailing_slash( - request, try_trailing_slash_on_400, False, - retry_on_dns_fail=retry_on_dns_fail, timeout=timeout, + request, + try_trailing_slash_on_400, + backoff_on_404=False, ignore_backoff=ignore_backoff, + retry_on_dns_fail=retry_on_dns_fail, + timeout=timeout, ) body = yield _handle_json_response( -- cgit 1.4.1