From 14515cfe27fbc5432c8248c41ac858e1385c543b Mon Sep 17 00:00:00 2001 From: Unrud Date: Thu, 25 Aug 2016 04:29:02 +0200 Subject: [PATCH 1/5] Fix logger configuration Apply patch from #485 --- radicale/log.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/radicale/log.py b/radicale/log.py index 308e2a4..d1bae70 100644 --- a/radicale/log.py +++ b/radicale/log.py @@ -29,7 +29,7 @@ import signal import sys -def configure_from_file(filename, debug, logger): +def configure_from_file(logger, filename, debug): logging.config.fileConfig(filename) if debug: logger.setLevel(logging.DEBUG) From c091399f5ee9d4bec5ba9d975b6af755ef83cd36 Mon Sep 17 00:00:00 2001 From: Unrud Date: Thu, 25 Aug 2016 04:29:39 +0200 Subject: [PATCH 2/5] Write log to stderr Be consistent with python's default behavior and play nice with CGI. --- logging | 2 +- radicale/log.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/logging b/logging index b237214..be03f84 100644 --- a/logging +++ b/logging @@ -39,7 +39,7 @@ handlers = console,file # Console handler class = StreamHandler level = INFO -args = (sys.stdout,) +args = (sys.stderr,) formatter = simple [handler_file] diff --git a/radicale/log.py b/radicale/log.py index d1bae70..6849af6 100644 --- a/radicale/log.py +++ b/radicale/log.py @@ -55,9 +55,9 @@ def start(name="radicale", filename=None, debug=False): # Default configuration, standard output if filename: logger.warning( - "Logging configuration file '%s' not found, using stdout." % + "Logging configuration file '%s' not found, using stderr." % filename) - handler = logging.StreamHandler(sys.stdout) + handler = logging.StreamHandler(sys.stderr) handler.setFormatter(logging.Formatter("%(message)s")) logger.addHandler(handler) if debug: From 3b71ab960e3440e8d0a0fe61c270bfb91fded762 Mon Sep 17 00:00:00 2001 From: Unrud Date: Thu, 25 Aug 2016 04:33:14 +0200 Subject: [PATCH 3/5] Log exceptions (Fixes #447) Exceptions were just written to stderr but not into logs. --- radicale/__init__.py | 21 +++++++++++++++++++++ radicale/__main__.py | 7 ++++++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/radicale/__init__.py b/radicale/__init__.py index 0bc8d1d..1a73686 100644 --- a/radicale/__init__.py +++ b/radicale/__init__.py @@ -28,6 +28,7 @@ should have been included in this package. import base64 import contextlib +import io import itertools import os import posixpath @@ -130,9 +131,29 @@ class ThreadedHTTPSServer(socketserver.ThreadingMixIn, HTTPSServer): class RequestHandler(wsgiref.simple_server.WSGIRequestHandler): """HTTP requests handler.""" + + # These class attributes must be set before creating instance + logger = None + + def __init__(self, *args, **kwargs): + # Store exception for logging + self.error_stream = io.StringIO() + super().__init__(*args, **kwargs) + + def get_stderr(self): + return self.error_stream + def log_message(self, *args, **kwargs): """Disable inner logging management.""" + def handle(self): + super().handle() + # Log exception + error = self.error_stream.getvalue().strip("\n") + if error: + self.logger.error("An exception occurred during request:\n" + + error) + class Application: """WSGI application managing collections.""" diff --git a/radicale/__main__.py b/radicale/__main__.py index c76acca..4236607 100644 --- a/radicale/__main__.py +++ b/radicale/__main__.py @@ -104,7 +104,11 @@ def run(): if not configuration_found: logger.warning("Configuration file '%s' not found" % options.config) - serve(configuration, logger) + try: + serve(configuration, logger) + except Exception: + logger.exception("An exception occurred during server startup:") + exit(1) def serve(configuration, logger): @@ -175,6 +179,7 @@ def serve(configuration, logger): server_class.max_connections = configuration.getint( "server", "max_connections") + RequestHandler.logger = logger if not configuration.getboolean("server", "dns_lookup"): RequestHandler.address_string = lambda self: self.client_address[0] From de8c2f0909f5b95759d894b2f19698ab6013fb1d Mon Sep 17 00:00:00 2001 From: Unrud Date: Thu, 25 Aug 2016 05:02:31 +0200 Subject: [PATCH 4/5] Fix SIGHUP handler The function handler_generator seems useless and the return statement is missing. --- radicale/log.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/radicale/log.py b/radicale/log.py index 6849af6..f091506 100644 --- a/radicale/log.py +++ b/radicale/log.py @@ -46,10 +46,8 @@ def start(name="radicale", filename=None, debug=False): configure_from_file(logger, filename, debug) # Reload config on SIGHUP (UNIX only) if hasattr(signal, "SIGHUP"): - def handler_generator(logger, filename, debug): - def handler(signum, frame): - configure_from_file(logger, filename, debug) - handler = handler_generator(logger, filename, debug) + def handler(signum, frame): + configure_from_file(logger, filename, debug) signal.signal(signal.SIGHUP, handler) else: # Default configuration, standard output From e40e46e16462c0db49b13f05735515854c8c9766 Mon Sep 17 00:00:00 2001 From: Unrud Date: Thu, 25 Aug 2016 05:03:22 +0200 Subject: [PATCH 5/5] Don't disable existing loggers The logger is retrieved before configure_from_file is called and gets disabled, the same happens when the logging configuration is reloaded. --- radicale/log.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/radicale/log.py b/radicale/log.py index f091506..34d4e4c 100644 --- a/radicale/log.py +++ b/radicale/log.py @@ -30,7 +30,7 @@ import sys def configure_from_file(logger, filename, debug): - logging.config.fileConfig(filename) + logging.config.fileConfig(filename, disable_existing_loggers=False) if debug: logger.setLevel(logging.DEBUG) for handler in logger.handlers: