Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix up pusher logging a bit #4716

Merged
merged 5 commits into from Feb 22, 2019
Merged
Diff settings

Always

Just for now

Correctly handle PusherConfigException

  • Loading branch information...
erikjohnston committed Feb 22, 2019
commit f2891d248756c968c346df052c796c0c69d6e764
Copy path View file
@@ -19,6 +19,7 @@
from twisted.internet import defer

from synapse.metrics.background_process_metrics import run_as_background_process
from synapse.push import PusherConfigException
from synapse.push.pusher import PusherFactory

logger = logging.getLogger(__name__)
@@ -222,6 +223,14 @@ def _start_pusher(self, pusherdict):
"""
try:
p = self.pusher_factory.create_pusher(pusherdict)
except PusherConfigException as e:
logger.warning(
"Pusher incorrectly configured user=%s, appid=%s, pushkey=%s: %s",
pusherdict.get('user_name'),
pusherdict.get('app_id'),
pusherdict.get('pushkey'),
e,

This comment has been minimized.

Copy link
@richvdh

richvdh Feb 22, 2019

Member

I think you generally need the type of the exception as well as the stringification

(mostly because KeyErrors are otherwise extremely confusing:

>>> try:
...     {}['a']
... except Exception as e:
...     print(e)
...
'a'

)

This comment has been minimized.

Copy link
@erikjohnston

erikjohnston Feb 22, 2019

Author Member

Err, we know the type since we're only catching the one PusherConfigException exception?

)
except Exception:
logger.exception("Couldn't start a pusher: caught Exception")
return
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.