Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Graceful exit when invalid CLI flags are passed #6485

Closed
ngotchac opened this issue Sep 8, 2017 · 5 comments
Closed

Graceful exit when invalid CLI flags are passed #6485

ngotchac opened this issue Sep 8, 2017 · 5 comments
Assignees
Labels
F3-annoyance 💩 The client behaves within expectations, however this “expected behaviour” itself is at issue. M4-core ⛓ Core client code / Rust. P2-asap 🌊 No need to stop dead in your tracks, however issue should be addressed as soon as possible.
Milestone

Comments

@ngotchac
Copy link
Contributor

ngotchac commented Sep 8, 2017

Parity should panic and exit with an non-0 exit code when invalid CLI flags are passed.

@ngotchac ngotchac added F3-annoyance 💩 The client behaves within expectations, however this “expected behaviour” itself is at issue. M4-core ⛓ Core client code / Rust. labels Sep 8, 2017
@rphmeier rphmeier changed the title Panic when invalid CLI flags are passed Graceful exit when invalid CLI flags are passed Sep 8, 2017
@rphmeier
Copy link
Contributor

rphmeier commented Sep 8, 2017

panicking seems a little drastic. a helpful message and exit should be fine

@5chdn 5chdn added the P5-sometimesoon 🌲 Issue is worth doing soon. label Sep 8, 2017
@5chdn
Copy link
Contributor

5chdn commented Sep 27, 2017

I'm not sure what this ticket is about.

0 ✓ user@alcor ~ $ parity --please-exit-gracefully
Unknown flag: '--please-exit-gracefully'

Usage:
  parity [options]
  parity ui [options]
  parity dapp <path> [options]
  parity daemon <pid-file> [options]
  parity account (new | list ) [options]
  parity account import <path>... [options]
  parity wallet import <path> --password FILE [options]
  parity import [ <file> ] [options]
  parity export (blocks | state) [ <file> ] [options]
  parity signer new-token [options]
  parity signer list [options]
  parity signer sign [ <id> ] [ --password FILE ] [options]
  parity signer reject <id> [options]
  parity snapshot <file> [options]
  parity restore [ <file> ] [options]
  parity tools hash <file>
  parity db kill [options]
1 ✗ user@alcor ~ $ parity --tracing noway
Loading config file from /home/user/.local/share/io.parity.ethereum/config.toml
Invalid switch value: noway
1 ✗ user@alcor ~ $ 

It always exits with a non-0 exit code (1 in both cases above, note the 1 ✗ in my PS1). This is 1.7.2.

@ngotchac
Copy link
Contributor Author

It used to work this way, but doesn't actually on master.

@5chdn
Copy link
Contributor

5chdn commented Sep 27, 2017

True.

@5chdn 5chdn added this to the 1.9 milestone Oct 5, 2017
@5chdn 5chdn added P2-asap 🌊 No need to stop dead in your tracks, however issue should be addressed as soon as possible. and removed P5-sometimesoon 🌲 Issue is worth doing soon. labels Oct 11, 2017
@5chdn 5chdn modified the milestones: 1.9, 1.8 Oct 11, 2017
@5chdn 5chdn mentioned this issue Oct 11, 2017
67 tasks
@axelchalon
Copy link
Contributor

axelchalon commented Oct 11, 2017

So, this is caused by https://github.com/paritytech/parity/blob/master/parity/cli/usage.rs#L540 :

.global_setting(AppSettings::AllowLeadingHyphen) // allow for example --allow-ips -10.0.0.0/8

If we remove this setting then parity --please-exit-gracefully throws an error as expected. But I don't think we can afford to remove this setting.

It's strange because when we're using AllowLeadingHyphen, --please-exit-gracefully isn't matched as an arg or value anywhere but yet it doesn't throw. Might be a bug on clap's side?

[xax]$ ./target/debug/parity --please-exit-gracefully
Matches ArgMatches { args: {}, subcommand: None, usage: Some("USAGE:\n    parity [FLAGS] [OPTIONS] [SUBCOMMAND]") }
...

edit: fix on its way

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
F3-annoyance 💩 The client behaves within expectations, however this “expected behaviour” itself is at issue. M4-core ⛓ Core client code / Rust. P2-asap 🌊 No need to stop dead in your tracks, however issue should be addressed as soon as possible.
Projects
None yet
Development

No branches or pull requests

4 participants