From 808d8e06aa6a3bdd9016f82d98088111e5741f4c Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 21 Aug 2018 11:09:46 +0100 Subject: [PATCH 1/3] Don't log exceptions when failing to fetch server keys Not being able to resolve or connect to remote servers is an expected error, so we shouldn't log at ERROR with stacktraces. --- synapse/crypto/keyclient.py | 8 ++++++-- synapse/federation/transport/server.py | 4 ++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/synapse/crypto/keyclient.py b/synapse/crypto/keyclient.py index c20a32096..e94400b8e 100644 --- a/synapse/crypto/keyclient.py +++ b/synapse/crypto/keyclient.py @@ -18,7 +18,9 @@ import logging from canonicaljson import json from twisted.internet import defer, reactor +from twisted.internet.error import ConnectError from twisted.internet.protocol import Factory +from twisted.names.error import DomainError from twisted.web.http import HTTPClient from synapse.http.endpoint import matrix_federation_endpoint @@ -47,12 +49,14 @@ def fetch_server_key(server_name, tls_client_options_factory, path=KEY_API_V1): server_response, server_certificate = yield protocol.remote_key defer.returnValue((server_response, server_certificate)) except SynapseKeyClientError as e: - logger.exception("Error getting key for %r" % (server_name,)) + logger.warn("Error getting key for %r: %s", server_name, e) if e.status.startswith("4"): # Don't retry for 4xx responses. raise IOError("Cannot get key for %r" % server_name) + except (ConnectError, DomainError) as e: + logger.warn("Error getting key for %r: %s", server_name, e) except Exception as e: - logger.exception(e) + logger.exception("Error getting key for %r", server_name) raise IOError("Cannot get key for %r" % server_name) diff --git a/synapse/federation/transport/server.py b/synapse/federation/transport/server.py index 77969a4f3..33a37d449 100644 --- a/synapse/federation/transport/server.py +++ b/synapse/federation/transport/server.py @@ -261,10 +261,10 @@ class BaseFederationServlet(object): except NoAuthenticationError: origin = None if self.REQUIRE_AUTH: - logger.exception("authenticate_request failed") + logger.warn("authenticate_request failed") raise except Exception: - logger.exception("authenticate_request failed") + logger.warn("authenticate_request failed") raise if origin: From 79d3b4689e1e9fa5304beb91c6b738f9131b6aa8 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 21 Aug 2018 11:21:48 +0100 Subject: [PATCH 2/3] Newsfile --- changelog.d/3727.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/3727.misc diff --git a/changelog.d/3727.misc b/changelog.d/3727.misc new file mode 100644 index 000000000..0b83220d9 --- /dev/null +++ b/changelog.d/3727.misc @@ -0,0 +1 @@ +Log failure to authenticate remote servers as warnings (without stack traces) From c2c153dd3b97ec98ec7ef295deeb466d72027a01 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 21 Aug 2018 11:41:07 +0100 Subject: [PATCH 3/3] Log more detail when we fail to authenticate request --- synapse/federation/transport/server.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/synapse/federation/transport/server.py b/synapse/federation/transport/server.py index 33a37d449..7a993fd1c 100644 --- a/synapse/federation/transport/server.py +++ b/synapse/federation/transport/server.py @@ -261,10 +261,10 @@ class BaseFederationServlet(object): except NoAuthenticationError: origin = None if self.REQUIRE_AUTH: - logger.warn("authenticate_request failed") + logger.warn("authenticate_request failed: missing authentication") raise - except Exception: - logger.warn("authenticate_request failed") + except Exception as e: + logger.warn("authenticate_request failed: %s", e) raise if origin: