From 1fa08f48d6cfc6cf7b52b2e2ea7ea253a520cf6c Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 1 Mar 2021 14:30:01 -0500 Subject: [PATCH 1/8] Add a proper return type to run_in_background. --- synapse/logging/context.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/logging/context.py b/synapse/logging/context.py index 78e27bfb00ed..3c8584eb7d97 100644 --- a/synapse/logging/context.py +++ b/synapse/logging/context.py @@ -669,7 +669,7 @@ def g(*args, **kwargs): return g -def run_in_background(f, *args, **kwargs): +def run_in_background(f, *args, **kwargs) -> defer.Deferred: """Calls a function, ensuring that the current context is restored after return from the function, and that the sentinel context is set once the deferred returned by the function completes. From 3ca2ea3127ca9217a8193f7e50275fe2d07a17d0 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 1 Mar 2021 14:30:11 -0500 Subject: [PATCH 2/8] Ignore an unknown attribute. --- tests/test_utils/logging_setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_utils/logging_setup.py b/tests/test_utils/logging_setup.py index 52ae5c57137f..74568b34f8c8 100644 --- a/tests/test_utils/logging_setup.py +++ b/tests/test_utils/logging_setup.py @@ -28,7 +28,7 @@ class ToTwistedHandler(logging.Handler): def emit(self, record): log_entry = self.format(record) log_level = record.levelname.lower().replace("warning", "warn") - self.tx_log.emit( + self.tx_log.emit( # type: ignore twisted.logger.LogLevel.levelWithName(log_level), "{entry}", entry=log_entry ) From f901773605c1ed10281956d769915fdd1bac4d7b Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 1 Mar 2021 14:39:35 -0500 Subject: [PATCH 3/8] Header access. --- synapse/http/federation/well_known_resolver.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/http/federation/well_known_resolver.py b/synapse/http/federation/well_known_resolver.py index 4def7d763304..ecd63e6596b4 100644 --- a/synapse/http/federation/well_known_resolver.py +++ b/synapse/http/federation/well_known_resolver.py @@ -322,7 +322,8 @@ def _cache_period_from_headers( def _parse_cache_control(headers: Headers) -> Dict[bytes, Optional[bytes]]: cache_controls = {} - for hdr in headers.getRawHeaders(b"cache-control", []): + cache_control_headers = headers.getRawHeaders(b"cache-control") or [] + for hdr in cache_control_headers: for directive in hdr.split(b","): splits = [x.strip() for x in directive.split(b"=", 1)] k = splits[0].lower() From 76f9ebe03f476dbb871384247435743fb4108688 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 1 Mar 2021 15:34:14 -0500 Subject: [PATCH 4/8] Minor changes to tests. --- tests/replication/_base.py | 27 +++++++++++++++++---------- tests/server.py | 2 +- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/tests/replication/_base.py b/tests/replication/_base.py index f6a6aed35e2b..20940c81071e 100644 --- a/tests/replication/_base.py +++ b/tests/replication/_base.py @@ -22,6 +22,7 @@ from twisted.internet.task import LoopingCall from twisted.web.http import HTTPChannel from twisted.web.resource import Resource +from twisted.web.server import Request, Site from synapse.app.generic_worker import ( GenericWorkerReplicationHandler, @@ -32,7 +33,10 @@ from synapse.replication.http import ReplicationRestResource from synapse.replication.tcp.handler import ReplicationCommandHandler from synapse.replication.tcp.protocol import ClientReplicationStreamProtocol -from synapse.replication.tcp.resource import ReplicationStreamProtocolFactory +from synapse.replication.tcp.resource import ( + ReplicationStreamProtocolFactory, + ServerReplicationStreamProtocol, +) from synapse.server import HomeServer from synapse.util import Clock @@ -59,7 +63,9 @@ def prepare(self, reactor, clock, hs): # build a replication server server_factory = ReplicationStreamProtocolFactory(hs) self.streamer = hs.get_replication_streamer() - self.server = server_factory.buildProtocol(None) + self.server = server_factory.buildProtocol( + None + ) # type: ServerReplicationStreamProtocol # Make a new HomeServer object for the worker self.reactor.lookups["testserv"] = "1.2.3.4" @@ -155,9 +161,7 @@ def handle_http_replication_attempt(self) -> SynapseRequest: request_factory = OneShotRequestFactory() # Set up the server side protocol - channel = _PushHTTPChannel(self.reactor) - channel.requestFactory = request_factory - channel.site = self.site + channel = _PushHTTPChannel(self.reactor, request_factory, self.site) # Connect client to server and vice versa. client_to_server_transport = FakeTransport( @@ -188,8 +192,9 @@ def assert_request_is_get_repl_stream_updates( fetching updates for given stream. """ + path = request.path # type: bytes # type: ignore self.assertRegex( - request.path, + path, br"^/_synapse/replication/get_repl_stream_updates/%s/[^/]+$" % (stream_name.encode("ascii"),), ) @@ -390,9 +395,7 @@ def _handle_http_replication_attempt(self, hs, repl_port): request_factory = OneShotRequestFactory() # Set up the server side protocol - channel = _PushHTTPChannel(self.reactor) - channel.requestFactory = request_factory - channel.site = self._hs_to_site[hs] + channel = _PushHTTPChannel(self.reactor, request_factory, self._hs_to_site[hs]) # Connect client to server and vice versa. client_to_server_transport = FakeTransport( @@ -475,9 +478,13 @@ class _PushHTTPChannel(HTTPChannel): makes it very hard to test. """ - def __init__(self, reactor: IReactorTime): + def __init__( + self, reactor: IReactorTime, request_factory: Callable[..., Request], site: Site + ): super().__init__() self.reactor = reactor + self.requestFactory = request_factory + self.site = site self._pull_to_push_producer = None # type: Optional[_PullToPushProducer] diff --git a/tests/server.py b/tests/server.py index 939a0008ca2e..863f6da7385c 100644 --- a/tests/server.py +++ b/tests/server.py @@ -188,7 +188,7 @@ def getResourceFor(self, request): def make_request( reactor, - site: Site, + site: Union[Site, FakeSite], method, path, content=b"", From 16bfe756e8735babdb135461712111c95de476ea Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 2 Mar 2021 11:09:54 -0500 Subject: [PATCH 5/8] Fix calls to logger.error. --- synapse/federation/federation_server.py | 2 +- synapse/handlers/pagination.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/federation/federation_server.py b/synapse/federation/federation_server.py index 2f832b47f65c..0f4e9e4eb951 100644 --- a/synapse/federation/federation_server.py +++ b/synapse/federation/federation_server.py @@ -361,7 +361,7 @@ async def process_pdus_for_room(room_id: str): logger.error( "Failed to handle PDU %s", event_id, - exc_info=(f.type, f.value, f.getTracebackObject()), + exc_info=(f.type, f.value, f.getTracebackObject()), # type: ignore ) await concurrently_execute( diff --git a/synapse/handlers/pagination.py b/synapse/handlers/pagination.py index 059064a4eb44..66dc886c8100 100644 --- a/synapse/handlers/pagination.py +++ b/synapse/handlers/pagination.py @@ -285,7 +285,7 @@ async def _purge_history( except Exception: f = Failure() logger.error( - "[purge] failed", exc_info=(f.type, f.value, f.getTracebackObject()) + "[purge] failed", exc_info=(f.type, f.value, f.getTracebackObject()) # type: ignore ) self._purges_by_id[purge_id].status = PurgeStatus.STATUS_FAILED finally: From 2b22d4cf2871f9925ea9d158ec6cc8f13daefc66 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 2 Mar 2021 11:36:45 -0500 Subject: [PATCH 6/8] Add type hints to the _log method. --- synapse/config/logger.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/synapse/config/logger.py b/synapse/config/logger.py index e56cf846f516..999aecce5c78 100644 --- a/synapse/config/logger.py +++ b/synapse/config/logger.py @@ -21,8 +21,10 @@ from string import Template import yaml +from zope.interface import implementer from twisted.logger import ( + ILogObserver, LogBeginner, STDLibLogObserver, eventAsText, @@ -227,7 +229,8 @@ def factory(*args, **kwargs): threadlocal = threading.local() - def _log(event): + @implementer(ILogObserver) + def _log(event: dict) -> None: if "log_text" in event: if event["log_text"].startswith("DNSDatagramProtocol starting on "): return From 2325c7380cd43efaf791d46a38ee436ff3b5b443 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 3 Mar 2021 15:52:53 -0500 Subject: [PATCH 7/8] Newsfragment --- changelog.d/9543.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/9543.misc diff --git a/changelog.d/9543.misc b/changelog.d/9543.misc new file mode 100644 index 000000000000..14c7b78dd97e --- /dev/null +++ b/changelog.d/9543.misc @@ -0,0 +1 @@ +Fix incorrect type hints. From 6d913a003f718763897acc0786891f4fdd5be3cf Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 8 Mar 2021 09:57:19 -0500 Subject: [PATCH 8/8] Wrap non-Deferreds in defer.succeed. --- synapse/logging/context.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/synapse/logging/context.py b/synapse/logging/context.py index 3c8584eb7d97..1a7ea4fa9629 100644 --- a/synapse/logging/context.py +++ b/synapse/logging/context.py @@ -697,8 +697,10 @@ def run_in_background(f, *args, **kwargs) -> defer.Deferred: if isinstance(res, types.CoroutineType): res = defer.ensureDeferred(res) + # At this point we should have a Deferred, if not then f was a synchronous + # function, wrap it in a Deferred for consistency. if not isinstance(res, defer.Deferred): - return res + return defer.succeed(res) if res.called and not res.paused: # The function should have maintained the logcontext, so we can