classification
Title: Exception reporting in `logging.config`
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: vinay.sajip Nosy List: Mariatta, cool-RR, python-dev, vinay.sajip
Priority: normal Keywords: patch

Created on 2016-10-01 18:20 by cool-RR, last changed 2016-10-03 19:00 by cool-RR. This issue is now closed.

Files
File name Uploaded Description Edit
raisefrom.patch sreyas, 2016-10-02 15:48 added raise from to preserve original tracebacks review
Messages (9)
msg277826 - (view) Author: Ram Rachum (cool-RR) * Date: 2016-10-01 18:20
In `logging.config.DictConfigurator.configure`, there are exceptions that are caught and then replaced with `ValueError` exceptions. I think you should use the `raise x from y` syntax so that the original tracebacks will be preserved. (I'm debugging such an exception now and it's quite frustrating that I'm not seeing the full traceback of the original exception.)
msg277828 - (view) Author: Ram Rachum (cool-RR) * Date: 2016-10-01 18:27
I now see similar raises that could use a `from` in other places in this  module, so I'd suggest going over the module and putting `from`s all over the place.
msg277889 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2016-10-02 13:19
This is marked as relevant to Python 3.7, so I ran a test script using the latest Python sources:

import logging
import logging.config
import sys

def main():
    config = {
        'version': 1,
        'formatters': {
            'default': {
                '()': 'foo',
            },
        }
    }
    logging.config.dictConfig(config)


if __name__ == '__main__':
    sys.exit(main())

The exception printouts with/without "raise ... from" are shown in this Gist: 

https://gist.github.com/vsajip/a17b51695bbe16bd3c9e156405388e57

I can't see much difference in terms of figuring out where the exception came from. The only substantial difference is one line "During handling of the above exception, another exception occurred:" with another "The above exception was the direct cause of the following exception:".
msg277890 - (view) Author: Ram Rachum (cool-RR) * Date: 2016-10-02 13:25
You're right Vinay: I missed that first traceback because I wasn't running code in the console, I was running it with a debugger. (I'll ask my debugger vendor to maybe include these extra tracebacks.)

But in any case, I think that the correct way to raise would be with `from`, don't you think? (It's also more likely that debugger vendor would include these tracebacks rather than the "During handling" traceback.
msg277899 - (view) Author: Ram Rachum (cool-RR) * Date: 2016-10-02 15:59
Thanks for the patch sreyas! Maybe we should also take this opportunity to remove the string formatting from the exception that shows the name of the original exception? Because when you think about it, it's just a poor man's `raise from`.
msg277979 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2016-10-03 18:23
Patch as posted will not work (contains a syntax error), and was thus never tested. Never mind, I will address this soon.
msg277987 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-10-03 18:46
New changeset 69bf09bf4952 by Vinay Sajip in branch 'default':
Closes #28335: used 'raise from' in logging configuration code.
https://hg.python.org/cpython/rev/69bf09bf4952
msg277989 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-10-03 18:51
New changeset 8c005be54305 by Vinay Sajip in branch 'default':
Issue #28335: made minor improvement to implementation.
https://hg.python.org/cpython/rev/8c005be54305
msg277991 - (view) Author: Ram Rachum (cool-RR) * Date: 2016-10-03 19:00
Thanks for the quick fix Vinay!
History
Date User Action Args
2016-10-03 19:00:58cool-RRsetmessages: + msg277991
2016-10-03 18:51:04python-devsetmessages: + msg277989
2016-10-03 18:46:16python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg277987

resolution: fixed
stage: resolved
2016-10-03 18:23:34vinay.sajipsetassignee: vinay.sajip
messages: + msg277979
2016-10-02 15:59:33cool-RRsetmessages: + msg277899
2016-10-02 15:48:07sreyassetfiles: + raisefrom.patch
keywords: + patch
2016-10-02 13:25:45cool-RRsetmessages: + msg277890
2016-10-02 13:19:01vinay.sajipsetmessages: + msg277889
2016-10-01 20:58:49Mariattasetnosy: + Mariatta
2016-10-01 18:27:33cool-RRsetmessages: + msg277828
2016-10-01 18:20:17cool-RRcreate