CLI: Bedrock support and misc improvements (#849)

* fix crash with one command-line argument

* implement ping() on BedrockServer

simply measures the latency of status()

* support Bedrock servers in CLI

done in a slightly ad-hoc way, but this is the best
we can do given the split of the response types.

* print server kind and tweak player sample printing

* JavaServer ping() doesn't work?

* fix precommit warnings

* review: remove Bedrock ping()

* review: change CLI ping comment to be more permanent

* review: formalise hostip/hostport within QueryResponse

* review: only squash traceback in common errors

* review: leading line break for multi-line motd

* Revert "review: formalise hostip/hostport within QueryResponse"

This reverts commit 3a0ee8c.

* review: use motd.to_minecraft() in json

* review amendment: factor out motd line breaking

* review: refactor CLI json() to use dataclasses.asdict()

* amendment: add NoNameservers and remove ValueError from squashed errors

ValueError might be thrown by programming errors in
json handling, for example.

* review: fallback logic in CLI ping

since this runs both ping() then status(), it can report
precisely when one fails and the other succeeds.

some kludgy logic to switch bedrock too.

* review: use ip/port fields in CLI's JSON output

in anticipation of #536

Co-authored-by: Perchun Pak <[email protected]>

* review: avoid kind() classmethod

* review: clarify MOTD serialisation comment

* review: simplify ping fallback logic

Co-authored-by: Perchun Pak <[email protected]>

* make version consistent between status and query

* review: apply simplify() to motd in CLI JSON output

Co-authored-by: Perchun Pak <[email protected]>

* review: use separate JSON field for simplified MOTD

* review: remove MOTD fixup comment

* review: update README with new CLI

* review: no raw motd

* no --help output in readme

* review: allow main() with no arguments

* Update mcstatus/__main__.py

Co-authored-by: Kevin Tindall <[email protected]>

* avoid json collision

* oops! good linter

* drike review

* good linter

* one more ci failure and i turn on the computer

* also squash ConnectionError

happens during server startup, for example

---------

Co-authored-by: Perchun Pak <[email protected]>
Co-authored-by: Kevin Tindall <[email protected]>

README.md

@@ -67,7 +67,10 @@ See the [documentation](https://mcstatus.readthedocs.io) to find what you can do
 
 ### Command Line Interface
 
-This only works with Java servers; Bedrock is not yet supported. Use `mcstatus -h` to see helpful information on how to use this script.
+The mcstatus library includes a simple CLI. Once installed, it can be used through:
+```bash
+python3 -m mcstatus --help
+```
 
 ## License
 

mcstatus/__main__.py

@@ -1,75 +1,159 @@
 from __future__ import annotations
 
+import dns.resolver
+import sys
+import json
 import argparse
 import socket
-from json import dumps as json_dumps
+import dataclasses
+from typing import TYPE_CHECKING
 
-from mcstatus import JavaServer
+from mcstatus import JavaServer, BedrockServer
+from mcstatus.responses import JavaStatusResponse
+from mcstatus.motd import Motd
 
+if TYPE_CHECKING:
+    SupportedServers = JavaServer | BedrockServer
 
-def ping(server: JavaServer) -> None:
-    print(f"{server.ping()}ms")
 
+def _motd(motd: Motd) -> str:
+    """Formats MOTD for human-readable output, with leading line break
+    if multiline."""
+    s = motd.to_ansi()
+    return f"\n{s}" if "\n" in s else f" {s}"
 
-def status(server: JavaServer) -> None:
+
+def _kind(serv: SupportedServers) -> str:
+    if isinstance(serv, JavaServer):
+        return "Java"
+    elif isinstance(serv, BedrockServer):
+        return "Bedrock"
+    else:
+        raise ValueError(f"unsupported server for kind: {serv}")
+
+
+def _ping_with_fallback(server: SupportedServers) -> float:
+    # bedrock doesn't have ping method
+    if isinstance(server, BedrockServer):
+        return server.status().latency
+
+    # try faster ping packet first, falling back to status with a warning.
+    ping_exc = None
+    try:
+        return server.ping(tries=1)
+    except Exception as e:
+        ping_exc = e
+
+    latency = server.status().latency
+
+    address = f"{server.address.host}:{server.address.port}"
+    print(
+        f"warning: contacting {address} failed with a 'ping' packet but succeeded with a 'status' packet,\n"
+        f"         this is likely a bug in the server-side implementation.\n"
+        f'         (note: ping packet failed due to "{ping_exc}")\n'
+        f"         for more details, see: https://mcstatus.readthedocs.io/en/stable/pages/faq/\n",
+        file=sys.stderr,
+    )
+
+    return latency
+
+
+def ping_cmd(server: SupportedServers) -> int:
+    print(_ping_with_fallback(server))
+    return 0
+
+
+def status_cmd(server: SupportedServers) -> int:
     response = server.status()
-    if response.players.sample is not None:
-        player_sample = str([f"{player.name} ({player.id})" for player in response.players.sample])
+
+    java_res = response if isinstance(response, JavaStatusResponse) else None
+
+    if not java_res:
+        player_sample = ""
+    elif java_res.players.sample is not None:
+        player_sample = str([f"{player.name} ({player.id})" for player in java_res.players.sample])
     else:
         player_sample = "No players online"
 
-    print(f"version: v{response.version.name} (protocol {response.version.protocol})")
-    print(f'motd: "{response.motd}"')
-    print(f"players: {response.players.online}/{response.players.max} {player_sample}")
+    if player_sample:
+        player_sample = " " + player_sample
+
+    print(f"version: {_kind(server)} {response.version.name} (protocol {response.version.protocol})")
+    print(f"motd:{_motd(response.motd)}")
+    print(f"players: {response.players.online}/{response.players.max}{player_sample}")
+    print(f"ping: {response.latency:.2f} ms")
+    return 0
 
 
-def json(server: JavaServer) -> None:
-    data = {}
-    data["online"] = False
-    # Build data with responses and quit on exception
+def json_cmd(server: SupportedServers) -> int:
+    data = {"online": False, "kind": _kind(server)}
+
+    status_res = query_res = exn = None
     try:
         status_res = server.status(tries=1)
-        data["version"] = status_res.version.name
-        data["protocol"] = status_res.version.protocol
-        data["motd"] = status_res.motd.raw
-        data["player_count"] = status_res.players.online
-        data["player_max"] = status_res.players.max
-        data["players"] = []
-        if status_res.players.sample is not None:
-            data["players"] = [{"name": player.name, "id": player.id} for player in status_res.players.sample]
-
-        data["ping"] = status_res.latency
-        data["online"] = True
-
-        query_res = server.query(tries=1)  # type: ignore[call-arg] # tries is supported with retry decorator
-        data["host_ip"] = query_res.raw["hostip"]
-        data["host_port"] = query_res.raw["hostport"]
-        data["map"] = query_res.map
-        data["plugins"] = query_res.software.plugins
-    except Exception:  # TODO: Check what this actually excepts
-        pass
-    print(json_dumps(data))
-
-
-def query(server: JavaServer) -> None:
+    except Exception as e:
+        exn = exn or e
+
+    try:
+        if isinstance(server, JavaServer):
+            query_res = server.query(tries=1)
+    except Exception as e:
+        exn = exn or e
+
+    # construct 'data' dict outside try/except to ensure data processing errors
+    # are noticed.
+    data["online"] = bool(status_res or query_res)
+    if not data["online"]:
+        assert exn, "server offline but no exception?"
+        data["error"] = str(exn)
+
+    if status_res is not None:
+        data["status"] = dataclasses.asdict(status_res)
+
+        # ensure we are overwriting the motd and not making a new dict field
+        assert "motd" in data["status"], "motd field missing. has it been renamed?"
+        data["status"]["motd"] = status_res.motd.simplify().to_minecraft()
+
+    if query_res is not None:
+        # TODO: QueryResponse is not (yet?) a dataclass
+        data["query"] = qdata = {}
+
+        qdata["ip"] = query_res.raw["hostip"]
+        qdata["port"] = query_res.raw["hostport"]
+        qdata["map"] = query_res.map
+        qdata["plugins"] = query_res.software.plugins
+        qdata["raw"] = query_res.raw
+
+    json.dump(data, sys.stdout)
+    return 0
+
+
+def query_cmd(server: SupportedServers) -> int:
+    if not isinstance(server, JavaServer):
+        print("The 'query' protocol is only supported by Java servers.", file=sys.stderr)
+        return 1
+
     try:
         response = server.query()
     except socket.timeout:
         print(
             "The server did not respond to the query protocol."
             "\nPlease ensure that the server has enable-query turned on,"
             " and that the necessary port (same as server-port unless query-port is set) is open in any firewall(s)."
-            "\nSee https://wiki.vg/Query for further information."
+            "\nSee https://wiki.vg/Query for further information.",
+            file=sys.stderr,
         )
-        return
+        return 1
+
     print(f"host: {response.raw['hostip']}:{response.raw['hostport']}")
-    print(f"software: v{response.software.version} {response.software.brand}")
+    print(f"software: {_kind(server)} {response.software.version} {response.software.brand}")
+    print(f"motd:{_motd(response.motd)}")
     print(f"plugins: {response.software.plugins}")
-    print(f'motd: "{response.motd}"')
     print(f"players: {response.players.online}/{response.players.max} {response.players.names}")
+    return 0
 
 
-def main() -> None:
+def main(argv: list[str] = sys.argv[1:]) -> int:
     parser = argparse.ArgumentParser(
         "mcstatus",
         description="""
@@ -80,25 +164,34 @@ def main() -> None:
     )
 
     parser.add_argument("address", help="The address of the server.")
+    parser.add_argument("--bedrock", help="Specifies that 'address' is a Bedrock server (default: Java).", action="store_true")
+
+    subparsers = parser.add_subparsers(title="commands", description="Command to run, defaults to 'status'.")
+    parser.set_defaults(func=status_cmd)
 
-    subparsers = parser.add_subparsers()
-    subparsers.add_parser("ping", help="Ping server for latency.").set_defaults(func=ping)
+    subparsers.add_parser("ping", help="Ping server for latency.").set_defaults(func=ping_cmd)
     subparsers.add_parser(
         "status", help="Prints server status. Supported by all Minecraft servers that are version 1.7 or higher."
-    ).set_defaults(func=status)
+    ).set_defaults(func=status_cmd)
     subparsers.add_parser(
         "query", help="Prints detailed server information. Must be enabled in servers' server.properties file."
-    ).set_defaults(func=query)
+    ).set_defaults(func=query_cmd)
     subparsers.add_parser(
         "json",
         help="Prints server status and query in json. Supported by all Minecraft servers that are version 1.7 or higher.",
-    ).set_defaults(func=json)
+    ).set_defaults(func=json_cmd)
 
-    args = parser.parse_args()
-    server = JavaServer.lookup(args.address)
+    args = parser.parse_args(argv)
+    lookup = JavaServer.lookup if not args.bedrock else BedrockServer.lookup
 
-    args.func(server)
+    try:
+        server = lookup(args.address)
+        return args.func(server)
+    except (socket.timeout, socket.gaierror, dns.resolver.NoNameservers, ConnectionError) as e:
+        # catch and hide traceback for expected user-facing errors
+        print(f"Error: {e}", file=sys.stderr)
+        return 1
 
 
 if __name__ == "__main__":
-    main()
+    sys.exit(main())