From fe6ddb837d18bd4e71cd22fc18272d31478b19f2 Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Wed, 5 Aug 2015 11:07:36 +0200 Subject: [PATCH] Fixed #24704 -- Made the autoreloader survive SyntaxErrors. With this change, it's expected to survive anything except errors that make it impossible to import the settings. It's too complex to fallback to a sensible behavior with a broken settings module. Harcoding things about runserver in ManagementUtility.execute is atrocious but it's the only way out of the chicken'n'egg problem: the current implementation of the autoreloader primarily watches imported Python modules -- and then a few other things that were bolted on top of this design -- but we want it to kick in even if the project contains import-time errors and django.setup() fails. At some point we should throw away this code and replace it by an off-the-shelf autoreloader that watches the working directory and re-runs `django-admin runserver` whenever something changes. --- django/core/management/__init__.py | 17 +++++++++++++++-- django/core/management/commands/runserver.py | 4 ++++ django/utils/autoreload.py | 13 ++++++++++++- docs/releases/1.8.5.txt | 2 ++ 4 files changed, 33 insertions(+), 3 deletions(-) diff --git a/django/core/management/__init__.py b/django/core/management/__init__.py index 051747947e..842fd6481f 100644 --- a/django/core/management/__init__.py +++ b/django/core/management/__init__.py @@ -14,7 +14,7 @@ from django.core.management.base import ( BaseCommand, CommandError, CommandParser, handle_default_options, ) from django.core.management.color import color_style -from django.utils import lru_cache, six +from django.utils import autoreload, lru_cache, six from django.utils._os import npath, upath @@ -308,7 +308,20 @@ class ManagementUtility(object): settings.configure() if settings.configured: - django.setup() + # Start the auto-reloading dev server even if the code is broken. + # The hardcoded condition is a code smell but we can't rely on a + # flag on the command class because we haven't located it yet. + if subcommand == 'runserver' and '--noreload' not in self.argv: + try: + autoreload.check_errors(django.setup)() + except Exception: + # The exception will be raised later in the child process + # started by the autoreloader. + pass + + # In all other cases, django.setup() is required to succeed. + else: + django.setup() self.autocomplete() diff --git a/django/core/management/commands/runserver.py b/django/core/management/commands/runserver.py index 700da7f31d..6d05fa8d6b 100644 --- a/django/core/management/commands/runserver.py +++ b/django/core/management/commands/runserver.py @@ -104,6 +104,10 @@ class Command(BaseCommand): self.inner_run(None, **options) def inner_run(self, *args, **options): + # If an exception was silenced in ManagementUtility.execute in order + # to be raised in the child process, raise it now. + autoreload.raise_last_exception() + threading = options.get('use_threading') shutdown_message = options.get('shutdown_message', '') quit_command = 'CTRL-BREAK' if sys.platform == 'win32' else 'CONTROL-C' diff --git a/django/utils/autoreload.py b/django/utils/autoreload.py index c4b12241f0..0cf00a9646 100644 --- a/django/utils/autoreload.py +++ b/django/utils/autoreload.py @@ -37,6 +37,7 @@ import traceback from django.apps import apps from django.conf import settings from django.core.signals import request_finished +from django.utils import six from django.utils._os import npath from django.utils.six.moves import _thread as thread @@ -72,6 +73,7 @@ I18N_MODIFIED = 2 _mtimes = {} _win = (sys.platform == "win32") +_exception = None _error_files = [] _cached_modules = set() _cached_filenames = [] @@ -219,11 +221,14 @@ def code_changed(): def check_errors(fn): def wrapper(*args, **kwargs): + global _exception try: fn(*args, **kwargs) except (ImportError, IndentationError, NameError, SyntaxError, TypeError, AttributeError): - et, ev, tb = sys.exc_info() + _exception = sys.exc_info() + + et, ev, tb = _exception if getattr(ev, 'filename', None) is None: # get the filename from the last item in the stack @@ -239,6 +244,12 @@ def check_errors(fn): return wrapper +def raise_last_exception(): + global _exception + if _exception is not None: + six.reraise(*_exception) + + def ensure_echo_on(): if termios: fd = sys.stdin diff --git a/docs/releases/1.8.5.txt b/docs/releases/1.8.5.txt index 543e8824be..ac1735bb16 100644 --- a/docs/releases/1.8.5.txt +++ b/docs/releases/1.8.5.txt @@ -9,6 +9,8 @@ Django 1.8.5 fixes several bugs in 1.8.4. Bugfixes ======== +* Made the development server's autoreload more robust (:ticket:`24704`). + * Fixed ``AssertionError`` in some delete queries with a model containing a field that is both a foreign and primary key (:ticket:`24951`).