From e25c3e8243b54767be31aa427f17dd4e6bb9db48 Mon Sep 17 00:00:00 2001 From: Maciek <108873945+rozwader@users.noreply.github.com> Date: Thu, 17 Sep 2026 23:23:46 +0200 Subject: [PATCH] feat: better error handling for process hooks (#847) --- packages/oneclient_core/src/game/error.rs | 24 ++++ packages/oneclient_core/src/game/launch.rs | 137 +++++++++++++++++---- 2 files changed, 138 insertions(+), 23 deletions(-) diff --git a/packages/oneclient_core/src/game/error.rs b/packages/oneclient_core/src/game/error.rs index 95d5d3f5..5057273a 100644 --- a/packages/oneclient_core/src/game/error.rs +++ b/packages/oneclient_core/src/game/error.rs @@ -37,4 +37,28 @@ pub enum GameError { #[error("failed to spawn the game process: {0}")] Spawn(String), + + #[error( + "the wrapper command '{program}' could not be started: {reason}. Check the Wrapper Command setting." + )] + WrapperSpawn { program: String, reason: String }, +} + +impl GameError { + pub(crate) fn wrapper_spawn(program: &str, err: &std::io::Error) -> Self { + let reason = match err.kind() { + std::io::ErrorKind::NotFound => { + "command not found".to_string() + } + std::io::ErrorKind::PermissionDenied => { + "insufficient permissions or file is not an executable".to_string() + } + _ => err.to_string(), + }; + + Self::WrapperSpawn { + program: program.to_string(), + reason, + } + } } diff --git a/packages/oneclient_core/src/game/launch.rs b/packages/oneclient_core/src/game/launch.rs index 01ac1539..e58868b0 100644 --- a/packages/oneclient_core/src/game/launch.rs +++ b/packages/oneclient_core/src/game/launch.rs @@ -5,6 +5,7 @@ use std::time::Duration; use chrono::{DateTime, Utc}; use interfrost::api::minecraft::ArgumentType; +use tokio::io::AsyncReadExt; use tokio::process::Command; use crate::ClusterStage; @@ -329,7 +330,13 @@ async fn start( )?; arguments::append_profile_game_arguments(&mut mc_args, profile.force_fullscreen, None); - run_hook(profile.hook_pre.as_deref(), &cwd).await; + if let Some(reason) = run_hook(profile.hook_pre.as_deref(), &cwd).await { + events + .notify("Pre-launch command failed") + .body(reason) + .error() + .send(); + } tracing::info!( cluster_id, @@ -341,7 +348,7 @@ async fn start( ); tracing::debug!(cluster_id, ?jvm_args, main_class = %version_info.main_class, "jvm arguments"); - let mut command = base_command(&profile, &java.absolute_path); + let (mut command, wrapper) = base_command(&profile, &java.absolute_path); if profile.use_discrete_gpu() { crate::game::gpu::prefer_discrete(&mut command, &java.absolute_path).await; @@ -384,9 +391,10 @@ async fn start( command.stdin(Stdio::null()); detach(&mut command); - let mut child = command - .spawn() - .map_err(|err| GameError::Spawn(err.to_string()))?; + let mut child = command.spawn().map_err(|err| match &wrapper { + Some(program) => GameError::wrapper_spawn(program, &err), + None => GameError::Spawn(err.to_string()), + })?; let pid = child.id(); stage(LaunchStage::Running); @@ -563,7 +571,15 @@ pub(crate) async fn finalize_session( recorder.finish_at(&end.ended_at.to_rfc3339(), code).await; } - run_hook(post_hook, cwd).await; + if let Some(reason) = run_hook(post_hook, cwd).await { + state + .services + .events + .notify("Post-exit command failed") + .body(reason) + .error() + .send(); + } if dedicated { // The folder stays materialized so it remains a real Minecraft directory @@ -664,21 +680,25 @@ pub async fn offer_repair( } } -fn base_command(profile: &GameSettingsProfile, java_path: &str) -> Command { - if let Some(wrapper) = profile +fn base_command(profile: &GameSettingsProfile, java_path: &str) -> (Command, Option) { + let Some(wrapper) = profile .hook_wrapper .as_deref() .map(str::trim) .filter(|hook| !hook.is_empty()) - { - let mut split = wrapper.split_whitespace(); - let mut command = Command::new(split.next().unwrap_or("sh")); - command.args(split); - command.arg(java_path); - command - } else { - Command::new(java_path) - } + else { + return (Command::new(java_path), None); + }; + + let mut split = wrapper.split_whitespace(); + let Some(program) = split.next() else { + return (Command::new(java_path), None); + }; + + let mut command = Command::new(program); + command.args(split); + command.arg(java_path); + (command, Some(program.to_string())) } fn apply_env(command: &mut Command, profile: &GameSettingsProfile) { @@ -694,10 +714,8 @@ fn apply_env(command: &mut Command, profile: &GameSettingsProfile) { } #[tracing::instrument(skip(cwd), fields(hook), level = "debug")] -async fn run_hook(hook: Option<&str>, cwd: &Path) { - let Some(hook) = hook.map(str::trim).filter(|h| !h.is_empty()) else { - return; - }; +async fn run_hook(hook: Option<&str>, cwd: &Path) -> Option { + let hook = hook.map(str::trim).filter(|h| !h.is_empty())?; #[cfg(windows)] let mut command = { @@ -713,8 +731,81 @@ async fn run_hook(hook: Option<&str>, cwd: &Path) { }; command.current_dir(cwd); + command.stderr(Stdio::piped()); + command.stdin(Stdio::null()); oneclient_common::process::no_window(command.as_std_mut()); - if let Err(err) = command.status().await { - tracing::warn!("hook '{hook}' failed: {err}"); + + let mut child = match command.spawn() { + Ok(child) => child, + Err(err) => { + tracing::warn!("hook '{hook}' could not start: {err}"); + return Some(format!("The command shell could not be started: {err}")); + } + }; + + let collector = child.stderr.take().map(|mut pipe| { + tokio::spawn(async move { + let mut buf = Vec::new(); + let _ = pipe.read_to_end(&mut buf).await; + buf + }) + }); + + let status = match child.wait().await { + Ok(status) => status, + Err(err) => { + if let Some(collector) = collector { + collector.abort(); + } + tracing::warn!("hook '{hook}' could not be waited on: {err}"); + return Some(format!("The command could not be waited on: {err}")); + } + }; + + if status.success() { + if let Some(collector) = collector { + collector.abort(); + } + return None; } + + let detail = match collector { + Some(mut collector) => { + match tokio::time::timeout(Duration::from_millis(200), &mut collector).await { + Ok(Ok(buf)) => stderr_detail(&buf), + _ => { + collector.abort(); + None + } + } + } + None => None, + }; + + let message = match (detail, status.code()) { + (Some(detail), _) => detail, + (None, Some(code)) => format!("The command exited with code {code}"), + (None, None) => "The command was stopped before it finished".to_string(), + }; + + tracing::warn!("hook '{hook}' failed: {message}"); + Some(message) +} + +fn stderr_detail(bytes: &[u8]) -> Option { + let text = String::from_utf8_lossy(bytes); + let line = text + .lines() + .rev() + .map(str::trim) + .find(|line| !line.is_empty())?; + + const MAX: usize = 300; + if line.chars().count() <= MAX { + return Some(line.to_string()); + } + + let mut clipped: String = line.chars().take(MAX).collect(); + clipped.push('…'); + Some(clipped) }