From 0b730bb6c6ee87b56c972aff9a45baa29c897cdf Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Mon, 1 Mar 2021 23:53:19 +0100 Subject: [PATCH 1/3] add logging --- synapse/util/async_helpers.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/synapse/util/async_helpers.py b/synapse/util/async_helpers.py index 719e35b78d72..fbb814f55475 100644 --- a/synapse/util/async_helpers.py +++ b/synapse/util/async_helpers.py @@ -76,11 +76,12 @@ def __init__(self, deferred: defer.Deferred, consumeErrors: bool = False): def callback(r): object.__setattr__(self, "_result", (True, r)) while self._observers: + observer = None try: - # TODO: Handle errors here. - self._observers.pop().callback(r) - except Exception: - pass + observer = self._observers.pop() + observer.callback(r) + except Exception as e: + logger.warning("%r threw an exception on .callback(%r), ignoring...", observer, r, exc_info=e) return r def errback(f): @@ -90,11 +91,12 @@ def errback(f): # traces when we `await` on one of the observer deferreds. f.value.__failure__ = f + observer = None try: - # TODO: Handle errors here. - self._observers.pop().errback(f) - except Exception: - pass + observer = self._observers.pop() + observer.errback(f) + except Exception as e: + logger.warning("%r threw an exception on .errback(%r), ignoring...", observer, f, exc_info=e) if consumeErrors: return None From a6da27291bee49de515b332b7675a64701f50ad2 Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Tue, 2 Mar 2021 00:05:06 +0100 Subject: [PATCH 2/3] lint, optimization, and news --- changelog.d/9523.misc | 1 + synapse/util/async_helpers.py | 20 ++++++++++++++------ 2 files changed, 15 insertions(+), 6 deletions(-) create mode 100644 changelog.d/9523.misc diff --git a/changelog.d/9523.misc b/changelog.d/9523.misc new file mode 100644 index 000000000000..f03e939efb2e --- /dev/null +++ b/changelog.d/9523.misc @@ -0,0 +1 @@ +Add extra logging to ObservableDeferred when callbacks throw exceptions. \ No newline at end of file diff --git a/synapse/util/async_helpers.py b/synapse/util/async_helpers.py index fbb814f55475..a59882631f9d 100644 --- a/synapse/util/async_helpers.py +++ b/synapse/util/async_helpers.py @@ -76,12 +76,16 @@ def __init__(self, deferred: defer.Deferred, consumeErrors: bool = False): def callback(r): object.__setattr__(self, "_result", (True, r)) while self._observers: - observer = None + observer = self._observers.pop() try: - observer = self._observers.pop() observer.callback(r) except Exception as e: - logger.warning("%r threw an exception on .callback(%r), ignoring...", observer, r, exc_info=e) + logger.warning( + "%r threw an exception on .callback(%r), ignoring...", + observer, + r, + exc_info=e, + ) return r def errback(f): @@ -91,12 +95,16 @@ def errback(f): # traces when we `await` on one of the observer deferreds. f.value.__failure__ = f - observer = None + observer = self._observers.pop() try: - observer = self._observers.pop() observer.errback(f) except Exception as e: - logger.warning("%r threw an exception on .errback(%r), ignoring...", observer, f, exc_info=e) + logger.warning( + "%r threw an exception on .errback(%r), ignoring...", + observer, + f, + exc_info=e, + ) if consumeErrors: return None From fea458380b817ef79521a50862a711ed6de3daaa Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Thu, 4 Mar 2021 18:16:21 +0100 Subject: [PATCH 3/3] change warning -> exception this way it'll get logged via sentry as well --- synapse/util/async_helpers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/util/async_helpers.py b/synapse/util/async_helpers.py index a59882631f9d..f33c115844db 100644 --- a/synapse/util/async_helpers.py +++ b/synapse/util/async_helpers.py @@ -80,7 +80,7 @@ def callback(r): try: observer.callback(r) except Exception as e: - logger.warning( + logger.exception( "%r threw an exception on .callback(%r), ignoring...", observer, r, @@ -99,7 +99,7 @@ def errback(f): try: observer.errback(f) except Exception as e: - logger.warning( + logger.exception( "%r threw an exception on .errback(%r), ignoring...", observer, f,