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

Commit 62d3b91

Browse files
author
Sean Quah
committed
Respect the @cancellable flag for ReplicationEndpoints
While `ReplicationEndpoint`s register themselves via `JsonResource`, they pass a method that calls the handler, instead of the handler itself, to `register_paths`. As a result, `JsonResource` will not correctly pick up the `@cancellable` flag and we have to apply it ourselves. Signed-off-by: Sean Quah <seanq@element.io>
1 parent c3eb1e3 commit 62d3b91

1 file changed

Lines changed: 19 additions & 2 deletions

File tree

synapse/replication/http/_base.py

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@
2626

2727
from synapse.api.errors import HttpResponseException, SynapseError
2828
from synapse.http import RequestTimedOutError
29-
from synapse.http.server import HttpServer
29+
from synapse.http.server import HttpServer, is_method_cancellable
30+
from synapse.http.site import SynapseRequest
3031
from synapse.logging import opentracing
3132
from synapse.logging.opentracing import trace
3233
from synapse.types import JsonDict
@@ -310,6 +311,12 @@ def register(self, http_server: HttpServer) -> None:
310311
url_args = list(self.PATH_ARGS)
311312
method = self.METHOD
312313

314+
if self.CACHE and is_method_cancellable(self._handle_request):
315+
raise Exception(
316+
f"{self.__class__.__name__} has been marked as cancellable, but CACHE "
317+
"is set. The cancellable flag would have no effect."
318+
)
319+
313320
if self.CACHE:
314321
url_args.append("txn_id")
315322

@@ -324,7 +331,7 @@ def register(self, http_server: HttpServer) -> None:
324331
)
325332

326333
async def _check_auth_and_handle(
327-
self, request: Request, **kwargs: Any
334+
self, request: SynapseRequest, **kwargs: Any
328335
) -> Tuple[int, JsonDict]:
329336
"""Called on new incoming requests when caching is enabled. Checks
330337
if there is a cached response for the request and returns that,
@@ -340,8 +347,18 @@ async def _check_auth_and_handle(
340347
if self.CACHE:
341348
txn_id = kwargs.pop("txn_id")
342349

350+
# We ignore the `@cancellable` flag, since cancellation wouldn't interupt
351+
# `_handle_request` and `ResponseCache` does not handle cancellation
352+
# correctly yet. In particular, there may be issues to do with logging
353+
# context lifetimes.
354+
343355
return await self.response_cache.wrap(
344356
txn_id, self._handle_request, request, **kwargs
345357
)
346358

359+
# The `@cancellable` decorator may be applied to `_handle_request`. But we
360+
# told `HttpServer.register_paths` that our handler is `_check_auth_and_handle`,
361+
# so we have to set up the cancellable flag ourselves.
362+
request.is_render_cancellable = is_method_cancellable(self._handle_request)
363+
347364
return await self._handle_request(request, **kwargs)

0 commit comments

Comments
 (0)