Skip to content

Friendly (traceless) error messages - #77

Open
DannyBen wants to merge 4 commits into
papertrail:masterfrom
DannyBen:arg-error
Open

Friendly (traceless) error messages#77
DannyBen wants to merge 4 commits into
papertrail:masterfrom
DannyBen:arg-error

Conversation

@DannyBen

@DannyBen DannyBen commented Aug 5, 2016

Copy link
Copy Markdown
Contributor

This PR comes to improve error messages by showing only the message, and not the trace.

It rescues two exceptions:

  • ArgumentError - raised in some cases across the code, for example - when providing an invalid argument to --min-time
  • OptionParser::InvalidOption - which is raised when parsing unexpected option
  • Closes Friendly error messages #61

After merging, you get these:

$ papertrail --min-time asd
Argument Error: Could not parse time string 'asd'

$ papertrail --asd
invalid option: --asd

@lmarburger

Copy link
Copy Markdown
Contributor

Good addition. I'd be in favor of moving the exception handling behavior out into bin/papertrail in the same way interrupts are handled by adding another rescue clause for those 2 exceptions.

@DannyBen

DannyBen commented Aug 8, 2016

Copy link
Copy Markdown
Contributor Author

Makes sense. Will update the PR later today.

@DannyBen

DannyBen commented Aug 8, 2016

Copy link
Copy Markdown
Contributor Author

@lmarburger - better?

@DannyBen

DannyBen commented Aug 9, 2016

Copy link
Copy Markdown
Contributor Author

Note that in the other bin+lib pairs, there is also some inconsistency.

For example:

  1. in the add group lib file, there are rescue statements (which should probably go to the bin, as we did here).
  2. To achieve consistent behavior of the different CLI bins, all the bins should probably rescue the same baseline of exceptions (and this code duplication will reduce code quality metrics...).

@lmarburger lmarburger removed their assignment Dec 19, 2017
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

Successfully merging this pull request may close these issues.

2 participants