Skip to content

Commit

Permalink
Simplify Exit Exception
Browse files Browse the repository at this point in the history
= Refactor BaseCommand to simplify Exit handling

BaseCommand.main() will get the return value from `self.invoke` and
pass it to `ctx.exit()`, so that commands which `return 1` will
`ctx.exit(1)`
This is not a behavioral change -- `ctx.exit()` already does this.

Do not reraise Exit exceptions when invoked non-standalone. Instead,
return their exit codes as the result of `BaseCommand.main`. So, a
command which explicitly invokes `ctx.exit(0)` in its execution will
effectively `return 0` instead of raising a specialized exception.

= Make Exit its own RuntimeError subtype, don't pollute stdio on exit

Exit exceptions should not be a subtype of ClickException with the
output pretty-printing (`show()`) which happens when they are raised in
non-standalone mode. That would make `ctx.exit(...)` needlessly pollute
stderr. This would have downstream impact on everyone using context exit
calls, and generally be a Bad Thing (tm).

Instead, like Abort, Exit is a subclass of RuntimeError with very little
"meat on its bones". It's an exception containing a single integer (exit
code)  which is then interpreted in that way in BaseCommand.main

closes pallets#533
closes pallets#667
  • Loading branch information
sirosen committed Aug 26, 2018
1 parent 225736b commit 1602436
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 53 deletions.
2 changes: 2 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ Version 7.0

(upcoming release with new features, release date to be decided)

- Non-standalone calls to Context.exit return the exit code, rather than
calling ``sys.exit`` (`#667`_)(`#533`_)
- Updated test env matrix. (`#1027`_)
- Fixes a ZeroDivisionError in ProgressBar.make_step,
when the arg passed to the first call of ProgressBar.update is 0. (`#1012`_)(`#447`_)
Expand Down
17 changes: 9 additions & 8 deletions click/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,7 @@ def abort(self):

def exit(self, code=0):
"""Exits the application with a given exit code."""
raise Exit(code, self)
raise Exit(code)

def get_usage(self):
"""Helper method to get formatted usage string for the current
Expand Down Expand Up @@ -714,16 +714,12 @@ def main(self, args=None, prog_name=None, complete_var=None,
rv = self.invoke(ctx)
if not standalone_mode:
return rv
ctx.exit()
if not rv:
rv = 0
ctx.exit(rv)
except (EOFError, KeyboardInterrupt):
echo(file=sys.stderr)
raise Abort()
except Exit as e:
if not standalone_mode:
if e.exit_code:
raise
return None
sys.exit(e.exit_code)
except ClickException as e:
if not standalone_mode:
raise
Expand All @@ -734,6 +730,11 @@ def main(self, args=None, prog_name=None, complete_var=None,
sys.exit(1)
else:
raise
except Exit as e:
if standalone_mode:
sys.exit(e.exit_code)
else:
return e.exit_code
except Abort:
if not standalone_mode:
raise
Expand Down
49 changes: 17 additions & 32 deletions click/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ class ClickException(Exception):

#: The exit code for this exception
exit_code = 1
show_prefix = 'Error: '

def __init__(self, message):
ctor_msg = message
Expand All @@ -38,17 +37,19 @@ def __str__(self):
def show(self, file=None):
if file is None:
file = get_text_stderr()
echo('%s%s' % (self.show_prefix, self.format_message()), file=file)
echo('Error: %s' % self.format_message(), file=file)


class ContextAwareException(ClickException):
"""An exception that knows how to use a :class:`~click.Context` object
to properly display itself.
class UsageError(ClickException):
"""An internal exception that signals a usage error. This typically
aborts any further handling.
:param message: the error message to display.
:param ctx: optionally the context that caused this error. Click will
fill in the context automatically in some situations.
"""
exit_code = 2

def __init__(self, message, ctx=None):
ClickException.__init__(self, message)
self.ctx = ctx
Expand All @@ -66,33 +67,7 @@ def show(self, file=None):
if self.ctx is not None:
color = self.ctx.color
echo(self.ctx.get_usage() + '\n%s' % hint, file=file, color=color)
echo('%s%s' % (self.show_prefix, self.format_message()), file=file, color=color)


class Exit(ContextAwareException):
"""An exception that indicates that the application should exit with some
status code.
:param code: the status code to exit with.
:param ctx: optionally the context that caused this error. Click will
fill in the context automatically in some situations.
"""
show_prefix = 'Exit: '

def __init__(self, code, ctx=None):
ContextAwareException.__init__(self, str(code), ctx=None)
self.exit_code = code


class UsageError(ContextAwareException):
"""An internal exception that signals a usage error. This typically
aborts any further handling.
:param message: the error message to display.
:param ctx: optionally the context that caused this error. Click will
fill in the context automatically in some situations.
"""
exit_code = 2
echo('Error: %s' % self.format_message(), file=file, color=color)


class BadParameter(UsageError):
Expand Down Expand Up @@ -248,3 +223,13 @@ def format_message(self):

class Abort(RuntimeError):
"""An internal signalling exception that signals Click to abort."""


class Exit(RuntimeError):
"""An exception that indicates that the application should exit with some
status code.
:param code: the status code to exit with.
"""
def __init__(self, code):
self.exit_code = code
17 changes: 4 additions & 13 deletions tests/test_context.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
# -*- coding: utf-8 -*-
import pytest

import click


Expand Down Expand Up @@ -195,6 +193,7 @@ def test_close_before_pop(runner):
@click.pass_context
def cli(ctx):
ctx.obj = 'test'

@ctx.call_on_close
def foo():
assert click.get_current_context().obj == 'test'
Expand Down Expand Up @@ -243,25 +242,17 @@ def test2(ctx, foo):
assert result.output == 'foocmd\n'


def test_exit_not_standalone_failure():
def test_exit_not_standalone():
@click.command()
@click.pass_context
def cli(ctx):
ctx.exit(1)

with pytest.raises(click.exceptions.Exit) as e:
cli.main([], 'test_exit_not_standalone', standalone_mode=False)
assert e.value.exit_code == 1

assert cli.main([], 'test_exit_not_standalone', standalone_mode=False) == 1

def test_exit_not_standalone_success():
@click.command()
@click.pass_context
def cli(ctx):
ctx.exit(0)

assert cli.main(
[],
'test_exit_not_standalone',
standalone_mode=False,
) is None
assert cli.main([], 'test_exit_not_standalone', standalone_mode=False) == 0

0 comments on commit 1602436

Please sign in to comment.