diff options
| author | Sean Quah <8349537+squahtx@users.noreply.github.com> | 2022-05-11 14:52:26 +0100 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2022-05-11 14:52:26 +0100 |
| commit | 6ee61b905256f87dc2b75007ed711cd59065db9a (patch) | |
| tree | cbde33f2ea43c6c7c1180e1f7085682c52fdf0d8 | |
| parent | Reload cache factors from disk on SIGHUP (#12673) (diff) | |
| download | synapse-6ee61b905256f87dc2b75007ed711cd59065db9a.tar.xz | |
Complain if a federation endpoint has the `@cancellable` flag (#12705)
`BaseFederationServlet` wraps its endpoints in a bunch of async code that has not been vetted for compatibility with cancellation. Fail CI if a `@cancellable` flag is applied to a federation endpoint. Signed-off-by: Sean Quah <seanq@element.io>
| -rw-r--r-- | changelog.d/12705.misc | 1 | ||||
| -rw-r--r-- | synapse/federation/transport/server/_base.py | 13 | ||||
| -rw-r--r-- | tests/federation/transport/server/test__base.py | 2 |
3 files changed, 15 insertions, 1 deletions
diff --git a/changelog.d/12705.misc b/changelog.d/12705.misc new file mode 100644 index 0000000000..a913d8bb85 --- /dev/null +++ b/changelog.d/12705.misc @@ -0,0 +1 @@ +Complain if a federation endpoint has the `@cancellable` flag, since some of the wrapper code may not handle cancellation correctly yet. diff --git a/synapse/federation/transport/server/_base.py b/synapse/federation/transport/server/_base.py index d629a3ecb5..103861644a 100644 --- a/synapse/federation/transport/server/_base.py +++ b/synapse/federation/transport/server/_base.py @@ -21,7 +21,7 @@ from typing import TYPE_CHECKING, Any, Awaitable, Callable, Dict, Optional, Tupl from synapse.api.errors import Codes, FederationDeniedError, SynapseError from synapse.api.urls import FEDERATION_V1_PREFIX -from synapse.http.server import HttpServer, ServletCallback +from synapse.http.server import HttpServer, ServletCallback, is_method_cancellable from synapse.http.servlet import parse_json_object_from_request from synapse.http.site import SynapseRequest from synapse.logging.context import run_in_background @@ -373,6 +373,17 @@ class BaseFederationServlet: if code is None: continue + if is_method_cancellable(code): + # The wrapper added by `self._wrap` will inherit the cancellable flag, + # but the wrapper itself does not support cancellation yet. + # Once resolved, the cancellation tests in + # `tests/federation/transport/server/test__base.py` can be re-enabled. + raise Exception( + f"{self.__class__.__name__}.on_{method} has been marked as " + "cancellable, but federation servlets do not support cancellation " + "yet." + ) + server.register_paths( method, (pattern,), diff --git a/tests/federation/transport/server/test__base.py b/tests/federation/transport/server/test__base.py index 98a951f03e..ac3695a8cc 100644 --- a/tests/federation/transport/server/test__base.py +++ b/tests/federation/transport/server/test__base.py @@ -59,6 +59,8 @@ class BaseFederationServletCancellationTests( ): """Tests for `BaseFederationServlet` cancellation.""" + skip = "`BaseFederationServlet` does not support cancellation yet." + path = f"{CancellableFederationServlet.PREFIX}{CancellableFederationServlet.PATH}" def create_test_resource(self): |