Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Commit

Permalink
Add Retry-After to M_LIMIT_EXCEEDED error responses (#16136)
Browse files Browse the repository at this point in the history
Implements MSC4041 behind an experimental configuration flag.
  • Loading branch information
Half-Shot authored and hughns committed Sep 4, 2023
1 parent 6d9bbbf commit 55939cb
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 7 deletions.
1 change: 1 addition & 0 deletions changelog.d/16136.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Return a `Retry-After` with `M_LIMIT_EXCEEDED` error responses.
10 changes: 9 additions & 1 deletion synapse/api/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
"""Contains exceptions and error codes."""

import logging
import math
import typing
from enum import Enum
from http import HTTPStatus
Expand Down Expand Up @@ -503,14 +504,21 @@ 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,
code: int = 429,
msg: str = "Too Many Requests",
retry_after_ms: Optional[int] = None,
errcode: str = Codes.LIMIT_EXCEEDED,
):
super().__init__(code, msg, errcode)
headers = (
{"Retry-After": str(math.ceil(retry_after_ms / 1000))}
if self.include_retry_after_header and retry_after_ms is not None
else None
)
super().__init__(code, msg, errcode, headers=headers)
self.retry_after_ms = retry_after_ms

def error_dict(self, config: Optional["HomeServerConfig"]) -> "JsonDict":
Expand Down
9 changes: 9 additions & 0 deletions synapse/config/experimental.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
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
Expand Down Expand Up @@ -406,3 +407,11 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
self.msc4010_push_rules_account_data = experimental.get(
"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
)
36 changes: 36 additions & 0 deletions tests/api/test_errors.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# Copyright 2023 The Matrix.org Foundation C.I.C.
#
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

from synapse.api.errors import LimitExceededError

from tests import unittest


class ErrorsTestCase(unittest.TestCase):
# 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 = ErrorsTestCase.LimitExceededErrorHeaders(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 = ErrorsTestCase.LimitExceededErrorHeaders(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")
24 changes: 18 additions & 6 deletions tests/rest/client/test_login.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,8 @@ def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer:
# which sets these values to 10000, but as we're overriding the entire
# 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:
Expand All @@ -189,12 +190,15 @@ def test_POST_ratelimiting_per_address(self) -> None:
if i == 5:
self.assertEqual(channel.code, 429, msg=channel.result)
retry_after_ms = int(channel.json_body["retry_after_ms"])
retry_header = channel.headers.getRawHeaders("Retry-After")
else:
self.assertEqual(channel.code, 200, msg=channel.result)

# Since we're ratelimiting at 1 request/min, retry_after_ms should be lower
# than 1min.
self.assertTrue(retry_after_ms < 6000)
self.assertLess(retry_after_ms, 6000)
assert retry_header
self.assertLessEqual(int(retry_header[0]), 6)

self.reactor.advance(retry_after_ms / 1000.0 + 1.0)

Expand All @@ -217,7 +221,8 @@ def test_POST_ratelimiting_per_address(self) -> None:
# which sets these values to 10000, but as we're overriding the entire
# 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:
Expand All @@ -234,12 +239,15 @@ def test_POST_ratelimiting_per_account(self) -> None:
if i == 5:
self.assertEqual(channel.code, 429, msg=channel.result)
retry_after_ms = int(channel.json_body["retry_after_ms"])
retry_header = channel.headers.getRawHeaders("Retry-After")
else:
self.assertEqual(channel.code, 200, msg=channel.result)

# Since we're ratelimiting at 1 request/min, retry_after_ms should be lower
# than 1min.
self.assertTrue(retry_after_ms < 6000)
self.assertLess(retry_after_ms, 6000)
assert retry_header
self.assertLessEqual(int(retry_header[0]), 6)

self.reactor.advance(retry_after_ms / 1000.0)

Expand All @@ -262,7 +270,8 @@ def test_POST_ratelimiting_per_account(self) -> None:
# rc_login dict here, we need to set this manually as well
"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:
Expand All @@ -279,12 +288,15 @@ def test_POST_ratelimiting_per_account_failed_attempts(self) -> None:
if i == 5:
self.assertEqual(channel.code, 429, msg=channel.result)
retry_after_ms = int(channel.json_body["retry_after_ms"])
retry_header = channel.headers.getRawHeaders("Retry-After")
else:
self.assertEqual(channel.code, 403, msg=channel.result)

# Since we're ratelimiting at 1 request/min, retry_after_ms should be lower
# than 1min.
self.assertTrue(retry_after_ms < 6000)
self.assertLess(retry_after_ms, 6000)
assert retry_header
self.assertLessEqual(int(retry_header[0]), 6)

self.reactor.advance(retry_after_ms / 1000.0 + 1.0)

Expand Down

0 comments on commit 55939cb

Please sign in to comment.