From 4cf4fe52b3a1631d4126eefcf179e36d8f05175e Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Sun, 21 Mar 2021 12:39:06 +0100 Subject: [PATCH 1/5] add more bugbear tests --- setup.cfg | 4 ++-- synapse/crypto/context_factory.py | 4 ++-- synapse/logging/context.py | 4 ++-- synapse/storage/database.py | 4 ++-- synapse/util/async_helpers.py | 4 ++-- synapse/util/caches/__init__.py | 2 +- tests/server.py | 2 +- 7 files changed, 12 insertions(+), 12 deletions(-) diff --git a/setup.cfg b/setup.cfg index 920868df203e..7329eed213d7 100644 --- a/setup.cfg +++ b/setup.cfg @@ -18,8 +18,8 @@ ignore = # E203: whitespace before ':' (which is contrary to pep8?) # E731: do not assign a lambda expression, use a def # E501: Line too long (black enforces this for us) -# B00: Subsection of the bugbear suite (TODO: add in remaining fixes) -ignore=W503,W504,E203,E731,E501,B00 +# B00*: Subsection of the bugbear suite (TODO: add in remaining fixes) +ignore=W503,W504,E203,E731,E501,B006,B007,B008 [isort] line_length = 88 diff --git a/synapse/crypto/context_factory.py b/synapse/crypto/context_factory.py index 4ca13011e54b..b23b7e11d135 100644 --- a/synapse/crypto/context_factory.py +++ b/synapse/crypto/context_factory.py @@ -191,9 +191,9 @@ def _context_info_cb(ssl_connection, where, ret): # ... we further assume that SSLClientConnectionCreator has set the # '_synapse_tls_verifier' attribute to a ConnectionVerifier object. tls_protocol._synapse_tls_verifier.verify_context_info_cb(ssl_connection, where) - except: # noqa: E722, taken from the twisted implementation + except BaseException as e: # taken from the twisted implementation logger.exception("Error during info_callback") - f = Failure() + f = Failure(e) tls_protocol.failVerification(f) diff --git a/synapse/logging/context.py b/synapse/logging/context.py index 1a7ea4fa9629..d4c9bc9ac567 100644 --- a/synapse/logging/context.py +++ b/synapse/logging/context.py @@ -689,10 +689,10 @@ def run_in_background(f, *args, **kwargs) -> defer.Deferred: current = current_context() try: res = f(*args, **kwargs) - except: # noqa: E722 + except Exception as e: # the assumption here is that the caller doesn't want to be disturbed # by synchronous exceptions, so let's turn them into Failures. - return defer.fail() + return defer.fail(e) if isinstance(res, types.CoroutineType): res = defer.ensureDeferred(res) diff --git a/synapse/storage/database.py b/synapse/storage/database.py index f1ba529a2d76..8642ff7f2618 100644 --- a/synapse/storage/database.py +++ b/synapse/storage/database.py @@ -670,10 +670,10 @@ async def runInteraction( for after_callback, after_args, after_kwargs in after_callbacks: after_callback(*after_args, **after_kwargs) - except: # noqa: E722, as we reraise the exception this is fine. + except Exception as e: for after_callback, after_args, after_kwargs in exception_callbacks: after_callback(*after_args, **after_kwargs) - raise + raise e return cast(R, result) diff --git a/synapse/util/async_helpers.py b/synapse/util/async_helpers.py index f33c115844db..106ffd17822d 100644 --- a/synapse/util/async_helpers.py +++ b/synapse/util/async_helpers.py @@ -496,8 +496,8 @@ def time_it_out(): try: deferred.cancel() - except: # noqa: E722, if we throw any exception it'll break time outs - logger.exception("Canceller failed during timeout") + except Exception as e: # if we throw any exception it'll break time outs + logger.exception("Canceller failed during timeout", exc_info=e) # the cancel() call should have set off a chain of errbacks which # will have errbacked new_d, but in case it hasn't, errback it now. diff --git a/synapse/util/caches/__init__.py b/synapse/util/caches/__init__.py index e676c2cac46b..f96870633447 100644 --- a/synapse/util/caches/__init__.py +++ b/synapse/util/caches/__init__.py @@ -116,7 +116,7 @@ def register_cache( """ if resizable: if not resize_callback: - resize_callback = getattr(cache, "set_cache_factor") + resize_callback = cache.set_cache_factor # type: ignore add_resizable_cache(cache_name, resize_callback) metric = CacheMetric(cache, cache_type, cache_name, collect_callback) diff --git a/tests/server.py b/tests/server.py index 2287d200763b..c571c29d1968 100644 --- a/tests/server.py +++ b/tests/server.py @@ -593,7 +593,7 @@ def flush(self, maxbytes=None): if self.disconnected: return - if getattr(self.other, "transport") is None: + if hasattr(self.other, "transport"): # the other has no transport yet; reschedule if self.autoflush: self._reactor.callLater(0.0, self.flush) From 09c22ff4e4027b82e17e338282d558d044307b48 Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Sun, 21 Mar 2021 13:06:04 +0100 Subject: [PATCH 2/5] add more bugbear tests --- changelog.d/9659.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/9659.misc diff --git a/changelog.d/9659.misc b/changelog.d/9659.misc new file mode 100644 index 000000000000..23c36929dc0b --- /dev/null +++ b/changelog.d/9659.misc @@ -0,0 +1 @@ +Add and fix more bugbear lint violations. \ No newline at end of file From 712d2b0bc057338d577ebc030af02b4da329d591 Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Sun, 21 Mar 2021 17:39:18 +0100 Subject: [PATCH 3/5] fix tests --- tests/server.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/server.py b/tests/server.py index c571c29d1968..57cc4ac60563 100644 --- a/tests/server.py +++ b/tests/server.py @@ -593,7 +593,7 @@ def flush(self, maxbytes=None): if self.disconnected: return - if hasattr(self.other, "transport"): + if not hasattr(self.other, "transport"): # the other has no transport yet; reschedule if self.autoflush: self._reactor.callLater(0.0, self.flush) From 18cee9343d9f720f799902083fd266b07866370b Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Wed, 24 Mar 2021 12:21:14 +0100 Subject: [PATCH 4/5] remove as e's --- synapse/crypto/context_factory.py | 4 ++-- synapse/logging/context.py | 4 ++-- synapse/storage/database.py | 4 ++-- synapse/util/async_helpers.py | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/synapse/crypto/context_factory.py b/synapse/crypto/context_factory.py index b23b7e11d135..c644b4dfc5ed 100644 --- a/synapse/crypto/context_factory.py +++ b/synapse/crypto/context_factory.py @@ -191,9 +191,9 @@ def _context_info_cb(ssl_connection, where, ret): # ... we further assume that SSLClientConnectionCreator has set the # '_synapse_tls_verifier' attribute to a ConnectionVerifier object. tls_protocol._synapse_tls_verifier.verify_context_info_cb(ssl_connection, where) - except BaseException as e: # taken from the twisted implementation + except BaseException: # taken from the twisted implementation logger.exception("Error during info_callback") - f = Failure(e) + f = Failure() tls_protocol.failVerification(f) diff --git a/synapse/logging/context.py b/synapse/logging/context.py index d4c9bc9ac567..03cf3c2b8ed3 100644 --- a/synapse/logging/context.py +++ b/synapse/logging/context.py @@ -689,10 +689,10 @@ def run_in_background(f, *args, **kwargs) -> defer.Deferred: current = current_context() try: res = f(*args, **kwargs) - except Exception as e: + except Exception: # the assumption here is that the caller doesn't want to be disturbed # by synchronous exceptions, so let's turn them into Failures. - return defer.fail(e) + return defer.fail() if isinstance(res, types.CoroutineType): res = defer.ensureDeferred(res) diff --git a/synapse/storage/database.py b/synapse/storage/database.py index 8642ff7f2618..5b0b9a20bf86 100644 --- a/synapse/storage/database.py +++ b/synapse/storage/database.py @@ -670,10 +670,10 @@ async def runInteraction( for after_callback, after_args, after_kwargs in after_callbacks: after_callback(*after_args, **after_kwargs) - except Exception as e: + except Exception: for after_callback, after_args, after_kwargs in exception_callbacks: after_callback(*after_args, **after_kwargs) - raise e + raise return cast(R, result) diff --git a/synapse/util/async_helpers.py b/synapse/util/async_helpers.py index 106ffd17822d..c3b2d981eadb 100644 --- a/synapse/util/async_helpers.py +++ b/synapse/util/async_helpers.py @@ -496,8 +496,8 @@ def time_it_out(): try: deferred.cancel() - except Exception as e: # if we throw any exception it'll break time outs - logger.exception("Canceller failed during timeout", exc_info=e) + except Exception: # if we throw any exception it'll break time outs + logger.exception("Canceller failed during timeout") # the cancel() call should have set off a chain of errbacks which # will have errbacked new_d, but in case it hasn't, errback it now. From 9409095abf3e195a8135e473e509c1b4d9c8403c Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 24 Mar 2021 08:48:47 -0400 Subject: [PATCH 5/5] Update changelog. --- changelog.d/9659.misc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/9659.misc b/changelog.d/9659.misc index 23c36929dc0b..6602c1cc6a98 100644 --- a/changelog.d/9659.misc +++ b/changelog.d/9659.misc @@ -1 +1 @@ -Add and fix more bugbear lint violations. \ No newline at end of file +Introduce bugbear to the test suite and fix some of it's lint violations.