Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Log looping call exceptions #4008

Merged
merged 4 commits into from
Oct 9, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion synapse/handlers/appservice.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
event_processing_loop_room_count,
)
from synapse.metrics.background_process_metrics import run_as_background_process
from synapse.util import make_log_failure_errback
from synapse.util.logcontext import make_deferred_yieldable, run_in_background
from synapse.util.metrics import Measure

Expand Down Expand Up @@ -112,7 +113,11 @@ def handle_event(event):

if not self.started_scheduler:
def start_scheduler():
return self.scheduler.start().addErrback(log_failure)
return self.scheduler.start().addErrback(
make_log_failure_errback(
"Application Services Failure",
)
)
run_as_background_process("as_scheduler", start_scheduler)
self.started_scheduler = True

Expand Down
43 changes: 29 additions & 14 deletions synapse/util/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
# See the License for the specific language governing permissions and
# limitations under the License.

import functools
import logging
from itertools import islice

Expand Down Expand Up @@ -67,9 +66,12 @@ def looping_call(self, f, msec):
f(function): The function to call repeatedly.
msec(float): How long to wait between calls in milliseconds.
"""
call = task.LoopingCall(_log_exception_wrapper(f))
call = task.LoopingCall(f)
call.clock = self._reactor
call.start(msec / 1000.0, now=False)
d = call.start(msec / 1000.0, now=False)
d.addErrback(make_log_failure_errback(
"Looping call died", consumeErrors=False,
))
return call

def call_later(self, delay, callback, *args, **kwargs):
Expand Down Expand Up @@ -112,17 +114,30 @@ def batch_iter(iterable, size):
return iter(lambda: tuple(islice(sourceiter, size)), ())


def _log_exception_wrapper(f):
"""Used to wrap looping calls to log loudly if they get killed
def make_log_failure_errback(msg, consumeErrors=True):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you not have msg and consumeErrors as params to log_failure and then just do addErrback(log_failure, msg) ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... yes.

This PR is going terribly.

"""Creates a function suitable for passing to `Deferred.addErrback` that
logs any failures that occur.

Args:
msg (str): Message to log
consumeErrors (bool): If true consumes the failure, otherwise passes
on down the callback chain

Returns:
func(Failure)
"""

@functools.wraps(f)
def wrap(*args, **kwargs):
try:
logger.info("Running looping call")
return f(*args, **kwargs)
except: # noqa: E722, as we reraise the exception this is fine.
logger.exception("Looping called died")
raise
def log_failure(failure):
logger.error(
msg,
exc_info=(
failure.type,
failure.value,
failure.getTracebackObject()
)
)

if not consumeErrors:
return failure

return wrap
return log_failure