aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSeason Saw <season.saw@gmail.com>2026-08-01 21:17:23 +0800
committerGitHub <noreply@github.com>2026-08-01 16:17:23 +0300
commitf99647b2c2d7fbfd11269330a25ea4f9be59bafa (patch)
tree1d8c9a35a40da30dc7dd0d1e58c1f833076ad002
parent26a7bc860e4af516ef687d90b9f9dd2830f88d2a (diff)
fix: report a clear cli error when no herdr server is running (#1963)
* fix: report a clear cli error when no herdr server is running socket cli commands surfaced a raw io::Error debug string (`Error: Os { code: 2, ... }`) when nothing was listening on the api socket, which read like a bad --cwd path. map dead-socket connect failures to a `server_not_running` json error carrying the resolved socket path, printed once at the edge that surfaces the error; recovering callers (plugin offline registry fallback, agent start polling) recognize the marker and keep their existing behavior. refs #1941 * fix: map dead-socket errors on the unchecked cli request path refs #1941 * fix: make server-not-running guidance session-aware refs #1941
-rw-r--r--src/cli.rs93
-rw-r--r--src/cli/agent.rs7
-rw-r--r--src/cli/plugin.rs21
-rw-r--r--src/cli/server_not_running.rs74
-rw-r--r--src/cli/status.rs9
-rw-r--r--src/main.rs8
-rw-r--r--tests/cli/sessions.rs63
7 files changed, 255 insertions, 20 deletions
diff --git a/src/cli.rs b/src/cli.rs
index 9d7389b5..4d3c67c0 100644
--- a/src/cli.rs
+++ b/src/cli.rs
@@ -18,6 +18,7 @@ mod plugin;
mod protocol_guard;
mod runtime;
mod server;
+mod server_not_running;
mod spec;
mod status;
mod tab;
@@ -745,17 +746,20 @@ pub(super) fn send_request(request: &Request) -> std::io::Result<serde_json::Val
ensure_server_protocol_compatible(&client, &request.id)?;
client
.request_value(request)
- .map_err(api_client_error_to_io)
+ .map_err(|err| map_server_not_running_or_io(err, &request.id, &client))
}
pub(super) fn send_request_unchecked(request: &Request) -> std::io::Result<serde_json::Value> {
- ApiClient::local()
+ let client = ApiClient::local();
+ client
.request_value(request)
- .map_err(api_client_error_to_io)
+ .map_err(|err| map_server_not_running_or_io(err, &request.id, &client))
}
fn ensure_server_protocol_compatible(client: &ApiClient, request_id: &str) -> std::io::Result<()> {
- let status = client.status().map_err(api_client_error_to_io)?;
+ let status = client
+ .status()
+ .map_err(|err| map_server_not_running_or_io(err, request_id, client))?;
let server_protocol = status
.protocol
.ok_or_else(|| std::io::Error::other("server ping did not include a protocol version"))?;
@@ -778,6 +782,49 @@ pub(crate) fn protocol_mismatch_was_reported(err: &std::io::Error) -> bool {
protocol_guard::was_reported(err)
}
+pub(crate) fn server_not_running_was_reported(err: &std::io::Error) -> bool {
+ server_not_running::was_reported(err)
+}
+
+/// Returns the `ErrorResponse` carried by a `server_not_running` marker, if any,
+/// so the edge that surfaces the error can print it exactly once (deferred
+/// printing: recovering callers like plugin offline fallback print nothing).
+pub(crate) fn server_not_running_reported_response(
+ err: &std::io::Error,
+) -> Option<&crate::api::schema::ErrorResponse> {
+ server_not_running::reported_response(err)
+}
+
+/// True when an io::Error indicates nothing is listening on the API socket.
+/// Classify by `ErrorKind` only: Windows named pipes surface different raw
+/// errno values than Unix domain sockets but the same error kinds.
+pub(super) fn server_not_running_error(err: &std::io::Error) -> bool {
+ matches!(
+ err.kind(),
+ std::io::ErrorKind::NotFound | std::io::ErrorKind::ConnectionRefused
+ )
+}
+
+/// Maps an `ApiClientError` from a socket command into the io::Error that
+/// bubbles up to `main`. A dead-server connect failure is reported as a
+/// friendly `server_not_running` JSON error plus a recognizable marker; all
+/// other errors fall through unchanged so existing handling is preserved.
+fn map_server_not_running_or_io(
+ err: ApiClientError,
+ request_id: &str,
+ client: &ApiClient,
+) -> std::io::Error {
+ match err {
+ ApiClientError::Io(io_err) if server_not_running_error(&io_err) => {
+ server_not_running::reported_error(server_not_running::response(
+ request_id,
+ &client.socket_path(),
+ ))
+ }
+ err => api_client_error_to_io(err),
+ }
+}
+
fn api_client_error_to_io(err: ApiClientError) -> std::io::Error {
match err {
ApiClientError::Io(err) => err,
@@ -1036,4 +1083,42 @@ mod tests {
"env must use KEY=VALUE"
);
}
+
+ #[test]
+ fn maps_dead_server_connect_failure_to_friendly_error() {
+ use crate::api::client::{ApiClient, ApiClientError};
+
+ let client = ApiClient::local();
+ let socket = client.socket_path().display().to_string();
+
+ // The helper does NOT print; it returns a recognizable marker carrying
+ // the ErrorResponse so the surfacing edge can print it exactly once.
+ let mapped = super::map_server_not_running_or_io(
+ ApiClientError::Io(std::io::Error::from(std::io::ErrorKind::NotFound)),
+ "cli:workspace:create",
+ &client,
+ );
+
+ let response = super::server_not_running::reported_response(&mapped)
+ .expect("dead-server connect failure should carry a server_not_running response");
+ assert_eq!(response.id, "cli:workspace:create");
+ assert_eq!(response.error.code, "server_not_running");
+ assert!(response.error.message.contains(&socket));
+
+ // The mapping is recognizable without string matching.
+ assert!(super::server_not_running::was_reported(&mapped));
+ }
+
+ #[test]
+ fn classifier_ignores_unrelated_io_kinds() {
+ use crate::api::client::{ApiClient, ApiClientError};
+
+ let client = ApiClient::local();
+ let mapped = super::map_server_not_running_or_io(
+ ApiClientError::Io(std::io::Error::from(std::io::ErrorKind::TimedOut)),
+ "cli:workspace:create",
+ &client,
+ );
+ assert!(!super::server_not_running::was_reported(&mapped));
+ }
}
diff --git a/src/cli/agent.rs b/src/cli/agent.rs
index e48d6b3b..e1271f48 100644
--- a/src/cli/agent.rs
+++ b/src/cli/agent.rs
@@ -577,6 +577,13 @@ fn print_agent_transport_error(
if super::protocol_mismatch_was_reported(&err) {
return Ok(1);
}
+ // A dead-server marker reaches here from `send_request` in the agent
+ // startup path; surface its deferred response exactly once instead of
+ // printing a second, generic transport-error line.
+ if let Some(response) = super::server_not_running_reported_response(&err) {
+ let value = serde_json::to_value(response).map_err(std::io::Error::other)?;
+ return super::print_response(&value);
+ }
super::print_response(&cli_agent_error(request_id, code, err.to_string()))
}
diff --git a/src/cli/plugin.rs b/src/cli/plugin.rs
index b4e6d514..efdb9eca 100644
--- a/src/cli/plugin.rs
+++ b/src/cli/plugin.rs
@@ -1618,14 +1618,19 @@ fn current_unix_ms() -> u64 {
}
fn is_connection_error(err: &std::io::Error) -> bool {
- matches!(
- err.kind(),
- std::io::ErrorKind::NotFound
- | std::io::ErrorKind::ConnectionRefused
- | std::io::ErrorKind::ConnectionAborted
- | std::io::ErrorKind::ConnectionReset
- | std::io::ErrorKind::BrokenPipe
- )
+ // A `server_not_running` marker is a connect failure for recovery purposes:
+ // treating it as a connection error lets plugin commands fall back to the
+ // offline registry. The marker carries (but does not print) a friendly
+ // response, so recovering here prints nothing.
+ super::server_not_running_was_reported(err)
+ || matches!(
+ err.kind(),
+ std::io::ErrorKind::NotFound
+ | std::io::ErrorKind::ConnectionRefused
+ | std::io::ErrorKind::ConnectionAborted
+ | std::io::ErrorKind::ConnectionReset
+ | std::io::ErrorKind::BrokenPipe
+ )
}
fn print_plugin_response(method: Method) -> std::io::Result<i32> {
diff --git a/src/cli/server_not_running.rs b/src/cli/server_not_running.rs
new file mode 100644
index 00000000..9e974f3c
--- /dev/null
+++ b/src/cli/server_not_running.rs
@@ -0,0 +1,74 @@
+use std::fmt;
+use std::path::Path;
+
+use crate::api::schema::{ErrorBody, ErrorResponse};
+
+/// Marker error signalling a dead API socket. Carries the `ErrorResponse` that
+/// should be printed at the edge that finally surfaces the error, so callers
+/// that recover (e.g. plugin offline fallback) print nothing. Mirrors
+/// `ProtocolMismatchReported`, except printing is deferred because several CLI
+/// commands recover from a dead server instead of reporting it.
+#[derive(Debug)]
+pub(super) struct ServerNotRunningReported {
+ pub(super) response: ErrorResponse,
+}
+
+impl fmt::Display for ServerNotRunningReported {
+ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+ // Delegate to the carried message so pre-existing paths that
+ // stringify transport errors still show the actionable text.
+ f.write_str(&self.response.error.message)
+ }
+}
+
+impl std::error::Error for ServerNotRunningReported {}
+
+/// Builds the friendly `server_not_running` ErrorResponse shown when no
+/// server is listening on the resolved API socket.
+pub(super) fn response(request_id: &str, socket_path: &Path) -> ErrorResponse {
+ let attach_command = startup_command(socket_path);
+ ErrorResponse {
+ id: request_id.to_string(),
+ error: ErrorBody {
+ code: "server_not_running".into(),
+ message: format!(
+ "no herdr server is running at {}; run `{attach_command}` to start or attach it",
+ socket_path.display()
+ ),
+ },
+ }
+}
+
+fn startup_command(socket_path: &Path) -> String {
+ let session_socket =
+ crate::session::api_socket_path_for(crate::session::active_name().as_deref());
+ if socket_path == session_socket {
+ crate::session::local_attach_command()
+ } else {
+ // A socket override wins over an inherited HERDR_SESSION. Keep the
+ // command in the current environment so it starts the overridden
+ // target instead of directing the user to an unrelated session.
+ "herdr".to_string()
+ }
+}
+
+/// Wraps the response in the recognizable marker WITHOUT printing. The caller
+/// that ultimately surfaces the error prints the carried response (see
+/// `reported_response`); recovering callers simply drop it.
+pub(super) fn reported_error(response: ErrorResponse) -> std::io::Error {
+ std::io::Error::other(ServerNotRunningReported { response })
+}
+
+pub(super) fn was_reported(err: &std::io::Error) -> bool {
+ err.get_ref()
+ .and_then(|source| source.downcast_ref::<ServerNotRunningReported>())
+ .is_some()
+}
+
+/// Returns the `ErrorResponse` carried by a `server_not_running` marker, if any,
+/// so the surfacing edge can print it exactly once.
+pub(super) fn reported_response(err: &std::io::Error) -> Option<&ErrorResponse> {
+ err.get_ref()
+ .and_then(|source| source.downcast_ref::<ServerNotRunningReported>())
+ .map(|reported| &reported.response)
+}
diff --git a/src/cli/status.rs b/src/cli/status.rs
index 49b3c129..74bb3e2a 100644
--- a/src/cli/status.rs
+++ b/src/cli/status.rs
@@ -159,20 +159,13 @@ fn read_server_runtime_status() -> std::io::Result<ServerRuntimeStatus> {
protocol: status.protocol,
capabilities: status.capabilities,
}),
- Err(ApiClientError::Io(err)) if server_not_running_error(&err) => {
+ Err(ApiClientError::Io(err)) if super::server_not_running_error(&err) => {
Ok(ServerRuntimeStatus::NotRunning)
}
Err(err) => Err(api_client_error_to_io(err)),
}
}
-fn server_not_running_error(err: &std::io::Error) -> bool {
- matches!(
- err.kind(),
- std::io::ErrorKind::NotFound | std::io::ErrorKind::ConnectionRefused
- )
-}
-
fn api_client_error_to_io(err: ApiClientError) -> std::io::Error {
match err {
ApiClientError::Io(err) => err,
diff --git a/src/main.rs b/src/main.rs
index 8fc51d6b..7b4058d2 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -497,6 +497,14 @@ fn main() -> io::Result<()> {
Ok(cli::CommandOutcome::Handled(code)) => std::process::exit(code),
Ok(cli::CommandOutcome::NotCli) => {}
Err(err) if cli::protocol_mismatch_was_reported(&err) => std::process::exit(1),
+ Err(err) if cli::server_not_running_was_reported(&err) => {
+ if let Some(response) = cli::server_not_running_reported_response(&err) {
+ if let Ok(json) = serde_json::to_string(response) {
+ eprintln!("{json}");
+ }
+ }
+ std::process::exit(1);
+ }
Err(err) => return Err(err),
}
diff --git a/tests/cli/sessions.rs b/tests/cli/sessions.rs
index 8cb0f4b7..ae7b8306 100644
--- a/tests/cli/sessions.rs
+++ b/tests/cli/sessions.rs
@@ -185,6 +185,69 @@ fn named_sessions_use_separate_servers_and_workspace_state() {
}
#[test]
+fn dead_server_cli_reports_one_session_aware_json_line() {
+ fn assert_server_not_running(
+ output: std::process::Output,
+ socket_path: &Path,
+ attach_command: &str,
+ ) {
+ assert_eq!(
+ output.status.code(),
+ Some(1),
+ "stdout={} stderr={}",
+ String::from_utf8_lossy(&output.stdout),
+ String::from_utf8_lossy(&output.stderr)
+ );
+ assert!(output.stdout.is_empty(), "server errors belong on stderr");
+
+ let stderr = String::from_utf8(output.stderr).unwrap();
+ let lines: Vec<_> = stderr.lines().collect();
+ assert_eq!(lines.len(), 1, "expected exactly one JSON line: {stderr:?}");
+
+ let response: serde_json::Value = serde_json::from_str(lines[0]).unwrap();
+ assert_eq!(response["id"], "cli:workspace:create");
+ assert_eq!(response["error"]["code"], "server_not_running");
+ assert_eq!(
+ response["error"]["message"],
+ format!(
+ "no herdr server is running at {}; run `{attach_command}` to start or attach it",
+ socket_path.display()
+ )
+ );
+ }
+
+ let base = unique_test_dir();
+ let config_home = base.join("config");
+ let runtime_dir = base.join("runtime");
+ fs::create_dir_all(&runtime_dir).unwrap();
+ register_runtime_dir(&runtime_dir);
+
+ let named_socket = named_session_socket(&config_home, "foo");
+ let missing = run_named_cli(
+ &config_home,
+ &runtime_dir,
+ &["--session", "foo", "workspace", "create"],
+ );
+ assert_server_not_running(missing, &named_socket, "herdr session attach foo");
+
+ let stale_socket = runtime_dir.join("stale.sock");
+ drop(UnixListener::bind(&stale_socket).unwrap());
+ let stale = Command::new(env!("CARGO_BIN_EXE_herdr"))
+ .args(["workspace", "create"])
+ .env("XDG_CONFIG_HOME", &config_home)
+ .env("XDG_RUNTIME_DIR", &runtime_dir)
+ .env("HERDR_SOCKET_PATH", &stale_socket)
+ .env("HERDR_SESSION", "unrelated")
+ .env_remove("HERDR_CLIENT_SOCKET_PATH")
+ .env_remove("HERDR_ENV")
+ .output()
+ .unwrap();
+ assert_server_not_running(stale, &stale_socket, "herdr");
+
+ cleanup_test_base(&base);
+}
+
+#[test]
fn integration_commands_run_locally_when_server_is_missing() {
let base = unique_test_dir();
let home_dir = base.join("home");