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

Logging improvements for the pusher #4691

Merged
merged 2 commits into from Feb 20, 2019
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.
+15 −13
Diff settings

Always

Just for now

Copy path View file
@@ -0,0 +1 @@
Improve the logging in the pusher process.
Copy path View file
@@ -333,10 +333,10 @@ def dispatch_push(self, event, tweaks, badge):
defer.returnValue([])
try:
resp = yield self.http_client.post_json_get_json(self.url, notification_dict)
except Exception:
logger.warn(
"Failed to push event %s to %s",
event.event_id, self.name, exc_info=True,
except Exception as e:
logger.warning(
"Failed to push event %s to %s: %s %s",
event.event_id, self.name, type(e), e,

This comment has been minimized.

Copy link
@erikjohnston

erikjohnston Feb 20, 2019

Member

I'm a bit cautious about catching all the exceptions here in case it masks actual programming exceptions, can we not catch HttpResponseException etc?

OTOH, it was already logging at warning, so this isn't the end of the world

This comment has been minimized.

Copy link
@richvdh

richvdh Feb 20, 2019

Author Member

wellll....

normally I'd agree with you, but the exception that was causing a problem is a twisted.web._newclient.RequestGenerationFailed: [<twisted.python.failure.Failure twisted.internet.defer.CancelledError: >]. AFAICT that doesn't seem to be part of twisted's public interface, so it seems to be not a great idea to try and catch it here.

This comment has been minimized.

Copy link
@erikjohnston

erikjohnston Feb 20, 2019

Member

Oh great. It also doesn't look like I added RequestSendFailed stuff to the normal http client either.

)
defer.returnValue(False)
rejected = []
@@ -367,10 +367,10 @@ def _send_badge(self, badge):
}
try:
resp = yield self.http_client.post_json_get_json(self.url, d)
except Exception:
logger.warn(
"Failed to send badge count to %s",
self.name, exc_info=True,
except Exception as e:
logger.warning(
"Failed to send badge count to %s: %s %s",
self.name, type(e), e,
)
defer.returnValue(False)
rejected = []
Copy path View file
@@ -52,11 +52,12 @@ def __init__(self, hs):
logger.info("defined email pusher type")

def create_pusher(self, pusherdict):
logger.info("trying to create_pusher for %r", pusherdict)

if pusherdict['kind'] in self.pusher_types:
logger.info("found pusher")
return self.pusher_types[pusherdict['kind']](self.hs, pusherdict)
kind = pusherdict['kind']
f = self.pusher_types.get(kind, None)
if not f:
return None
logger.info("creating %s pusher for %r", kind, pusherdict)
return f(self.hs, pusherdict)

def _create_email_pusher(self, _hs, pusherdict):
app_name = self._app_name_from_pusherdict(pusherdict)
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.