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

This commit is contained in:
Patrick Cloke 2024-03-08 04:33:46 -05:00 committed by GitHub
parent 4af33015af
commit 696cc9e802
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 5 additions and 21 deletions

View File

@ -0,0 +1 @@
Include `Retry-After` header by default per [MSC4041](https://github.com/matrix-org/matrix-spec-proposals/pull/4041). Contributed by @clokep.

View File

@ -517,8 +517,6 @@ class InvalidCaptchaError(SynapseError):
class LimitExceededError(SynapseError): 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."""
include_retry_after_header = False
def __init__( def __init__(
self, self,
limiter_name: str, limiter_name: str,
@ -526,9 +524,10 @@ class LimitExceededError(SynapseError):
retry_after_ms: Optional[int] = None, retry_after_ms: Optional[int] = None,
errcode: str = Codes.LIMIT_EXCEEDED, errcode: str = Codes.LIMIT_EXCEEDED,
): ):
# Use HTTP header Retry-After to enable library-assisted retry handling.
headers = ( headers = (
{"Retry-After": str(math.ceil(retry_after_ms / 1000))} {"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 else None
) )
super().__init__(code, "Too Many Requests", errcode, headers=headers) super().__init__(code, "Too Many Requests", errcode, headers=headers)

View File

@ -25,7 +25,6 @@ from typing import TYPE_CHECKING, Any, Optional
import attr import attr
import attr.validators import attr.validators
from synapse.api.errors import LimitExceededError
from synapse.api.room_versions import KNOWN_ROOM_VERSIONS, RoomVersions from synapse.api.room_versions import KNOWN_ROOM_VERSIONS, RoomVersions
from synapse.config import ConfigError from synapse.config import ConfigError
from synapse.config._base import Config, RootConfig from synapse.config._base import Config, RootConfig
@ -415,14 +414,6 @@ class ExperimentalConfig(Config):
"msc4010_push_rules_account_data", False "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( self.msc4028_push_encrypted_events = experimental.get(
"msc4028_push_encrypted_events", False "msc4028_push_encrypted_events", False
) )

View File

@ -33,18 +33,14 @@ class LimitExceededErrorTestCase(unittest.TestCase):
self.assertIn("needle", err.debug_context) self.assertIn("needle", err.debug_context)
self.assertNotIn("needle", serialised) 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: 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) self.assertEqual(err.error_dict(None).get("retry_after_ms"), 100)
assert err.headers is not None assert err.headers is not None
self.assertEqual(err.headers.get("Retry-After"), "1") self.assertEqual(err.headers.get("Retry-After"), "1")
def test_limit_exceeded_rounding(self) -> None: 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) self.assertEqual(err.error_dict(None).get("retry_after_ms"), 3001)
assert err.headers is not None assert err.headers is not None
self.assertEqual(err.headers.get("Retry-After"), "4") self.assertEqual(err.headers.get("Retry-After"), "4")

View File

@ -177,7 +177,6 @@ class LoginRestServletTestCase(unittest.HomeserverTestCase):
# rc_login dict here, we need to set this manually as well # rc_login dict here, we need to set this manually as well
"account": {"per_second": 10000, "burst_count": 10000}, "account": {"per_second": 10000, "burst_count": 10000},
}, },
"experimental_features": {"msc4041_enabled": True},
} }
) )
def test_POST_ratelimiting_per_address(self) -> None: def test_POST_ratelimiting_per_address(self) -> None:
@ -229,7 +228,6 @@ class LoginRestServletTestCase(unittest.HomeserverTestCase):
# rc_login dict here, we need to set this manually as well # rc_login dict here, we need to set this manually as well
"address": {"per_second": 10000, "burst_count": 10000}, "address": {"per_second": 10000, "burst_count": 10000},
}, },
"experimental_features": {"msc4041_enabled": True},
} }
) )
def test_POST_ratelimiting_per_account(self) -> None: def test_POST_ratelimiting_per_account(self) -> None:
@ -278,7 +276,6 @@ class LoginRestServletTestCase(unittest.HomeserverTestCase):
"address": {"per_second": 10000, "burst_count": 10000}, "address": {"per_second": 10000, "burst_count": 10000},
"failed_attempts": {"per_second": 0.17, "burst_count": 5}, "failed_attempts": {"per_second": 0.17, "burst_count": 5},
}, },
"experimental_features": {"msc4041_enabled": True},
} }
) )
def test_POST_ratelimiting_per_account_failed_attempts(self) -> None: def test_POST_ratelimiting_per_account_failed_attempts(self) -> None: