This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: logging.Handler.handleError should be called from logging.Handler.handle
Type: behavior Stage:
Components: Library (Lib) Versions: Python 3.6
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: palaviv, vinay.sajip
Priority: normal Keywords: patch

Created on 2016-04-06 20:03 by palaviv, last changed 2022-04-11 14:58 by admin.

Files
File name Uploaded Description Edit
logging-handle-error.patch palaviv, 2016-04-06 20:03 review
logging-SafeHandle.patch palaviv, 2016-04-10 21:11 review
logging-handleException.patch palaviv, 2016-04-10 21:12 review
Messages (3)
msg262962 - (view) Author: Aviv Palivoda (palaviv) * Date: 2016-04-06 20:03
Currently all the stdlib logging handlers (except BufferingHandler) emit method have the following structure:
    def emit(self, record):
        try:
            // do the emit
        except Exception:
            self.handleError(record)

I suggest changing this so that the handle method will do the exception handling of the emit:

    def handle(self, record): 
        rv = self.filter(record)
        if rv:
            self.acquire()
            try:
                self.emit(record)
            except Exception:
                self.handleError(record)
            finally:
                self.release()
        return rv

Now the emit() method can be override without the need to handle it's own exceptions.

I think this is more clear with the current documentation as well. For example in the handleError function it says that "This method should be called from handlers when an exception is encountered during an emit() call". In addition in the only example that implement the emit() function https://docs.python.org/3/howto/logging-cookbook.html#speaking-logging-messages there is no error handling at all.
msg262965 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2016-04-06 21:16
Thanks for the suggestion, but I'm not sure this can be accepted without violating backward compatibility. It forces each handler implementation to either accept the base implementation of handleError(), or to override it. And if there are existing handler implementations out there (i.e. not in the stdlib) which don't call handleError in their emit() (i.e. allow exceptions to propagate upwards), their behaviour would change, wouldn't it? That's not backwards-compatible.
msg263151 - (view) Author: Aviv Palivoda (palaviv) * Date: 2016-04-10 21:11
I see the backwards compatibility issue. I have two suggestion's how to improve the code without breaking backwards compatibility:

1. Add a new Handler class named SafeHandler that will implement handle in the way suggested in the previous patch. All the stdlib handler's will inherit from SafeHandler and in the documentation we will suggest using this handler. I am adding a patch (logging-SafeGandle.patch) with this suggestion.

2. Add new module-level attribute handleException that will deprecate raiseException. When raiseException is set or when handleException is at the default value the current behavior will remain. You can set handleException to the following values:
   a. print - print exception to stderr
   b. propagate - propagate exception
   c. ignore - swallow the exception

The current behavior has a few inconsistencies with raiseException. For example when raiseException is True the handleError method don't propagate the excpetion as will be expected. This patch will solve this problem in addition to the one named in the previous message. I am adding a patch with this suggested behavior (logging-handleException). 

Both patch's are in initial steps and are just to show more clearly what i am suggesting. There are currently no tests or documentation for either patch. Both patch's pass all logging test's without any changes to prove the backwards compatibility.
History
Date User Action Args
2022-04-11 14:58:29adminsetgithub: 70892
2016-04-10 21:12:18palavivsetfiles: + logging-handleException.patch
2016-04-10 21:12:00palavivsetfiles: + logging-SafeHandle.patch

messages: + msg263151
2016-04-06 21:16:35vinay.sajipsetmessages: + msg262965
2016-04-06 20:03:37palavivcreate