From f09cb2db4fefb845775a5d44e0869847c2d1cf83 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 25 Jun 2025 11:06:11 +0100 Subject: [PATCH 1/4] Pass Clock to AsyncResource --- synapse/http/additional_resource.py | 2 +- synapse/http/proxy.py | 2 +- synapse/http/server.py | 13 ++++++++----- synapse/rest/consent/consent_resource.py | 2 +- .../rest/synapse/client/federation_whitelist.py | 2 +- synapse/rest/synapse/client/jwks.py | 2 +- synapse/rest/synapse/client/new_user_consent.py | 2 +- .../client/oidc/backchannel_logout_resource.py | 2 +- .../rest/synapse/client/oidc/callback_resource.py | 2 +- synapse/rest/synapse/client/password_reset.py | 2 +- synapse/rest/synapse/client/pick_idp.py | 2 +- synapse/rest/synapse/client/pick_username.py | 4 ++-- synapse/rest/synapse/client/rendezvous.py | 2 +- .../synapse/client/saml2/response_resource.py | 2 +- synapse/rest/synapse/client/sso_register.py | 2 +- synapse/rest/synapse/client/unsubscribe.py | 2 +- synapse/rest/well_known.py | 2 +- tests/test_server.py | 15 ++++++++------- 18 files changed, 33 insertions(+), 29 deletions(-) diff --git a/synapse/http/additional_resource.py b/synapse/http/additional_resource.py index 2b9830b540d..e65af85570e 100644 --- a/synapse/http/additional_resource.py +++ b/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]]: diff --git a/synapse/http/proxy.py b/synapse/http/proxy.py index 5cd990b0d07..9b044f3b0ae 100644 --- a/synapse/http/proxy.py +++ b/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 diff --git a/synapse/http/server.py b/synapse/http/server.py index 311e21774f3..29fcf4295b5 100644 --- a/synapse/http/server.py +++ b/synapse/http/server.py @@ -74,7 +74,7 @@ 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 +308,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: @@ -393,8 +394,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 +453,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 diff --git a/synapse/rest/consent/consent_resource.py b/synapse/rest/consent/consent_resource.py index ff5dc51a010..a20eaf70e7a 100644 --- a/synapse/rest/consent/consent_resource.py +++ b/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 diff --git a/synapse/rest/synapse/client/federation_whitelist.py b/synapse/rest/synapse/client/federation_whitelist.py index 2b8f0320e0e..bccc3c3f37e 100644 --- a/synapse/rest/synapse/client/federation_whitelist.py +++ b/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 diff --git a/synapse/rest/synapse/client/jwks.py b/synapse/rest/synapse/client/jwks.py index 5f581d34450..dd70a4015a9 100644 --- a/synapse/rest/synapse/client/jwks.py +++ b/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 diff --git a/synapse/rest/synapse/client/new_user_consent.py b/synapse/rest/synapse/client/new_user_consent.py index 8b00b8c0127..dfe5dadf606 100644 --- a/synapse/rest/synapse/client/new_user_consent.py +++ b/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 diff --git a/synapse/rest/synapse/client/oidc/backchannel_logout_resource.py b/synapse/rest/synapse/client/oidc/backchannel_logout_resource.py index 3f4cf16934c..abc39234a73 100644 --- a/synapse/rest/synapse/client/oidc/backchannel_logout_resource.py +++ b/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: diff --git a/synapse/rest/synapse/client/oidc/callback_resource.py b/synapse/rest/synapse/client/oidc/callback_resource.py index 84077684d4d..6ce259d0ae7 100644 --- a/synapse/rest/synapse/client/oidc/callback_resource.py +++ b/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: diff --git a/synapse/rest/synapse/client/password_reset.py b/synapse/rest/synapse/client/password_reset.py index 29e4b2d07a5..28f924f4c76 100644 --- a/synapse/rest/synapse/client/password_reset.py +++ b/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 diff --git a/synapse/rest/synapse/client/pick_idp.py b/synapse/rest/synapse/client/pick_idp.py index 5e599f85b0b..e980e1e2abc 100644 --- a/synapse/rest/synapse/client/pick_idp.py +++ b/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 diff --git a/synapse/rest/synapse/client/pick_username.py b/synapse/rest/synapse/client/pick_username.py index 7d16b796d40..e11c3f43028 100644 --- a/synapse/rest/synapse/client/pick_username.py +++ b/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]: diff --git a/synapse/rest/synapse/client/rendezvous.py b/synapse/rest/synapse/client/rendezvous.py index 5216d30d1f4..ceb61ef7a10 100644 --- a/synapse/rest/synapse/client/rendezvous.py +++ b/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: diff --git a/synapse/rest/synapse/client/saml2/response_resource.py b/synapse/rest/synapse/client/saml2/response_resource.py index 7b8667e04c9..74e22f49bcf 100644 --- a/synapse/rest/synapse/client/saml2/response_resource.py +++ b/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() diff --git a/synapse/rest/synapse/client/sso_register.py b/synapse/rest/synapse/client/sso_register.py index be562ad8dc2..1a5afddaeeb 100644 --- a/synapse/rest/synapse/client/sso_register.py +++ b/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: diff --git a/synapse/rest/synapse/client/unsubscribe.py b/synapse/rest/synapse/client/unsubscribe.py index 6d4bd9f2ed1..b5077d57412 100644 --- a/synapse/rest/synapse/client/unsubscribe.py +++ b/synapse/rest/synapse/client/unsubscribe.py @@ -38,7 +38,7 @@ class UnsubscribeResource(DirectServeHtmlResource): SUCCESS_HTML = b"You have been unsubscribed" 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() diff --git a/synapse/rest/well_known.py b/synapse/rest/well_known.py index d336d60c93b..45d81af8422 100644 --- a/synapse/rest/well_known.py +++ b/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]: diff --git a/tests/test_server.py b/tests/test_server.py index 9ebfbf96e7b..b7e62ba0e61 100644 --- a/tests/test_server.py +++ b/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 From 3a083f8944e775750861d4aaecf3b8ec93eb2425 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 25 Jun 2025 11:06:25 +0100 Subject: [PATCH 2/4] Pause at outer edge when ratelimiting --- synapse/api/errors.py | 2 ++ synapse/api/ratelimiting.py | 4 +--- synapse/http/server.py | 8 +++++++- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/synapse/api/errors.py b/synapse/api/errors.py index 3eb533f7d57..4aca7176789 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -535,6 +535,7 @@ def __init__( 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 +546,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) diff --git a/synapse/api/ratelimiting.py b/synapse/api/ratelimiting.py index 4f3bf8f7704..509ef6b2c18 100644 --- a/synapse/api/ratelimiting.py +++ b/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, ) diff --git a/synapse/http/server.py b/synapse/http/server.py index 29fcf4295b5..ec99aa39f99 100644 --- a/synapse/http/server.py +++ b/synapse/http/server.py @@ -67,6 +67,7 @@ from synapse.api.errors import ( CodeMessageException, Codes, + LimitExceededError, RedirectException, SynapseError, UnrecognizedRequestError, @@ -330,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 From 8f486f1c677265e9cf08700f1b87209095924339 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 25 Jun 2025 13:22:14 +0100 Subject: [PATCH 3/4] Newsfile --- changelog.d/18595.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/18595.misc diff --git a/changelog.d/18595.misc b/changelog.d/18595.misc new file mode 100644 index 00000000000..b1a39b43f76 --- /dev/null +++ b/changelog.d/18595.misc @@ -0,0 +1 @@ +Better handling of ratelimited requests. From 79341f5e9f4182bcafaeaeed0b81bbf58489a51f Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 25 Jun 2025 15:02:40 +0100 Subject: [PATCH 4/4] Docs --- synapse/api/errors.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/synapse/api/errors.py b/synapse/api/errors.py index 4aca7176789..601a09efe17 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -527,7 +527,11 @@ 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,