Lift pausing on ratelimited requests to http layer

changelog.d/18595.misc

@@ -0,0 +1 @@
+Better handling of ratelimited requests.

synapse/api/errors.py

@@ -527,14 +527,19 @@ def error_dict(self, config: Optional["HomeServerConfig"]) -> "JsonDict":
 
 
 class LimitExceededError(SynapseError):
-    """A client has sent too many requests and is being throttled."""
+    """A client has sent too many requests and is being throttled.
+
+    Args:
+        pause: Optional time in seconds to pause before responding to the client.
+    """
 
     def __init__(
         self,
         limiter_name: str,
         code: int = 429,
         retry_after_ms: Optional[int] = None,
         errcode: str = Codes.LIMIT_EXCEEDED,
+        pause: Optional[float] = None,
     ):
         # Use HTTP header Retry-After to enable library-assisted retry handling.
         headers = (
@@ -545,6 +550,7 @@ def __init__(
         super().__init__(code, "Too Many Requests", errcode, headers=headers)
         self.retry_after_ms = retry_after_ms
         self.limiter_name = limiter_name
+        self.pause = pause
 
     def error_dict(self, config: Optional["HomeServerConfig"]) -> "JsonDict":
         return cs_error(self.msg, self.errcode, retry_after_ms=self.retry_after_ms)

synapse/api/ratelimiting.py

@@ -338,12 +338,10 @@ async def ratelimit(
         )
 
         if not allowed:
-            if pause:
-                await self.clock.sleep(pause)
-
             raise LimitExceededError(
                 limiter_name=self._limiter_name,
                 retry_after_ms=int(1000 * (time_allowed - time_now_s)),
+                pause=pause,
             )
 
 

synapse/http/additional_resource.py

@@ -53,7 +53,7 @@ def __init__(
             hs: homeserver
             handler: function to be called to handle the request.
         """
-        super().__init__()
+        super().__init__(hs.get_clock())
         self._handler = handler
 
     async def _async_render(self, request: Request) -> Optional[Tuple[int, Any]]:

synapse/http/proxy.py

@@ -106,7 +106,7 @@ class ProxyResource(_AsyncResource):
     isLeaf = True
 
     def __init__(self, reactor: ISynapseReactor, hs: "HomeServer"):
-        super().__init__(True)
+        super().__init__(hs.get_clock(), True)
 
         self.reactor = reactor
         self.agent = hs.get_federation_http_client().agent

synapse/http/server.py

@@ -67,14 +67,15 @@
 from synapse.api.errors import (
     CodeMessageException,
     Codes,
+    LimitExceededError,
     RedirectException,
     SynapseError,
     UnrecognizedRequestError,
 )
 from synapse.config.homeserver import HomeServerConfig
 from synapse.logging.context import defer_to_thread, preserve_fn, run_in_background
 from synapse.logging.opentracing import active_span, start_active_span, trace_servlet
-from synapse.util import json_encoder
+from synapse.util import Clock, json_encoder
 from synapse.util.caches import intern_dict
 from synapse.util.cancellation import is_function_cancellable
 from synapse.util.iterutils import chunk_seq
@@ -308,9 +309,10 @@ class _AsyncResource(resource.Resource, metaclass=abc.ABCMeta):
             context from the request the servlet is handling.
     """
 
-    def __init__(self, extract_context: bool = False):
+    def __init__(self, clock: Clock, extract_context: bool = False):
         super().__init__()
 
+        self._clock = clock
         self._extract_context = extract_context
 
     def render(self, request: "SynapseRequest") -> int:
@@ -329,7 +331,12 @@ async def _async_render_wrapper(self, request: "SynapseRequest") -> None:
             request.request_metrics.name = self.__class__.__name__
 
             with trace_servlet(request, self._extract_context):
-                callback_return = await self._async_render(request)
+                try:
+                    callback_return = await self._async_render(request)
+                except LimitExceededError as e:
+                    if e.pause:
+                        self._clock.sleep(e.pause)
+                    raise
 
                 if callback_return is not None:
                     code, response = callback_return
@@ -393,8 +400,10 @@ class DirectServeJsonResource(_AsyncResource):
     formatting responses and errors as JSON.
     """
 
-    def __init__(self, canonical_json: bool = False, extract_context: bool = False):
-        super().__init__(extract_context)
+    def __init__(
+        self, clock: Clock, canonical_json: bool = False, extract_context: bool = False
+    ):
+        super().__init__(clock, extract_context)
         self.canonical_json = canonical_json
 
     def _send_response(
@@ -450,8 +459,8 @@ def __init__(
         canonical_json: bool = True,
         extract_context: bool = False,
     ):
-        super().__init__(canonical_json, extract_context)
         self.clock = hs.get_clock()
+        super().__init__(self.clock, canonical_json, extract_context)
         # Map of path regex -> method -> callback.
         self._routes: Dict[Pattern[str], Dict[bytes, _PathEntry]] = {}
         self.hs = hs

synapse/rest/consent/consent_resource.py

@@ -81,7 +81,7 @@ class ConsentResource(DirectServeHtmlResource):
     """
 
     def __init__(self, hs: "HomeServer"):
-        super().__init__()
+        super().__init__(hs.get_clock())
 
         self.hs = hs
         self.store = hs.get_datastores().main

synapse/rest/synapse/client/federation_whitelist.py

@@ -44,7 +44,7 @@ class FederationWhitelistResource(DirectServeJsonResource):
     PATH = "/_synapse/client/v1/config/federation_whitelist"
 
     def __init__(self, hs: "HomeServer"):
-        super().__init__()
+        super().__init__(hs.get_clock())
 
         self._federation_whitelist = hs.config.federation.federation_domain_whitelist
 

synapse/rest/synapse/client/jwks.py

@@ -33,7 +33,7 @@
 
 class JwksResource(DirectServeJsonResource):
     def __init__(self, hs: "HomeServer"):
-        super().__init__(extract_context=True)
+        super().__init__(hs.get_clock(), extract_context=True)
 
         # Parameters that are allowed to be exposed in the public key.
         # This is done manually, because authlib's private to public key conversion

synapse/rest/synapse/client/new_user_consent.py

@@ -48,7 +48,7 @@ class NewUserConsentResource(DirectServeHtmlResource):
     """
 
     def __init__(self, hs: "HomeServer"):
-        super().__init__()
+        super().__init__(hs.get_clock())
         self._sso_handler = hs.get_sso_handler()
         self._server_name = hs.hostname
         self._consent_version = hs.config.consent.user_consent_version

synapse/rest/synapse/client/oidc/backchannel_logout_resource.py

@@ -35,7 +35,7 @@ class OIDCBackchannelLogoutResource(DirectServeJsonResource):
     isLeaf = 1
 
     def __init__(self, hs: "HomeServer"):
-        super().__init__()
+        super().__init__(hs.get_clock())
         self._oidc_handler = hs.get_oidc_handler()
 
     async def _async_render_POST(self, request: SynapseRequest) -> None:

synapse/rest/synapse/client/oidc/callback_resource.py

@@ -35,7 +35,7 @@ class OIDCCallbackResource(DirectServeHtmlResource):
     isLeaf = 1
 
     def __init__(self, hs: "HomeServer"):
-        super().__init__()
+        super().__init__(hs.get_clock())
         self._oidc_handler = hs.get_oidc_handler()
 
     async def _async_render_GET(self, request: SynapseRequest) -> None:

synapse/rest/synapse/client/password_reset.py

@@ -47,7 +47,7 @@ def __init__(self, hs: "HomeServer"):
         Args:
             hs: server
         """
-        super().__init__()
+        super().__init__(hs.get_clock())
 
         self.clock = hs.get_clock()
         self.store = hs.get_datastores().main

synapse/rest/synapse/client/pick_idp.py

@@ -44,7 +44,7 @@ class PickIdpResource(DirectServeHtmlResource):
     """
 
     def __init__(self, hs: "HomeServer"):
-        super().__init__()
+        super().__init__(hs.get_clock())
         self._sso_handler = hs.get_sso_handler()
         self._sso_login_idp_picker_template = (
             hs.config.sso.sso_login_idp_picker_template

synapse/rest/synapse/client/pick_username.py

@@ -62,7 +62,7 @@ def pick_username_resource(hs: "HomeServer") -> Resource:
 
 class AvailabilityCheckResource(DirectServeJsonResource):
     def __init__(self, hs: "HomeServer"):
-        super().__init__()
+        super().__init__(hs.get_clock())
         self._sso_handler = hs.get_sso_handler()
 
     async def _async_render_GET(self, request: Request) -> Tuple[int, JsonDict]:
@@ -78,7 +78,7 @@ async def _async_render_GET(self, request: Request) -> Tuple[int, JsonDict]:
 
 class AccountDetailsResource(DirectServeHtmlResource):
     def __init__(self, hs: "HomeServer"):
-        super().__init__()
+        super().__init__(hs.get_clock())
         self._sso_handler = hs.get_sso_handler()
 
         def template_search_dirs() -> Generator[str, None, None]:

synapse/rest/synapse/client/rendezvous.py

@@ -30,7 +30,7 @@ class MSC4108RendezvousSessionResource(DirectServeJsonResource):
     isLeaf = True
 
     def __init__(self, hs: "HomeServer") -> None:
-        super().__init__()
+        super().__init__(hs.get_clock())
         self._handler = hs.get_rendezvous_handler()
 
     async def _async_render_GET(self, request: SynapseRequest) -> None:

synapse/rest/synapse/client/saml2/response_resource.py

@@ -35,7 +35,7 @@ class SAML2ResponseResource(DirectServeHtmlResource):
     isLeaf = 1
 
     def __init__(self, hs: "HomeServer"):
-        super().__init__()
+        super().__init__(hs.get_clock())
         self._saml_handler = hs.get_saml_handler()
         self._sso_handler = hs.get_sso_handler()
 

synapse/rest/synapse/client/sso_register.py

@@ -43,7 +43,7 @@ class SsoRegisterResource(DirectServeHtmlResource):
     """
 
     def __init__(self, hs: "HomeServer"):
-        super().__init__()
+        super().__init__(hs.get_clock())
         self._sso_handler = hs.get_sso_handler()
 
     async def _async_render_GET(self, request: Request) -> None:

synapse/rest/synapse/client/unsubscribe.py

@@ -38,7 +38,7 @@ class UnsubscribeResource(DirectServeHtmlResource):
     SUCCESS_HTML = b"<html><body>You have been unsubscribed</body><html>"
 
     def __init__(self, hs: "HomeServer"):
-        super().__init__()
+        super().__init__(hs.get_clock())
         self.notifier = hs.get_notifier()
         self.auth = hs.get_auth()
         self.pusher_pool = hs.get_pusherpool()

synapse/rest/well_known.py

@@ -86,7 +86,7 @@ class ClientWellKnownResource(DirectServeJsonResource):
     isLeaf = 1
 
     def __init__(self, hs: "HomeServer"):
-        super().__init__()
+        super().__init__(hs.get_clock())
         self._well_known_builder = WellKnownBuilder(hs)
 
     async def _async_render_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]:

tests/test_server.py

@@ -316,15 +316,16 @@ async def _async_render_GET(self, request: SynapseRequest) -> None:
             await self.callback(request)
 
     def setUp(self) -> None:
-        reactor, _ = get_clock()
+        reactor, clock = get_clock()
         self.reactor = reactor
+        self.clock = clock
 
     def test_good_response(self) -> None:
         async def callback(request: SynapseRequest) -> None:
             request.write(b"response")
             request.finish()
 
-        res = WrapHtmlRequestHandlerTests.TestResource()
+        res = WrapHtmlRequestHandlerTests.TestResource(self.clock)
         res.callback = callback
 
         channel = make_request(
@@ -344,7 +345,7 @@ def test_redirect_exception(self) -> None:
         async def callback(request: SynapseRequest, **kwargs: object) -> None:
             raise RedirectException(b"/look/an/eagle", 301)
 
-        res = WrapHtmlRequestHandlerTests.TestResource()
+        res = WrapHtmlRequestHandlerTests.TestResource(self.clock)
         res.callback = callback
 
         channel = make_request(
@@ -366,7 +367,7 @@ async def callback(request: SynapseRequest, **kwargs: object) -> NoReturn:
             e.cookies.append(b"session=yespls")
             raise e
 
-        res = WrapHtmlRequestHandlerTests.TestResource()
+        res = WrapHtmlRequestHandlerTests.TestResource(self.clock)
         res.callback = callback
 
         channel = make_request(
@@ -387,7 +388,7 @@ async def callback(request: SynapseRequest) -> None:
             request.write(b"response")
             request.finish()
 
-        res = WrapHtmlRequestHandlerTests.TestResource()
+        res = WrapHtmlRequestHandlerTests.TestResource(self.clock)
         res.callback = callback
 
         channel = make_request(
@@ -400,7 +401,7 @@ async def callback(request: SynapseRequest) -> None:
 
 class CancellableDirectServeJsonResource(DirectServeJsonResource):
     def __init__(self, clock: Clock):
-        super().__init__()
+        super().__init__(clock)
         self.clock = clock
 
     @cancellable
@@ -417,7 +418,7 @@ class CancellableDirectServeHtmlResource(DirectServeHtmlResource):
     ERROR_TEMPLATE = "{code} {msg}"
 
     def __init__(self, clock: Clock):
-        super().__init__()
+        super().__init__(clock)
         self.clock = clock
 
     @cancellable