Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Making TomlDecodeError more useful #35

Open
ksamuel opened this issue Jan 2, 2018 · 1 comment
Open

Making TomlDecodeError more useful #35

ksamuel opened this issue Jan 2, 2018 · 1 comment

Comments

@ksamuel
Copy link

ksamuel commented Jan 2, 2018

I have some code parsing TOML doing:

    except toml.TomlDecodeError as e:
        msg = "'{}' is not a valid TOML file (Error given is: {})\n"
        error  = e.args[0]
        if error == 'Invalid date or number':
            msg += (
                "One frequent cause of this is forgetting to put quotes "
                "around secret keys. Check the file."
            )

        match = re.search(r"What\? (\w+) ", error)
        duplicate = match and next(iter(match.groups()), None)
        if duplicate:
            msg += (
                "One frequent cause of this is using the same account name "
                "twice. Check that you didn't use '{}' several times."
            ).format(duplicate)

        ctx.fail(msg.format(secrets_file, e))

All that is because I get bare TomlDecodeError exceptions.

When raising an exception, we could attach some data so that catching code can make more accurate decision.

E.G:

TomlDecodeError with Invalid date or number and What? Foo already exists should have a field attribute so that we can inspect what data causes the problem.

Also, it's good to have TomlDecodeError subclasses for each particular problem so that we can have a more granular control over what we catch.

E.G:

class InvalidDateOrNumberTomlError(TomlDecodeError, ValueError):
    pass

class DuplicateKeyTomlError(TomlDecodeError, KeyError):
    pass

In the end, my ugly hack code could be replaced by:

    except InvalidDateOrNumberTomlError as e:
        ctx.fail((
            'Duplicate key: "{}" '
            'One frequent cause of this is using the same account name twice.'
        )).format(e.field))
    except DuplicateKeyTomlError as e:
        ctx.fail((
            'Invalid date or number on field "{}" '
            'One frequent cause of this is forgetting to put quotes '
            'around secret keys. Check the file.'
        )).format(e.field))

Which is shorter, clearer, less error prone, and less likely to break in the future.

It's fair to expect that programmers will want to give feedback to their user if parsing a config file fail, so let's make that easy.

@avakar
Copy link
Owner

avakar commented Jan 2, 2018

Thank you for the issue!

I agree with you, however time constraints don't allow me to work on this. Pull requests are welcome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants