From 06d1a59513f8d4eec35c982028a9dc6cff779e5b Mon Sep 17 00:00:00 2001 From: Arun Persaud Date: Sun, 2 Mar 2025 08:10:49 -0800 Subject: [PATCH 1/8] nikola serve: Better error handling for already used port. Fixes #3824 --- nikola/plugins/command/serve.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/nikola/plugins/command/serve.py b/nikola/plugins/command/serve.py index 0b2cbac564..86067b3dad 100644 --- a/nikola/plugins/command/serve.py +++ b/nikola/plugins/command/serve.py @@ -26,6 +26,7 @@ """Start test server.""" import atexit +import errno import os import sys import re @@ -144,7 +145,16 @@ def _execute(self, options, args): handler_factory = OurHTTPRequestHandler else: handler_factory = _create_RequestHandler_removing_basepath(base_path) - self.httpd = OurHTTP((options['address'], options['port']), handler_factory) + + try: + self.httpd = OurHTTP((options['address'], options['port']), handler_factory) + except OSError as e: + if e.errno == errno.EADDRINUSE: + print(f"Port address {options['port']} already in use, please use " + f"the `-p ` option to select a different one.") + sys.exit(1) + else: + raise sa = self.httpd.socket.getsockname() if ipv6: From b27f85178ed892ea1cb5a6a7a424983c8cb2d834 Mon Sep 17 00:00:00 2001 From: Arun Persaud Date: Sun, 2 Mar 2025 08:12:44 -0800 Subject: [PATCH 2/8] Added Changelog entry --- CHANGES.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGES.txt b/CHANGES.txt index 6646630ec4..ea4795865a 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -43,6 +43,7 @@ Features via an optional ``conf.py`` method ``TEMPLATE_ENGINE_FACTORY``. * Switch to pyproject.toml * Add path handler ``slug_source`` linking to source of post. +* Better error message in ``nikola serve`` when port is already in use Bugfixes -------- From 69fd34df4304395df8dfa99a303b3300eef816a7 Mon Sep 17 00:00:00 2001 From: Arun Persaud Date: Sun, 2 Mar 2025 20:28:09 -0800 Subject: [PATCH 3/8] switch from print to logger --- nikola/plugins/command/serve.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nikola/plugins/command/serve.py b/nikola/plugins/command/serve.py index 86067b3dad..9bf5cd4b6e 100644 --- a/nikola/plugins/command/serve.py +++ b/nikola/plugins/command/serve.py @@ -150,8 +150,8 @@ def _execute(self, options, args): self.httpd = OurHTTP((options['address'], options['port']), handler_factory) except OSError as e: if e.errno == errno.EADDRINUSE: - print(f"Port address {options['port']} already in use, please use " - f"the `-p ` option to select a different one.") + self.logger.error(f"Port address {options['port']} already in use, please use " + "the `-p ` option to select a different one.") sys.exit(1) else: raise From 5bc8120db5faf90d0bd3004d2708015fe52eda3e Mon Sep 17 00:00:00 2001 From: Arun Persaud Date: Sun, 2 Mar 2025 20:48:10 -0800 Subject: [PATCH 4/8] add integration test --- tests/integration/test_dev_server_serve.py | 65 ++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/tests/integration/test_dev_server_serve.py b/tests/integration/test_dev_server_serve.py index a91362b641..d6614ea906 100644 --- a/tests/integration/test_dev_server_serve.py +++ b/tests/integration/test_dev_server_serve.py @@ -7,6 +7,71 @@ from nikola.utils import base_path_from_siteuri from .dev_server_test_helper import MyFakeSite, SERVER_ADDRESS, find_unused_port, LOGGER, OUTPUT_FOLDER +def test_two_serves_with_different_port( site_and_base_path: Tuple[MyFakeSite, str], expected_text: str +): + site, base_path = site_and_base_path + command_serveA = serve.CommandServe() + command_serveA.set_site(site) + command_serveB = serve.CommandServe() + command_serveB.set_site(site) + with ThreadPoolExecutor(max_workers=2) as executor: + options = { + "address": SERVER_ADDRESS, + "port": find_unused_port(), + "browser": False, + "detach": False, + "ipv6": False, + } + future_to_run_web_serverA = executor.submit(lambda: command_serveA.execute(options=options)) + options = { + "address": SERVER_ADDRESS, + "port": find_unused_port(), + "browser": False, + "detach": False, + "ipv6": False, + } + future_to_run_web_serverB = executor.submit(lambda: command_serveB.execute(options=options)) + sleep(0.1) + try: + command_serveA.shutdown() + future_to_run_web_serverA.result() + except SystemExit as e: + assert e.code == 0 + try: + command_serveB.shutdown() + future_to_run_web_serverB.result() + except SystemExit as e: + assert e.code == 0 + +def test_two_serves_with_same_port( site_and_base_path: Tuple[MyFakeSite, str], expected_text: str +): + site, base_path = site_and_base_path + command_serveA = serve.CommandServe() + command_serveA.set_site(site) + command_serveB = serve.CommandServe() + command_serveB.set_site(site) + port = find_unused_port() + with ThreadPoolExecutor(max_workers=2) as executor: + options = { + "address": SERVER_ADDRESS, + "port": port, + "browser": False, + "detach": False, + "ipv6": False, + } + future_to_run_web_serverA = executor.submit(lambda: command_serveA.execute(options=options)) + future_to_run_web_serverB = executor.submit(lambda: command_serveB.execute(options=options)) + sleep(0.1) + try: + command_serveA.shutdown() + future_to_run_web_serverA.result() + except SystemExit as e: + assert e.code == 0 + try: + command_serveB.shutdown() + future_to_run_web_serverB.result() + except SystemExit as e: + assert e.code == 1 def test_serves_root_dir( site_and_base_path: tuple[MyFakeSite, str], expected_text: str From c67e0677603d6220a185e37c9ca39787a54a9c3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20Kr=C3=BCger?= Date: Tue, 4 Mar 2025 22:35:55 +0100 Subject: [PATCH 5/8] Test for the improved "nikola serve" error handling. --- nikola/plugins/command/serve.py | 4 +- tests/integration/test_dev_server_serve.py | 104 +++++++++------------ 2 files changed, 44 insertions(+), 64 deletions(-) diff --git a/nikola/plugins/command/serve.py b/nikola/plugins/command/serve.py index 9bf5cd4b6e..65676cef58 100644 --- a/nikola/plugins/command/serve.py +++ b/nikola/plugins/command/serve.py @@ -151,8 +151,8 @@ def _execute(self, options, args): except OSError as e: if e.errno == errno.EADDRINUSE: self.logger.error(f"Port address {options['port']} already in use, please use " - "the `-p ` option to select a different one.") - sys.exit(1) + "the `-p ` option to select a different one.") + return 3 else: raise diff --git a/tests/integration/test_dev_server_serve.py b/tests/integration/test_dev_server_serve.py index d6614ea906..38e1fa6fba 100644 --- a/tests/integration/test_dev_server_serve.py +++ b/tests/integration/test_dev_server_serve.py @@ -1,3 +1,7 @@ +import logging +import re +import socket +from io import StringIO from time import sleep import requests import pytest @@ -7,77 +11,53 @@ from nikola.utils import base_path_from_siteuri from .dev_server_test_helper import MyFakeSite, SERVER_ADDRESS, find_unused_port, LOGGER, OUTPUT_FOLDER -def test_two_serves_with_different_port( site_and_base_path: Tuple[MyFakeSite, str], expected_text: str -): - site, base_path = site_and_base_path - command_serveA = serve.CommandServe() - command_serveA.set_site(site) - command_serveB = serve.CommandServe() - command_serveB.set_site(site) - with ThreadPoolExecutor(max_workers=2) as executor: - options = { - "address": SERVER_ADDRESS, - "port": find_unused_port(), - "browser": False, - "detach": False, - "ipv6": False, - } - future_to_run_web_serverA = executor.submit(lambda: command_serveA.execute(options=options)) - options = { - "address": SERVER_ADDRESS, - "port": find_unused_port(), - "browser": False, - "detach": False, - "ipv6": False, - } - future_to_run_web_serverB = executor.submit(lambda: command_serveB.execute(options=options)) - sleep(0.1) - try: - command_serveA.shutdown() - future_to_run_web_serverA.result() - except SystemExit as e: - assert e.code == 0 - try: - command_serveB.shutdown() - future_to_run_web_serverB.result() - except SystemExit as e: - assert e.code == 0 -def test_two_serves_with_same_port( site_and_base_path: Tuple[MyFakeSite, str], expected_text: str -): +def test_server_on_used_port(site_and_base_path: Tuple[MyFakeSite, str]): site, base_path = site_and_base_path - command_serveA = serve.CommandServe() - command_serveA.set_site(site) - command_serveB = serve.CommandServe() - command_serveB.set_site(site) - port = find_unused_port() - with ThreadPoolExecutor(max_workers=2) as executor: - options = { - "address": SERVER_ADDRESS, - "port": port, - "browser": False, - "detach": False, - "ipv6": False, - } - future_to_run_web_serverA = executor.submit(lambda: command_serveA.execute(options=options)) - future_to_run_web_serverB = executor.submit(lambda: command_serveB.execute(options=options)) - sleep(0.1) - try: - command_serveA.shutdown() - future_to_run_web_serverA.result() - except SystemExit as e: - assert e.code == 0 + command_serve = serve.CommandServe() + command_serve.set_site(site) + command_serve.serve_pidfile = "there is no file with this name we hope" + command_serve.logger = logging.getLogger("dev_server_test") + catch_log = StringIO() + catch_log_handler = logging.StreamHandler(catch_log) + logging.getLogger().addHandler(catch_log_handler) + try: + s = socket.socket() try: - command_serveB.shutdown() - future_to_run_web_serverB.result() - except SystemExit as e: - assert e.code == 1 + ANY_PORT = 0 + s.bind((SERVER_ADDRESS, ANY_PORT)) + address, port = s.getsockname() + with ThreadPoolExecutor(max_workers=2) as executor: + options = { + "address": SERVER_ADDRESS, + "port": port, + "browser": False, + "detach": False, + "ipv6": False, + } + future_to_run_web_server = executor.submit(lambda: command_serve.execute(options=options)) + command_serve.shutdown() + result = future_to_run_web_server.result() + assert 3 == result + assert re.match( + r"Port address \d+ already in use, " + r"please use the `\-p \` option to select a different one\.", + catch_log.getvalue() + ) + assert "OSError" not in catch_log.getvalue() + finally: + s.close() + finally: + logging.getLogger().removeHandler(catch_log_handler) + def test_serves_root_dir( site_and_base_path: tuple[MyFakeSite, str], expected_text: str ) -> None: site, base_path = site_and_base_path command_serve = serve.CommandServe() + command_serve.serve_pidfile = "there is no file with this name we hope" + command_serve.logger = logging.getLogger("dev_server_test") command_serve.set_site(site) options = { "address": SERVER_ADDRESS, From 1e3e9dc2bfc7369281a75945aa09eed90b0c14da Mon Sep 17 00:00:00 2001 From: Arun Persaud Date: Thu, 6 Mar 2025 07:03:45 -0800 Subject: [PATCH 6/8] small fixes for Windows (not tested) --- tests/helper.py | 1 + tests/integration/test_dev_server_serve.py | 14 +++++++++----- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/tests/helper.py b/tests/helper.py index 8fbebdb45b..2bc995d0b3 100644 --- a/tests/helper.py +++ b/tests/helper.py @@ -31,6 +31,7 @@ def __init__(self): self.template_system = self self.invariant = False self.debug = True + self.show_tracebacks = True self.config = { "DISABLED_PLUGINS": [], "EXTRA_PLUGINS": [], diff --git a/tests/integration/test_dev_server_serve.py b/tests/integration/test_dev_server_serve.py index 38e1fa6fba..7ec061a963 100644 --- a/tests/integration/test_dev_server_serve.py +++ b/tests/integration/test_dev_server_serve.py @@ -1,6 +1,7 @@ import logging import re import socket +import sys from io import StringIO from time import sleep import requests @@ -39,11 +40,14 @@ def test_server_on_used_port(site_and_base_path: Tuple[MyFakeSite, str]): command_serve.shutdown() result = future_to_run_web_server.result() assert 3 == result - assert re.match( - r"Port address \d+ already in use, " - r"please use the `\-p \` option to select a different one\.", - catch_log.getvalue() - ) + + + if not sys.platform == 'win32': + assert re.match( + r"Port address \d+ already in use, " + r"please use the `\-p \` option to select a different one\.", + catch_log.getvalue() + ) assert "OSError" not in catch_log.getvalue() finally: s.close() From 240f34bff1e58509ae7b0f204b99fe17dde1be50 Mon Sep 17 00:00:00 2001 From: Arun Persaud Date: Thu, 6 Mar 2025 19:41:35 -0800 Subject: [PATCH 7/8] tests: added a doc string and a comment on why we skip the windows platform for an assert at the moment --- tests/integration/test_dev_server_serve.py | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/tests/integration/test_dev_server_serve.py b/tests/integration/test_dev_server_serve.py index 7ec061a963..5959b2551b 100644 --- a/tests/integration/test_dev_server_serve.py +++ b/tests/integration/test_dev_server_serve.py @@ -13,8 +13,20 @@ from .dev_server_test_helper import MyFakeSite, SERVER_ADDRESS, find_unused_port, LOGGER, OUTPUT_FOLDER -def test_server_on_used_port(site_and_base_path: Tuple[MyFakeSite, str]): +def test_server_on_used_port(site_and_base_path: Tuple[MyFakeSite, str]) -> None: + """Check error if port for nikola serve is already being used. + + `nikola serve` uses a default port and if that port is already in use it should print out a nice + error message that tells the user what happend and how to fix this. + + To test the case where the port is already in use, we open a socket on the same port that we use + for `nikola serve` before starting the server. + + The program should exit with a return code of 3 in this case and print out a message to the user. + """ + site, base_path = site_and_base_path + site.show_tracebacks = False command_serve = serve.CommandServe() command_serve.set_site(site) command_serve.serve_pidfile = "there is no file with this name we hope" @@ -41,7 +53,8 @@ def test_server_on_used_port(site_and_base_path: Tuple[MyFakeSite, str]): result = future_to_run_web_server.result() assert 3 == result - + # TODO: check if this works on windows + # for now we skip this assert on windows platforms. if not sys.platform == 'win32': assert re.match( r"Port address \d+ already in use, " From f744367eee6676babbd8cc456e968ddd02c85164 Mon Sep 17 00:00:00 2001 From: Arun Persaud Date: Wed, 1 Apr 2026 12:17:15 -0700 Subject: [PATCH 8/8] fix type definition from Tuple to tuple --- tests/integration/test_dev_server_serve.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/test_dev_server_serve.py b/tests/integration/test_dev_server_serve.py index 5959b2551b..085e11e7f0 100644 --- a/tests/integration/test_dev_server_serve.py +++ b/tests/integration/test_dev_server_serve.py @@ -13,7 +13,7 @@ from .dev_server_test_helper import MyFakeSite, SERVER_ADDRESS, find_unused_port, LOGGER, OUTPUT_FOLDER -def test_server_on_used_port(site_and_base_path: Tuple[MyFakeSite, str]) -> None: +def test_server_on_used_port(site_and_base_path: tuple[MyFakeSite, str]) -> None: """Check error if port for nikola serve is already being used. `nikola serve` uses a default port and if that port is already in use it should print out a nice