Stabilize support for Retry-After header (MSC4014) (#16947)

Author: clokep

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.

synapse/api/errors.py

@@ -517,18 +517,17 @@ 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,
         code: int = 429,
         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)

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
         )

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")

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: