Fixed #18387 -- Do not call sys.exit during call_command.

Moved sys.exit(1) so as failing management commands reach it
only when running from command line.

7 files changed, 56 insertions, 44 deletions

diff --git a/django/contrib/auth/tests/management.py b/django/contrib/auth/tests/management.py
index 5d31bd70a4..81ab0aa052 100644
--- a/django/contrib/auth/tests/management.py
+++ b/django/contrib/auth/tests/management.py
@@ -2,6 +2,7 @@ from StringIO import StringIO
 
 from django.contrib.auth import models, management
 from django.contrib.auth.management.commands import changepassword
+from django.core.management.base import CommandError
 from django.test import TestCase
 
 
@@ -56,16 +57,10 @@ class ChangepasswordManagementCommandTestCase(TestCase):
     def test_that_max_tries_exits_1(self):
         """
         A CommandError should be thrown by handle() if the user enters in
-        mismatched passwords three times. This should be caught by execute() and
-        converted to a SystemExit
+        mismatched passwords three times.
         """
         command = changepassword.Command()
         command._get_pass = lambda *args: args or 'foo'
 
-        self.assertRaises(
-            SystemExit,
-            command.execute,
-            "joe",
-            stdout=self.stdout,
-            stderr=self.stderr
-        )
+        with self.assertRaises(CommandError):
+            command.execute("joe", stdout=self.stdout, stderr=self.stderr)
diff --git a/django/core/management/base.py b/django/core/management/base.py
index 6e06991bf0..a204f6f0bc 100644
--- a/django/core/management/base.py
+++ b/django/core/management/base.py
@@ -97,8 +97,9 @@ class BaseCommand(object):
        output and, if the command is intended to produce a block of
        SQL statements, will be wrapped in ``BEGIN`` and ``COMMIT``.
 
-    4. If ``handle()`` raised a ``CommandError``, ``execute()`` will
-       instead print an error message to ``stderr``.
+    4. If ``handle()`` or ``execute()`` raised any exception (e.g.
+       ``CommandError``), ``run_from_argv()`` will  instead print an error
+       message to ``stderr``.
 
     Thus, the ``handle()`` method is typically the starting point for
     subclasses; many built-in commands and command types either place
@@ -210,23 +211,27 @@ class BaseCommand(object):
     def run_from_argv(self, argv):
         """
         Set up any environment changes requested (e.g., Python path
-        and Django settings), then run this command.
-
+        and Django settings), then run this command. If the
+        command raises a ``CommandError``, intercept it and print it sensibly
+        to stderr.
         """
         parser = self.create_parser(argv[0], argv[1])
         options, args = parser.parse_args(argv[2:])
         handle_default_options(options)
-        self.execute(*args, **options.__dict__)
+        try:
+            self.execute(*args, **options.__dict__)
+        except Exception as e:
+            if options.traceback:
+                self.stderr.write(traceback.format_exc())
+            self.stderr.write('%s: %s' % (e.__class__.__name__, e))
+            sys.exit(1)
 
     def execute(self, *args, **options):
         """
         Try to execute this command, performing model validation if
         needed (as controlled by the attribute
-        ``self.requires_model_validation``, except if force-skipped). If the
-        command raises a ``CommandError``, intercept it and print it sensibly
-        to stderr.
+        ``self.requires_model_validation``, except if force-skipped).
         """
-        show_traceback = options.get('traceback', False)
 
         # Switch to English, because django-admin.py creates database content
         # like permissions, and those shouldn't contain any translations.
@@ -237,18 +242,9 @@ class BaseCommand(object):
         self.stderr = OutputWrapper(options.get('stderr', sys.stderr), self.style.ERROR)
 
         if self.can_import_settings:
-            try:
-                from django.utils import translation
-                saved_lang = translation.get_language()
-                translation.activate('en-us')
-            except ImportError as e:
-                # If settings should be available, but aren't,
-                # raise the error and quit.
-                if show_traceback:
-                    traceback.print_exc()
-                else:
-                    self.stderr.write('Error: %s' % e)
-                sys.exit(1)
+            from django.utils import translation
+            saved_lang = translation.get_language()
+            translation.activate('en-us')
 
         try:
             if self.requires_model_validation and not options.get('skip_validation'):
@@ -265,12 +261,6 @@ class BaseCommand(object):
                 self.stdout.write(output)
                 if self.output_transaction:
                     self.stdout.write('\n' + self.style.SQL_KEYWORD("COMMIT;"))
-        except CommandError as e:
-            if show_traceback:
-                traceback.print_exc()
-            else:
-                self.stderr.write('Error: %s' % e)
-            sys.exit(1)
         finally:
             if saved_lang is not None:
                 translation.activate(saved_lang)
diff --git a/docs/howto/custom-management-commands.txt b/docs/howto/custom-management-commands.txt
index 2b34d35de7..4a27bdf7a9 100644
--- a/docs/howto/custom-management-commands.txt
+++ b/docs/howto/custom-management-commands.txt
@@ -317,8 +317,12 @@ Exception class indicating a problem while executing a management
 command.
 
 If this exception is raised during the execution of a management
-command, it will be caught and turned into a nicely-printed error
-message to the appropriate output stream (i.e., stderr); as a
-result, raising this exception (with a sensible description of the
+command from a command line console, it will be caught and turned into a
+nicely-printed error message to the appropriate output stream (i.e., stderr);
+as a result, raising this exception (with a sensible description of the
 error) is the preferred way to indicate that something has gone
 wrong in the execution of a command.
+
+If a management command is called from code through
+:ref:`call_command <call-command>`, it's up to you to catch the exception
+when needed.
diff --git a/docs/releases/1.5.txt b/docs/releases/1.5.txt
index 0fec53ab6d..0d86a52670 100644
--- a/docs/releases/1.5.txt
+++ b/docs/releases/1.5.txt
@@ -75,6 +75,10 @@ Django 1.5 also includes several smaller improvements worth noting:
 
 * The generic views support OPTIONS requests.
 
+* Management commands do not raise ``SystemExit`` any more when called by code
+  from :ref:`call_command <call-command>`. Any exception raised by the command
+  (mostly :ref:`CommandError <ref-command-exceptions>`) is propagated.
+
 * The dumpdata management command outputs one row at a time, preventing
   out-of-memory errors when dumping large datasets.
 
diff --git a/tests/modeltests/fixtures/tests.py b/tests/modeltests/fixtures/tests.py
index 48a5fe7f16..478bbe9129 100644
--- a/tests/modeltests/fixtures/tests.py
+++ b/tests/modeltests/fixtures/tests.py
@@ -185,14 +185,14 @@ class FixtureLoadingTests(TestCase):
             exclude_list=['fixtures.Article', 'fixtures.Book', 'sites'])
 
         # Excluding a bogus app should throw an error
-        self.assertRaises(SystemExit,
+        self.assertRaises(management.CommandError,
                           self._dumpdata_assert,
                           ['fixtures', 'sites'],
                           '',
                           exclude_list=['foo_app'])
 
         # Excluding a bogus model should throw an error
-        self.assertRaises(SystemExit,
+        self.assertRaises(management.CommandError,
                           self._dumpdata_assert,
                           ['fixtures', 'sites'],
                           '',
diff --git a/tests/modeltests/user_commands/management/commands/dance.py b/tests/modeltests/user_commands/management/commands/dance.py
index 4ad5579e1e..911530d223 100644
--- a/tests/modeltests/user_commands/management/commands/dance.py
+++ b/tests/modeltests/user_commands/management/commands/dance.py
@@ -1,6 +1,6 @@
 from optparse import make_option
 
-from django.core.management.base import BaseCommand
+from django.core.management.base import BaseCommand, CommandError
 
 
 class Command(BaseCommand):
@@ -8,11 +8,13 @@ class Command(BaseCommand):
     args = ''
     requires_model_validation = True
 
-    option_list =[
+    option_list = BaseCommand.option_list + (
         make_option("-s", "--style", default="Rock'n'Roll"),
         make_option("-x", "--example")
-    ]
+    )
 
     def handle(self, *args, **options):
         example = options["example"]
+        if example == "raise":
+            raise CommandError()
         self.stdout.write("I don't feel like dancing %s." % options["style"])
diff --git a/tests/modeltests/user_commands/tests.py b/tests/modeltests/user_commands/tests.py
index c1e2bf9a74..509f13f62f 100644
--- a/tests/modeltests/user_commands/tests.py
+++ b/tests/modeltests/user_commands/tests.py
@@ -1,3 +1,4 @@
+import sys
 from StringIO import StringIO
 
 from django.core import management
@@ -26,4 +27,20 @@ class CommandTests(TestCase):
             self.assertEqual(translation.get_language(), 'fr')
 
     def test_explode(self):
+        """ Test that an unknown command raises CommandError """
         self.assertRaises(CommandError, management.call_command, ('explode',))
+
+    def test_system_exit(self):
+        """ Exception raised in a command should raise CommandError with
+            call_command, but SystemExit when run from command line
+        """
+        with self.assertRaises(CommandError):
+            management.call_command('dance', example="raise")
+        old_stderr = sys.stderr
+        sys.stderr = err = StringIO()
+        try:
+            with self.assertRaises(SystemExit):
+                management.ManagementUtility(['manage.py', 'dance', '--example=raise']).execute()
+        finally:
+            sys.stderr = old_stderr
+        self.assertIn("CommandError", err.getvalue())