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

Unprintable MissingParameter exception #1139

Closed
tom-dalton-fanduel opened this issue Oct 8, 2018 · 4 comments
Closed

Unprintable MissingParameter exception #1139

tom-dalton-fanduel opened this issue Oct 8, 2018 · 4 comments
Milestone

Comments

@tom-dalton-fanduel
Copy link

@tom-dalton-fanduel tom-dalton-fanduel commented Oct 8, 2018

My code is (correctly) hitting https://github.com/pallets/click/blob/master/click/core.py#L1444 and raising a MissingParameter exception. However, there's no message passed there, which means exc.message is defaulted to None by the base class (ClickException) constructor here https://github.com/pallets/click/blob/master/click/exceptions.py#L23.

With other ClickExceptions I've seen (e.g. NoSuchOption https://github.com/pallets/click/blob/master/click/exceptions.py#L165), if message is set to None then it gets reset as something more sensible e.g. "No such option 'foo'". This means that doing str(exc) works nicely. However, MissingParameter doesn't have this, which means any attempt to stringify (e.g. print or log) a MissingParameter actually causes a TypeError: __str__ returned non-string (type NoneType) and then things go badly wrong.

Is it expected that MissingParameter is an unprintable exception when caused e.g. by missing out a required argument? It seems odd that no one else has come across this before. I'm using click in a slightly unusual context in that I'm using the group/command/arg/option parsing stuff outside of a traditional command line script, which might be why no one has seen this if click does it's own special handling for MissingParameter errors.

TL;DR It would be nice if MissingParameter was printable when raised via Parameter.full_process_value (or ideally more generally).

It might be enough to have something along the lines of what NoSuchOption does?

@taimurrabuske

This comment has been minimized.

Copy link

@taimurrabuske taimurrabuske commented Dec 20, 2018

Hello. Any progress on this?

@taimurrabuske

This comment has been minimized.

Copy link

@taimurrabuske taimurrabuske commented Dec 20, 2018

The PR above does not pass the tests. I can't investigate further at the moment.

@warsaw

This comment has been minimized.

Copy link

@warsaw warsaw commented Mar 27, 2019

This is still a problem. I just hit this myself in click 7.0.0

@davidism davidism added this to the 7.1 milestone May 6, 2019
@Ketzalkotal

This comment has been minimized.

Copy link
Contributor

@Ketzalkotal Ketzalkotal commented May 6, 2019

I'll take a look at this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked pull requests

Successfully merging a pull request may close this issue.

5 participants
You can’t perform that action at this time.