Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify error about taken port in serve; allow to open a browser #3498

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 10 additions & 3 deletions docs/user-guide/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -644,15 +644,22 @@ This is also available as a command line flag: `--strict`.

**default**: `false`

### serve_port

Determines the port used when running `mkdocs serve`.

This lets you permanently change the port instead of passing it through the `--port`/`-p` option every time the `mkdocs serve` command is called.

**default**: `8000`

### dev_addr

Determines the address used when running `mkdocs serve`. Must be of the format
`IP:PORT`.
Determines the address used when running `mkdocs serve`. Can be just the IP address, or `IP:PORT` (thus covering also [`serve_port`](#serve_port)).

Allows a custom default to be set without the need to pass it through the
`--dev-addr` option every time the `mkdocs serve` command is called.

**default**: `'127.0.0.1:8000'`
**default**: `'127.0.0.1'`

See also: [site_url](#site_url).

Expand Down
8 changes: 6 additions & 2 deletions mkdocs/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,10 @@ def __del__(self):
config_help = (
"Provide a specific MkDocs config. This can be a file name, or '-' to read from stdin."
)
dev_addr_help = "IP address and port to serve documentation locally (default: localhost:8000)"
dev_addr_help = "IP address to serve documentation locally (default: localhost)."
serve_port_help = (
"Port to serve documentation locally (default: 8000). Overrides the port from dev_addr."
)
serve_open_help = "Open the website in a Web browser after the initial build finishes."
strict_help = "Enable strict mode. This will cause MkDocs to abort the build on any warnings."
theme_help = "The theme to use when building your documentation."
Expand Down Expand Up @@ -249,8 +252,9 @@ def cli():


@cli.command(name="serve")
@click.option('-a', '--dev-addr', help=dev_addr_help, metavar='<IP:PORT>')
@click.option('-a', '--dev-addr', help=dev_addr_help, metavar='IP:PORT')
Copy link
Sponsor Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, please don't get me wrong, but IMHO, this is not a very consistent design. You can specify a port via --dev-addr, but also via --port? That's two ways of achieving the same thing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK what do you suggest?

Copy link
Sponsor Contributor

@squidfunk squidfunk Dec 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either remove --port or deprecate and split up --dev-addr into --host and --port. I'm not sure what you're aiming here for however, because --dev-addr is working perfectly fine.

Copy link
Sponsor Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also note that all of this might incentivize people to use the development server as a production server, because now you can only set the --port. --dev-addr carries dev in it, which I think was put there deliberately to keep people from using it in production. Still happens more often than one might think, as judging from issues I answered in the past.

Copy link
Sponsor Contributor

@pawamoy pawamoy Dec 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would find a -p option really convenient. I literally never change the default host, and if I had to, it would probably be in a container setup where I would set it to 0.0.0.0 and never change it again. Whereas in (rare) cases where I serve multiple sites, I need to change the port, and having to re-specify the default localhost is a bit annoying.

So I wouldn't mind if --dev-addr was deprecated in favor of two --host and --port options. However I'm not the only MkDocs user and I fear it would be quite disruptive to deprecate --dev-addr for, again, rarely happening situations. Maybe they could both (dev-addr and host+port) live together, forever, as mutually exclusive options? Or maybe --dev-addr could be modified so that we can pass either HOST:PORT, or just HOST, or just PORT? It looks like it's easy to distinguish the three cases?

@click.option('-o', '--open', 'open_in_browser', help=serve_open_help, is_flag=True)
@click.option('-p', '--port', type=int, help=serve_port_help)
@click.option('--no-livereload', 'livereload', flag_value=False, help=no_reload_help)
@click.option('--livereload', 'livereload', flag_value=True, default=True, hidden=True)
@click.option('--dirtyreload', 'build_type', flag_value='dirty', hidden=True)
Expand Down
78 changes: 75 additions & 3 deletions mkdocs/commands/serve.py
Original file line number Diff line number Diff line change
@@ -1,21 +1,29 @@
from __future__ import annotations

import hashlib
import json
import logging
import os.path
import shutil
import tempfile
import urllib.request
from os.path import isdir, isfile, join
from typing import TYPE_CHECKING
from typing import TYPE_CHECKING, Any, Mapping
from urllib.error import HTTPError
from urllib.parse import urlsplit

from mkdocs.commands.build import build
from mkdocs.config import load_config
from mkdocs.livereload import LiveReloadServer, _serve_url
from mkdocs.exceptions import Abort
from mkdocs.livereload import LiveReloadServer, ServerBindError, _serve_url

if TYPE_CHECKING:
from mkdocs.config.defaults import MkDocsConfig

log = logging.getLogger(__name__)

DEFAULT_PORT = 8000


def serve(
config_file: str | None = None,
Expand All @@ -24,6 +32,7 @@ def serve(
watch_theme: bool = False,
watch: list[str] = [],
*,
port: int | None = None,
open_in_browser: bool = False,
**kwargs,
) -> None:
Expand Down Expand Up @@ -55,6 +64,15 @@ def get_config():
config.plugins.on_startup(command=('build' if is_clean else 'serve'), dirty=is_dirty)

host, port = config.dev_addr
if port is None:
port = DEFAULT_PORT

origin_info = dict(
path=os.path.dirname(config.config_file_path) if config.config_file_path else os.getcwd(),
site_name=config.site_name,
site_url=config.site_url,
)

mount_path = urlsplit(config.site_url or '/').path
config.site_url = serve_url = _serve_url(host, port, mount_path)

Expand All @@ -67,7 +85,12 @@ def builder(config: MkDocsConfig | None = None):
build(config, serve_url=None if is_clean else serve_url, dirty=is_dirty)

server = LiveReloadServer(
builder=builder, host=host, port=port, root=site_dir, mount_path=mount_path
builder=builder,
host=host,
port=port,
root=site_dir,
mount_path=mount_path,
origin_info=origin_info,
)

def error_handler(code) -> bytes | None:
Expand Down Expand Up @@ -102,6 +125,11 @@ def error_handler(code) -> bytes | None:

try:
server.serve(open_in_browser=open_in_browser)
except ServerBindError as e:
log.error(f"Could not start a server on port {port}: {e}")
msg = diagnose_taken_port(port, url=server.url, origin_info=origin_info)
raise Abort(msg)

except KeyboardInterrupt:
log.info("Shutting down...")
finally:
Expand All @@ -110,3 +138,47 @@ def error_handler(code) -> bytes | None:
config.plugins.on_shutdown()
if isdir(site_dir):
shutil.rmtree(site_dir)


def diagnose_taken_port(port: int, *, url: str, origin_info: Mapping[str, Any]) -> str:
origin_info = dict(origin_info)
path: str = origin_info.pop('path')

message = f"Attempted to listen on port {port} but "
other_info = None
try:
with urllib.request.urlopen(f'http://127.0.0.1:{port}/livereload/.info.json') as resp:
if resp.status == 200:
other_info = json.load(resp)
except HTTPError as e:
message += "some unrecognized HTTP server is already running on that port."
server = e.headers.get('server')
if server:
message += f" ({server!r})"
except ValueError:
message += "some unrecognized HTTP server is already running on that port."
except Exception:
message += "failed. And there isn't an HTTP server running on that port, but maybe another process is occupying it anyway."

if other_info:
message += "a live-reload server is already running on that port."
if other_info['origin_info'].get('path') == path and other_info.get('url') == url:
message += f"\nIt actually serves the same path '{path}', try simply visiting {url}"
else:
message += f" It serves a different path '{path}'."

if "the same path" not in message:
new_port = get_random_port(origin_info)
message += (
f"\n\nTry serving on another port by passing the flag `-p {new_port}` (as an example)."
)
if port == DEFAULT_PORT:
message += f" Or permanently use a distinct port for this site by adding `serve_port: {new_port}` to its config."

return message


def get_random_port(origin_info: dict[str, Any]) -> int:
"""Produce a "random" port number in range 8001-8064 that is reproducible for the current site."""
hasher = hashlib.sha256(json.dumps(origin_info, sort_keys=True).encode())
return DEFAULT_PORT + 1 + hasher.digest()[0] % 64
34 changes: 25 additions & 9 deletions mkdocs/config/config_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import ipaddress
import logging
import os
import re
import string
import sys
import traceback
Expand Down Expand Up @@ -454,9 +455,11 @@ def reset_warnings(self):

class _IpAddressValue(NamedTuple):
host: str
port: int
port: int | None = None

def __str__(self) -> str:
if self.port is None:
return self.host
return f'{self.host}:{self.port}'


Expand All @@ -467,10 +470,28 @@ class IpAddress(OptionallyRequired[_IpAddressValue]):
Validate that an IP address is in an appropriate format
"""

def __init__(self, default: str | None = None, *, port_key: str | None = None) -> None:
super().__init__(default=default)
self.port_key = port_key

def pre_validation(self, config: Config, key_name: str):
self._port: int | None = config[self.port_key] if self.port_key else None

def run_validation(self, value: object) -> _IpAddressValue:
if not isinstance(value, str) or ':' not in value:
raise ValidationError("Must be a string of format 'IP:PORT'")
host, port_str = value.rsplit(':', 1)
if not isinstance(value, str):
raise ValidationError("Must be a string")
port: int | None
if m := re.fullmatch(r'(.+):(\w+)', value):
if self._port is not None:
raise ValidationError(f"The port is already specified through '{self.port_key}'")
try:
port = int(m[2])
except Exception:
raise ValidationError(f"'{m[2]}' is not a valid port")
host = m[1]
else:
port = self._port
host = value

if host != 'localhost':
if host.startswith('[') and host.endswith(']'):
Expand All @@ -481,11 +502,6 @@ def run_validation(self, value: object) -> _IpAddressValue:
except ValueError as e:
raise ValidationError(e)

try:
port = int(port_str)
except Exception:
raise ValidationError(f"'{port_str}' is not a valid port")

return _IpAddressValue(host, port)

def post_validation(self, config: Config, key_name: str):
Expand Down
5 changes: 4 additions & 1 deletion mkdocs/config/defaults.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,12 @@ class MkDocsConfig(base.Config):
"""set of values for Google analytics containing the account IO and domain
this should look like, ['UA-27795084-5', 'mkdocs.org']"""

dev_addr = c.IpAddress(default='127.0.0.1:8000')
dev_addr = c.IpAddress(default='127.0.0.1', port_key='serve_port')
"""The address on which to serve the live reloading docs server."""

serve_port = c.Optional(c.Type(int))
"""The port on which to serve the live reloading docs server."""

use_directory_urls = c.Type(bool, default=True)
"""If `True`, use `<page_name>/index.html` style files with hyperlinks to
the directory. If `False`, use `<page_name>.html style file with
Expand Down
21 changes: 19 additions & 2 deletions mkdocs/livereload/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import functools
import io
import ipaddress
import json
import logging
import mimetypes
import os
Expand All @@ -21,7 +22,7 @@
import webbrowser
import wsgiref.simple_server
import wsgiref.util
from typing import Any, BinaryIO, Callable, Iterable
from typing import Any, BinaryIO, Callable, Iterable, Mapping

import watchdog.events
import watchdog.observers.polling
Expand Down Expand Up @@ -104,8 +105,11 @@ def __init__(
mount_path: str = "/",
polling_interval: float = 0.5,
shutdown_delay: float = 0.25,
*,
origin_info: Mapping[str, Any] | None = None,
) -> None:
self.builder = builder
self._host = host
try:
if isinstance(ipaddress.ip_address(host), ipaddress.IPv6Address):
self.address_family = socket.AF_INET6
Expand All @@ -116,6 +120,7 @@ def __init__(
self.url = _serve_url(host, port, mount_path)
self.build_delay = 0.1
self.shutdown_delay = shutdown_delay
self._origin_info = origin_info
# To allow custom error pages.
self.error_handler: Callable[[int], bytes | None] = lambda code: None

Expand Down Expand Up @@ -170,7 +175,10 @@ def unwatch(self, path: str) -> None:
self.observer.unschedule(self._watch_refs.pop(path))

def serve(self, *, open_in_browser=False):
self.server_bind()
try:
self.server_bind()
except OSError as e:
raise ServerBindError(str(e)) from e
self.server_activate()

if self._watched_paths:
Expand Down Expand Up @@ -282,6 +290,11 @@ def condition():
self._epoch_cond.wait_for(condition, timeout=self.poll_response_timeout)
return [b"%d" % self._visible_epoch]

elif path == "/livereload/.info.json":
start_response("200 OK", [("Content-Type", "application/json")])
info = dict(origin_info=self._origin_info, url=self.url)
return [json.dumps(info).encode()]

if (path + "/").startswith(self.mount_path):
rel_file_path = path[len(self.mount_path) :]

Expand Down Expand Up @@ -367,6 +380,10 @@ def log_message(self, format, *args):
log.debug(format, *args)


class ServerBindError(OSError):
pass


def _timestamp() -> int:
return round(time.monotonic() * 1000)

Expand Down