forked from MirrorHub/synapse
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>
This commit is contained in:
parent
d38d242411
commit
6ee61b9052
3 changed files with 15 additions and 1 deletions
1
changelog.d/12705.misc
Normal file
1
changelog.d/12705.misc
Normal file
|
@ -0,0 +1 @@
|
||||||
|
Complain if a federation endpoint has the `@cancellable` flag, since some of the wrapper code may not handle cancellation correctly yet.
|
|
@ -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.errors import Codes, FederationDeniedError, SynapseError
|
||||||
from synapse.api.urls import FEDERATION_V1_PREFIX
|
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.servlet import parse_json_object_from_request
|
||||||
from synapse.http.site import SynapseRequest
|
from synapse.http.site import SynapseRequest
|
||||||
from synapse.logging.context import run_in_background
|
from synapse.logging.context import run_in_background
|
||||||
|
@ -373,6 +373,17 @@ class BaseFederationServlet:
|
||||||
if code is None:
|
if code is None:
|
||||||
continue
|
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(
|
server.register_paths(
|
||||||
method,
|
method,
|
||||||
(pattern,),
|
(pattern,),
|
||||||
|
|
|
@ -59,6 +59,8 @@ class BaseFederationServletCancellationTests(
|
||||||
):
|
):
|
||||||
"""Tests for `BaseFederationServlet` cancellation."""
|
"""Tests for `BaseFederationServlet` cancellation."""
|
||||||
|
|
||||||
|
skip = "`BaseFederationServlet` does not support cancellation yet."
|
||||||
|
|
||||||
path = f"{CancellableFederationServlet.PREFIX}{CancellableFederationServlet.PATH}"
|
path = f"{CancellableFederationServlet.PREFIX}{CancellableFederationServlet.PATH}"
|
||||||
|
|
||||||
def create_test_resource(self):
|
def create_test_resource(self):
|
||||||
|
|
Loading…
Reference in a new issue