Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 89 additions & 4 deletions src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ mod plugin;
mod protocol_guard;
mod runtime;
mod server;
mod server_not_running;
mod spec;
mod status;
mod tab;
Expand Down Expand Up @@ -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))
}
Comment thread
greptile-apps[bot] marked this conversation as resolved.

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"))?;
Expand All @@ -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,
Expand Down Expand Up @@ -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));
}
}
7 changes: 7 additions & 0 deletions src/cli/agent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()))
}

Expand Down
21 changes: 13 additions & 8 deletions src/cli/plugin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
74 changes: 74 additions & 0 deletions src/cli/server_not_running.rs
Original file line number Diff line number Diff line change
@@ -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)
}
9 changes: 1 addition & 8 deletions src/cli/status.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
8 changes: 8 additions & 0 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}

Expand Down
63 changes: 63 additions & 0 deletions tests/cli/sessions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,69 @@ fn named_sessions_use_separate_servers_and_workspace_state() {
cleanup_test_base(&base);
}

#[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();
Expand Down
Loading