Address review comments

Signed-off-by: Casper Beyer <casper@synadia.com>

Author: caspervonb

.github/workflows/test.yml

@@ -52,25 +52,25 @@ jobs:
         strategy:
             matrix:
                 python-version: ["3.11", "3.12", "3.13"]
-                os: ["ubuntu-latest", "macos-latest"]
+                os: ["ubuntu-latest", "macos-latest", "windows-latest"]
                 nats-server-version: ["latest"]
                 project: ["nats-server"]
         steps:
             - name: Checkout repository
-              uses: actions/checkout@v2
+              uses: actions/checkout@v5
 
             - name: Set up Go
-              uses: actions/setup-go@v2
+              uses: actions/setup-go@v6
               with:
-                  go-version: "1.23.4"
+                  go-version: "stable"
 
             - name: Install NATS Server
               run: |
                   go install github.com/nats-io/nats-server/v2@${{ matrix.nats-server-version }}
               shell: bash
 
             - name: Set up Python ${{ matrix.python-version }}
-              uses: actions/setup-python@v2
+              uses: actions/setup-python@v6
               with:
                   python-version: ${{ matrix.python-version }}
 

nats-server/README.md

@@ -12,7 +12,7 @@ Manage NATS server instances from python.
 pip install nats-server
 ```
 
-Requires Python 3.8+ and the NATS server binary in your PATH.
+Requires Python 3.11+ and the NATS server binary in your PATH.
 
 ## Usage
 
@@ -26,6 +26,7 @@ async def main():
     print(f"Server running on {server.host}:{server.port}")
 
     # Do something with the server...
+    # e.g connect with server.client_url
 
     # Clean shutdown
     await server.shutdown()

nats-server/src/nats/server/__init__.py

@@ -28,12 +28,12 @@
 logger = logging.getLogger(__name__)
 
 # Regex patterns for parsing server output
+FATAL_PATTERN = re.compile(r"\[FTL\]\s+(.*)")
+INFO_PATTERN = re.compile(r"\[INF\]\s+(.*)")
 READY_PATTERN = re.compile(r"Server is ready")
 LISTENING_PATTERN = re.compile(
     r"Listening for client connections on ([^:]+):(\d+)"
 )
-FATAL_PATTERN = re.compile(r"\[FTL\]\s+(.*)")
-INFO_PATTERN = re.compile(r"INFO\s+({.*})")
 
 
 class ServerError(Exception):
@@ -66,7 +66,6 @@ def __init__(
         self._port = port
         self._jetstream = jetstream
         self._connect_urls = connect_urls or []
-        self._is_running = True
 
     @property
     def host(self) -> str:
@@ -190,41 +189,38 @@ async def run(
     )
 
     async def wait_ready() -> tuple[str, int]:
-        host = None
-        port = None
+        """Clean version using early returns and better error handling"""
+        host = port = None
         error_lines = []
 
-        while True:
-            line = await process.stderr.readline()
-            if not line:
-                # EOF reached, process stderr for errors
-                await process.wait()
+        async for stderr_line in process.stderr:
+            stderr_line = stderr_line.decode().strip()
 
-                # If the process exited with an error code and we have stderr content
-                if process.returncode != 0:
-                    if error_lines:
-                        raise ServerError("\n".join(error_lines))
+            if READY_PATTERN.search(stderr_line):
+                return host, port
 
-                    msg = f"Server exited with code {process.returncode}"
-                    raise ServerError(msg)
+            if match := LISTENING_PATTERN.search(stderr_line):
+                host, port = match.group(1), int(match.group(2))
 
-            line_str = line.decode()
-            # Store all stderr lines for potential error reporting
-            error_lines.append(line_str.strip())
+            if INFO_PATTERN.search(stderr_line):
+                continue
 
-            # Check for fatal errors
-            if match := FATAL_PATTERN.search(line_str):
-                raise ServerError(match.group(1))
+            if match := FATAL_PATTERN.search(stderr_line):
+                error_lines.append(match.group(1))
 
-            # Get host and port from listening message
-            if match := LISTENING_PATTERN.search(line_str):
-                host = match.group(1)
-                port = int(match.group(2))
+            error_lines.append(stderr_line)
 
-            # Check if server is ready
-            if READY_PATTERN.search(line_str):
-                error_lines.clear()
-                return host, port
+        # Process ended without becoming being ready
+        returncode = await process.wait()
+
+        if returncode != 0:
+            msg = f"Server exited with code {returncode}"
+            if error_lines:
+                msg += f"\nErrors:\n{''.join(error_lines)}"
+
+            raise ServerError(msg)
+
+        raise ServerError("Server ended without becoming ready")
 
     assigned_host, assigned_port = await asyncio.wait_for(
         wait_ready(), timeout=timeout

nats-server/tests/test_server.py

@@ -201,3 +201,17 @@ async def test_run_with_jetstream_and_store_dir(tmp_path):
     finally:
         # Always shutdown the server
         await server.shutdown()
+
+
+async def test_run_with_store_dir_as_file(tmp_path):
+    """Test the run function fails when store_dir points to a file instead of a directory."""
+    # Create a file instead of a directory
+    store_file = tmp_path / "not_a_directory"
+    store_file.write_text("this is a file, not a directory")
+
+    # Try to start server with JetStream using a file as store_dir
+    with pytest.raises(ServerError) as exc_info:
+        await run(port=0, jetstream=True, store_dir=str(store_file), timeout=2.0)
+
+    # Verify the error message indicates the storage directory issue
+    assert "storage directory is not a directory" in str(exc_info.value).lower()