Enforce server binding at compile time instead of runtime panics

[coderabbitai]

Problem

The run() and run_with_shutdown() methods in WireframeServer currently panic at runtime if bind() wasn't called beforehand. This leads to poor developer experience and potential runtime crashes.

Suggested Solution

Consider using a typestate or builder pattern to enforce binding at compile time instead of panicking at runtime. This would make the API safer and catch configuration errors during development rather than in production.

Context

Identified during code review of PR #262: #262
Comment: #262 (comment)
Issue: #263

Files Affected

• src/server/runtime.rs
• src/server/config.rs

[leynos]

@coderabbitai looking at the codebase, is this issue still relevant? If the issue has been fixed in the code, does the documentation reflect this?

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, let's check if the mentioned files exist and explore the server directory structure
echo "=== Checking server directory structure ==="
fd -t f . src/server/ 2>/dev/null || echo "src/server/ directory not found"

echo -e "\n=== Looking for WireframeServer implementation ==="
# Search for WireframeServer struct and its methods
ast-grep --pattern 'struct WireframeServer'

echo -e "\n=== Looking for bind method ==="
ast-grep --pattern 'fn bind($$$) {
  $$$
}'

echo -e "\n=== Looking for run methods ==="
ast-grep --pattern 'fn run($$$) {
  $$$
}'

ast-grep --pattern 'fn run_with_shutdown($$$) {
  $$$
}'

echo -e "\n=== Searching for panic calls in server-related files ==="
# Look for panic! macro usage in server files
rg "panic!" --glob "**/*server*" -A 2 -B 2

echo -e "\n=== Checking for typestate or builder pattern implementations ==="
# Look for builder pattern or typestate-related structs
rg "(Builder|State|Bound|Unbound)" --glob "**/*server*" -A 2

Length of output: 1118

---

🏁 Script executed:

#!/bin/bash
echo "=== Content of src/server/runtime.rs ==="
cat src/server/runtime.rs

echo -e "\n\n=== Content of src/server/mod.rs ==="
cat src/server/mod.rs

echo -e "\n\n=== Looking for README or documentation files ==="
fd -t f "README" . | head -5
fd -t f -e md . docs/ 2>/dev/null | head -10

Length of output: 11482

---

🏁 Script executed:

#!/bin/bash
echo "=== Content of src/server/config/mod.rs ==="
cat src/server/config/mod.rs

echo -e "\n\n=== Checking for bind-related examples in documentation ==="
rg "bind\(" -A 3 -B 1 --glob "*.md"

echo -e "\n\n=== Looking for any examples or tests showing server usage ==="
rg "WireframeServer::new" -A 10 -B 2

Length of output: 38061

---

@leynos, after examining the codebase, this issue is still partially relevant. Here's what I found:

Current Implementation

The code has been improved to avoid runtime panics - instead of panicking, run() and run_with_shutdown() now return an io::Error with the message "listener not bound" if bind() wasn't called first:

// In src/server/runtime.rs
let listener = listener.ok_or_else(|| io::Error::other("listener not bound"))?;

Issue Status

✅ Fixed: Runtime panics have been replaced with proper error handling
❌ Not Fixed: The suggested compile-time enforcement using typestate or builder pattern hasn't been implemented

The current API still allows constructing code that will fail at runtime:

// This compiles but fails at runtime
WireframeServer::new(factory).run().await  // Returns Err: "listener not bound"

Documentation

The documentation correctly reflects the current implementation. All examples show the required pattern:

WireframeServer::new(factory)
    .bind(address)?  // Required step
    .run()
    .await

The issue remains relevant if you want to enforce binding at compile-time to catch configuration errors during development rather than at runtime. The typestate pattern suggested in the issue would make it impossible to call run() without first calling bind(), providing a safer API.