Suppress click stacktrace in case of invalid cli option (#2854) by cdeler · Pull Request #2915 · python-poetry/poetry

cdeler · 2020-09-14T12:13:01Z

Pull Request Check List

Resolves: #2854

Added tests for changed code (I didn't find any cli-parsing tests in CI)
Updated documentation for changed code.

After the fix, the stacktrace looks the different:

(poetry) sergei.krotov ~/workspace/skv/poetry % poetry run poetry --vers

  Stack trace:

  1  ~/.pyenv/versions/3.8.5/envs/poetry/lib/python3.8/site-packages/clikit/console_application.py:123 in run
     io = io_factory(

  PoetryException

  The "--vers" option does not exist.

  at poetry/console/config/application_config.py:234 in create_io
      230│
      231│             new_ex = PoetryException(str(ex))
      232│             new_ex.__cause__ = None
      233│             new_ex.__suppress_context__ = True
    → 234│             raise new_ex
      235│
      236│         # If the current command is the run one, skip option
      237│         # check and interpret them as part of the executed command
      238│         if resolved_command.command.name == "run":

cdeler · 2020-10-02T09:41:30Z

Hello @abn

In discord chat link you said, that I can ping you if the PR hasn't been being reviewed for a week.

Please could you take a look at the PR?

abn

Apologies for the delay. Looks good in general, couple of comments.

abn · 2020-10-23T21:33:15Z

poetry/console/config/application_config.py

+            # according to https://www.python.org/dev/peps/pep-0415/
+            # raise ... from ... overrides __cause__, so do it manually
+            # it should be replaced by raise PoetryException(str(ex)) from None
+            # as soon as python2.7 has been dropped


This can be dropped now. :)

I did it 👍

abn · 2020-10-23T21:34:15Z

tests/console/commands/test_invalid_command.py

+from poetry.exceptions import PoetryException
+
+
+def test_overwrite_exception_in_case_of_invalid_command(app):


Any chance we can add a fil ltest to validate the io output does not contain the stack trace?

I can add this check, but from my point of view it makes the test fragile.

I can check that the exception has ex.__cause__ is None. For us it means that the exception doesn't contain the original stacktrace from click

UPD well, we can check that all stacktrace belongs to poetry

abn · 2020-10-23T21:34:27Z

poetry/console/config/application_config.py

-        resolved_command = application.resolve_command(args)
+        try:
+            resolved_command = application.resolve_command(args)
+        except NoSuchOptionException as ex:


Should we expand to this to all similar exceptions?

Do you mean that I should catch bare Exception there? I didn't find any root CliKit exception, which I can catch there (but well, I can catch RuntimeError)

…y#2854)

…nal libraries)

…eptions

abn · 2022-05-07T21:38:39Z

@cdeler apologies this has not received the attention it needs. Is this something you still want to look at?

Bobronium · 2022-05-07T22:11:26Z

~~I don't think it's the way to go.~~ See Upd.

IMO, this is still way too verbose, but now it also tells much less about what's happened, so it makes it difficult to understand the nature of error in case it's needed.

I think poetry should output only error message by default, and output full stack trace if being asked with --verbose, e. g.

Upd. I dug a bit more and realised this really silence exceptions only upon resolving a command, `except Exception` made me to think it catches all of exceptions that may happen on execution.

I have a suggestion then! Will introduce it soon.

$ poetry --vers

Error: The "--vers" option does not exist.

$ poetry --vers --verbose

[NoSuchOptionException]
The "--vers" option does not exist.

Traceback (most recent call last):
  File "/home/user/.local/lib/python3.8/site-packages/clikit/console_application.py", line 123, in run
    io = io_factory(
  File "/home/user/.local/lib/python3.8/site-packages/poetry/console/config/application_config.py", line 168, in create_io
    resolved_command = application.resolve_command(args)
  File "/home/user/.local/lib/python3.8/site-packages/clikit/console_application.py", line 110, in resolve_command
    return self._config.command_resolver.resolve(args, self)
  File "/home/user/.local/lib/python3.8/site-packages/clikit/resolver/default_resolver.py", line 43, in resolve
    result = self.process_default_commands(args, application.default_commands)
  File "/home/user/.local/lib/python3.8/site-packages/clikit/resolver/default_resolver.py", line 104, in process_default_commands
    if resolved_command.is_parsable():
  File "/home/user/.local/lib/python3.8/site-packages/clikit/resolver/resolve_result.py", line 43, in is_parsable
    self._parse()
  File "/home/user/.local/lib/python3.8/site-packages/clikit/resolver/resolve_result.py", line 49, in _parse
    self._parsed_args = self._command.parse(self._raw_args)
  File "/home/user/.local/lib/python3.8/site-packages/clikit/api/command/command.py", line 113, in parse
    return self._config.args_parser.parse(args, self._args_format, lenient)
  File "/home/user/.local/lib/python3.8/site-packages/clikit/args/default_args_parser.py", line 53, in parse
    self._parse(args, _fmt, lenient)
  File "/home/user/.local/lib/python3.8/site-packages/clikit/args/default_args_parser.py", line 101, in _parse
    self._parse_long_option(token, tokens, fmt, lenient)
  File "/home/user/.local/lib/python3.8/site-packages/clikit/args/default_args_parser.py", line 247, in _parse_long_option
    self._add_long_option(name, None, tokens, fmt, lenient)
  File "/home/user/.local/lib/python3.8/site-packages/clikit/args/default_args_parser.py", line 300, in _add_long_option
    raise NoSuchOptionException(name)

github-actions · 2024-02-29T13:06:51Z

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

cdeler force-pushed the suppress-click-stacktrace branch 2 times, most recently from 80a7e72 to 049efdc Compare September 14, 2020 12:41

finswimmer requested a review from a team September 15, 2020 18:31

abn requested changes Oct 23, 2020

View reviewed changes

cdeler added 3 commits October 24, 2020 21:56

Suppress click stacktrace in case of invalid cli option (python-poetr…

c056fb6

…y#2854)

Fixed python2.7 compatibility issues

fe8fb07

Dropped python2.7 support

9933cd0

cdeler force-pushed the suppress-click-stacktrace branch from 049efdc to 9933cd0 Compare October 24, 2020 19:14

Checked that all the exceptions came from poetry (i.e. not from exter…

30bbc4b

…nal libraries)

cdeler force-pushed the suppress-click-stacktrace branch from 3184032 to 10b2112 Compare October 26, 2020 09:22

Exclude the test file from the stacktrace check

dfc5a2b

cdeler force-pushed the suppress-click-stacktrace branch from 10b2112 to dfc5a2b Compare October 26, 2020 09:31

As most CliKit exceptions inherit RuntimeError, try to catch such exc…

680b887

…eptions

cdeler requested a review from abn October 26, 2020 09:45

This was referenced May 8, 2022

Reduce default errors verbosity #5559

Closed

Decrease default errors verbosity python-poetry/cleo#132

Merged

branchv mentioned this pull request Jun 3, 2022

build: bump cleo #5766

Merged

branchv closed this in #5766 Jun 3, 2022

github-actions bot locked as resolved and limited conversation to collaborators Feb 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

Suppress click stacktrace in case of invalid cli option (#2854)#2915

Suppress click stacktrace in case of invalid cli option (#2854)#2915
cdeler wants to merge 6 commits intopython-poetry:masterfrom
cdeler:suppress-click-stacktrace

cdeler commented Sep 14, 2020 •

edited

Loading

Uh oh!

cdeler commented Oct 2, 2020

Uh oh!

abn left a comment

Uh oh!

abn Oct 23, 2020

Uh oh!

cdeler Oct 24, 2020

Uh oh!

abn Oct 23, 2020

Uh oh!

cdeler Oct 24, 2020 •

edited

Loading

Uh oh!

abn Oct 23, 2020

Uh oh!

cdeler Oct 24, 2020 •

edited

Loading

Uh oh!

abn commented May 7, 2022 •

edited

Loading

Uh oh!

Bobronium commented May 7, 2022 •

edited

Loading

Uh oh!

github-actions bot commented Feb 29, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		from poetry.exceptions import PoetryException


		def test_overwrite_exception_in_case_of_invalid_command(app):

Comments

Conversation

cdeler commented Sep 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Check List

Uh oh!

cdeler commented Oct 2, 2020

Uh oh!

abn left a comment

Choose a reason for hiding this comment

Uh oh!

abn Oct 23, 2020

Choose a reason for hiding this comment

Uh oh!

cdeler Oct 24, 2020

Choose a reason for hiding this comment

Uh oh!

abn Oct 23, 2020

Choose a reason for hiding this comment

Uh oh!

cdeler Oct 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abn Oct 23, 2020

Choose a reason for hiding this comment

Uh oh!

cdeler Oct 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abn commented May 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Bobronium commented May 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Upd. I dug a bit more and realised this really silence exceptions only upon resolving a command, except Exception made me to think it catches all of exceptions that may happen on execution.

Uh oh!

github-actions bot commented Feb 29, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cdeler commented Sep 14, 2020 •

edited

Loading

cdeler Oct 24, 2020 •

edited

Loading

cdeler Oct 24, 2020 •

edited

Loading

abn commented May 7, 2022 •

edited

Loading

Bobronium commented May 7, 2022 •

edited

Loading

Upd. I dug a bit more and realised this really silence exceptions only upon resolving a command, `except Exception` made me to think it catches all of exceptions that may happen on execution.