diff --git a/changelog.d/16947.feature b/changelog.d/16947.feature new file mode 100644 index 0000000000..efd0dbddb2 --- /dev/null +++ b/changelog.d/16947.feature @@ -0,0 +1 @@ +Include `Retry-After` header by default per [MSC4041](https://github.com/matrix-org/matrix-spec-proposals/pull/4041). Contributed by @clokep. diff --git a/synapse/api/errors.py b/synapse/api/errors.py index b44088f9b3..dd4a1ae706 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -517,8 +517,6 @@ def error_dict(self, config: Optional["HomeServerConfig"]) -> "JsonDict": class LimitExceededError(SynapseError): """A client has sent too many requests and is being throttled.""" - include_retry_after_header = False - def __init__( self, limiter_name: str, @@ -526,9 +524,10 @@ def __init__( retry_after_ms: Optional[int] = None, errcode: str = Codes.LIMIT_EXCEEDED, ): + # Use HTTP header Retry-After to enable library-assisted retry handling. headers = ( {"Retry-After": str(math.ceil(retry_after_ms / 1000))} - if self.include_retry_after_header and retry_after_ms is not None + if retry_after_ms is not None else None ) super().__init__(code, "Too Many Requests", errcode, headers=headers) diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index d43d9da956..0bd3befdc2 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -25,7 +25,6 @@ import attr import attr.validators -from synapse.api.errors import LimitExceededError from synapse.api.room_versions import KNOWN_ROOM_VERSIONS, RoomVersions from synapse.config import ConfigError from synapse.config._base import Config, RootConfig @@ -415,14 +414,6 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: "msc4010_push_rules_account_data", False ) - # MSC4041: Use HTTP header Retry-After to enable library-assisted retry handling - # - # This is a bit hacky, but the most reasonable way to *alway* include the - # headers. - LimitExceededError.include_retry_after_header = experimental.get( - "msc4041_enabled", False - ) - self.msc4028_push_encrypted_events = experimental.get( "msc4028_push_encrypted_events", False ) diff --git a/tests/api/test_errors.py b/tests/api/test_errors.py index 25fa93b9d8..efa3addf00 100644 --- a/tests/api/test_errors.py +++ b/tests/api/test_errors.py @@ -33,18 +33,14 @@ def test_key_appears_in_context_but_not_error_dict(self) -> None: self.assertIn("needle", err.debug_context) self.assertNotIn("needle", serialised) - # Create a sub-class to avoid mutating the class-level property. - class LimitExceededErrorHeaders(LimitExceededError): - include_retry_after_header = True - def test_limit_exceeded_header(self) -> None: - err = self.LimitExceededErrorHeaders(limiter_name="test", retry_after_ms=100) + err = LimitExceededError(limiter_name="test", retry_after_ms=100) self.assertEqual(err.error_dict(None).get("retry_after_ms"), 100) assert err.headers is not None self.assertEqual(err.headers.get("Retry-After"), "1") def test_limit_exceeded_rounding(self) -> None: - err = self.LimitExceededErrorHeaders(limiter_name="test", retry_after_ms=3001) + err = LimitExceededError(limiter_name="test", retry_after_ms=3001) self.assertEqual(err.error_dict(None).get("retry_after_ms"), 3001) assert err.headers is not None self.assertEqual(err.headers.get("Retry-After"), "4") diff --git a/tests/rest/client/test_login.py b/tests/rest/client/test_login.py index 3d3a7b0aa7..3a1f150082 100644 --- a/tests/rest/client/test_login.py +++ b/tests/rest/client/test_login.py @@ -177,7 +177,6 @@ def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer: # rc_login dict here, we need to set this manually as well "account": {"per_second": 10000, "burst_count": 10000}, }, - "experimental_features": {"msc4041_enabled": True}, } ) def test_POST_ratelimiting_per_address(self) -> None: @@ -229,7 +228,6 @@ def test_POST_ratelimiting_per_address(self) -> None: # rc_login dict here, we need to set this manually as well "address": {"per_second": 10000, "burst_count": 10000}, }, - "experimental_features": {"msc4041_enabled": True}, } ) def test_POST_ratelimiting_per_account(self) -> None: @@ -278,7 +276,6 @@ def test_POST_ratelimiting_per_account(self) -> None: "address": {"per_second": 10000, "burst_count": 10000}, "failed_attempts": {"per_second": 0.17, "burst_count": 5}, }, - "experimental_features": {"msc4041_enabled": True}, } ) def test_POST_ratelimiting_per_account_failed_attempts(self) -> None: