Don't abort mkdocs serve for build errors during rebuilds (#3255)

mkdocs · Jun 12, 2023 · b6ff158 · b6ff158
1 parent 82aa863
commit b6ff158
Show file tree

Hide file tree

Showing 6 changed files with 62 additions and 14 deletions.
diff --git a/mkdocs/commands/build.py b/mkdocs/commands/build.py
@@ -353,7 +353,7 @@ def build(
         counts = warning_counter.get_counts()
         if counts:
             msg = ', '.join(f'{v} {k.lower()}s' for k, v in counts)
-            raise Abort(f'\nAborted with {msg} in strict mode!')
+            raise Abort(f'Aborted with {msg} in strict mode!')
 
         log.info(f'Documentation built in {time.monotonic() - start:.2f} seconds')
 
@@ -362,7 +362,7 @@ def build(
         config.plugins.run_event('build_error', error=e)
         if isinstance(e, BuildError):
             log.error(str(e))
-            raise Abort('\nAborted with a BuildError!')
+            raise Abort('Aborted with a BuildError!')
         raise
 
     finally:

diff --git a/mkdocs/commands/serve.py b/mkdocs/commands/serve.py
@@ -73,10 +73,7 @@ def builder(config: MkDocsConfig | None = None):
             config = get_config()
 
         # combine CLI watch arguments with config file values
-        if config.watch is None:
-            config.watch = watch
-        else:
-            config.watch.extend(watch)
+        config.watch.extend(watch)
 
         # Override a few config settings after validation
         config.site_url = f'http://{config.dev_addr}{mount_path(config)}'

diff --git a/mkdocs/config/base.py b/mkdocs/config/base.py
@@ -360,10 +360,10 @@ def load_config(config_file: str | IO | None = None, **kwargs) -> MkDocsConfig:
         log.debug(f"Config value '{key}' = {value!r}")
 
     if len(errors) > 0:
-        raise exceptions.Abort(f"Aborted with {len(errors)} Configuration Errors!")
-    elif cfg['strict'] and len(warnings) > 0:
+        raise exceptions.Abort(f"Aborted with {len(errors)} configuration errors!")
+    elif cfg.strict and len(warnings) > 0:
         raise exceptions.Abort(
-            f"Aborted with {len(warnings)} Configuration Warnings in 'strict' mode!"
+            f"Aborted with {len(warnings)} configuration warnings in 'strict' mode!"
         )
 
     return cfg
diff --git a/mkdocs/exceptions.py b/mkdocs/exceptions.py
@@ -8,11 +8,13 @@ class MkDocsException(ClickException):
     not be raised directly. One of the subclasses should be raised instead."""
 
 
-class Abort(MkDocsException):
+class Abort(MkDocsException, SystemExit):
     """Abort the build"""
 
+    code = 1
+
     def show(self, *args, **kwargs) -> None:
-        echo(self.format_message())
+        echo('\n' + self.format_message())
 
 
 class ConfigurationError(MkDocsException):

diff --git a/mkdocs/livereload/__init__.py b/mkdocs/livereload/__init__.py
@@ -13,8 +13,10 @@
 import socket
 import socketserver
 import string
+import sys
 import threading
 import time
+import traceback
 import urllib.parse
 import warnings
 import wsgiref.simple_server
@@ -185,8 +187,18 @@ def _build_loop(self):
                 funcs = list(self._to_rebuild)
                 self._to_rebuild.clear()
 
-            for func in funcs:
-                func()
+            try:
+                for func in funcs:
+                    func()
+            except Exception as e:
+                if isinstance(e, SystemExit):
+                    print(e, file=sys.stderr)
+                else:
+                    traceback.print_exc()
+                log.error(
+                    "An error happened during the rebuild. The server will appear stuck until build errors are resolved."
+                )
+                continue
 
             with self._epoch_cond:
                 log.info("Reloading browsers")

diff --git a/mkdocs/tests/livereload_tests.py b/mkdocs/tests/livereload_tests.py
@@ -287,6 +287,44 @@ def rebuild():
             _, output = do_request(server, "GET /foo.site")
             self.assertEqual(output, "ccccc")
 
+    @tempdir({"foo.docs": "a"})
+    @tempdir({"foo.site": "original"})
+    def test_recovers_from_build_error(self, site_dir, docs_dir):
+        started_building = threading.Event()
+        build_count = 0
+
+        def rebuild():
+            started_building.set()
+            nonlocal build_count
+            build_count += 1
+            if build_count == 1:
+                raise ValueError("oh no")
+            else:
+                content = Path(docs_dir, "foo.docs").read_text()
+                Path(site_dir, "foo.site").write_text(content * 5)
+
+        with testing_server(site_dir, rebuild) as server:
+            server.watch(docs_dir)
+            time.sleep(0.01)
+
+            err = io.StringIO()
+            with contextlib.redirect_stderr(err), self.assertLogs("mkdocs.livereload") as cm:
+                Path(docs_dir, "foo.docs").write_text("b")
+                started_building.wait(timeout=10)
+
+                Path(docs_dir, "foo.docs").write_text("c")
+
+                _, output = do_request(server, "GET /foo.site")
+
+            self.assertIn("ValueError: oh no", err.getvalue())
+            self.assertRegex(
+                "\n".join(cm.output),
+                r".*Detected file changes\n"
+                r".*An error happened during the rebuild.*\n"
+                r".*Detected file changes\n",
+            )
+            self.assertEqual(output, "ccccc")
+
     @tempdir(
         {
             "normal.html": "<html><body>hello</body></html>",
@@ -426,7 +464,6 @@ def test_error_handler(self, site_dir):
 
     @tempdir()
     def test_bad_error_handler(self, site_dir):
-        self.maxDiff = None
         with testing_server(site_dir) as server:
             server.error_handler = lambda code: 0 / 0
             with self.assertLogs("mkdocs.livereload") as cm: