Exceptions caught incorrectly in get_context_from_config

[bignose-debian]

The exception handling in get_context_from_config makes incorrect assumptions, and catches exceptions that it does not handle correctly.

In get_context_from_config, the statements

parser = get_config()
return parser.getint("ipdb", "context")

are wrapped in a try block. But the except clauses evidently anticipate exceptions from the second statement only: they expect that the parser object exists and it was the parser.getint call that raised an exception.

This assumption is incorrect.

When the call to get_config raises a ValueError, it is erroneously caught as though raised from an already existing parser. Then the except clause attempts to use that object and it fails during exception handling.

File "[…]/ipdb/main.py", line 89, in get_context_from_config
value = parser.get("ipdb", "context")
UnboundLocalError: local variable 'parser' referenced before assignment

[bignose-debian]

An easy way to have the get_config call fail, is to have a badly-formatted configuration file that get_config will read (such as pyproject.toml). If get_config raises a ValueError it will be erroneously caught in the get_context_from_config logic as described above.

[bignose-debian]

I have implemented a test case which demonstrates this, and a simple change to correct it. See the branch refine-exception-handling in my personal fork.

The test case (after refactoring to allow adding this test case) is:

    def test_noenv_nodef_invalid_setup(self):
        """
        Setup: $IPDB_CONFIG unset, $HOME/.ipdb does not exist,
            setup.cfg does not exist, pyproject.toml content is invalid.
        Result: Propagate exception from `get_config`.
        """
        os.unlink(self.env_filename)
        os.unlink(self.default_filename)
        os.unlink(self.setup_filename)
        write_lines_to_file(
            self.pyproject_filename,
            [
                "[ipdb]",
                "spam = abc",
            ],
        )
        with ModifiedEnvironment(IPDB_CONFIG=None, HOME=self.tmpd):
            try:
                __ = get_context_from_config()
            except TomlDecodeError:
                pass
            else:
                self.fail("Expected TomlDecodeError from invalid config file")

The correction is as simple as moving a line out of the exception handling so that its exceptions propagate normally:

modified   ipdb/__main__.py
@@ -80,8 +80,8 @@ def set_trace(frame=None, context=None, cond=True):
 
 
 def get_context_from_config():
+    parser = get_config()
     try:
-        parser = get_config()
         return parser.getint("ipdb", "context")
     except (configparser.NoSectionError, configparser.NoOptionError):
         return 3

[gotcha]

Thanks for the analysis and the test !

Any reason this is not a PR ?

[bignose-debian]

On 09-Sep-2021, Godefroid Chapelle wrote: Thanks for the analysis and the test !

You're welcome, I hope it helps.

Any reason this is not a PR ?

While Git is entirely distributed and allows us to collaborate, the GitHub proprietary PR system doesn't work for repositories hosted outside GitHub.

…

-- \ “The Things to do are: the things that need doing, that you see | `\ need to be done, and that no one else seems to see need to be | _o__) done.” —Richard Buckminster Fuller, 1970-02-16 | Ben Finney ***@***.***>

[gotcha]

@bignose-debian I am fine adding your repo as remote to merge your changes myself.
Can you point me to the proper https git url ?

[bignose-debian]

@bignose-debian I am fine adding your repo as remote to merge your changes myself.
Can you point me to the proper https git url ?

Yes, see the summary page for that repository.

[gotcha]

Your work has been included in master. See 333605e.

Thanks a lot !

[gotcha]

Released in https://pypi.org/project/ipdb/0.13.11/