diff --git a/CHANGELOG.md b/CHANGELOG.md index e24865d3a4..eaf68ba426 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) * errors: Remove more `unwrwap`s from server code (https://github.com/zellij-org/zellij/pull/2069) * fix: support UTF-8 character in tab name and pane name (https://github.com/zellij-org/zellij/pull/2102) * fix: handle missing/inaccessible cache directory (https://github.com/zellij-org/zellij/pull/2093) +* errors: Improve client disconnect handling (https://github.com/zellij-org/zellij/pull/2068) ## [0.34.4] - 2022-12-13 diff --git a/xtask/src/pipelines.rs b/xtask/src/pipelines.rs index 600d1b60d6..b2266a18f8 100644 --- a/xtask/src/pipelines.rs +++ b/xtask/src/pipelines.rs @@ -104,6 +104,7 @@ pub fn run(sh: &Shell, flags: flags::Run) -> anyhow::Result<()> { .arg("--no-default-features") .args(["--features", "disable_automatic_asset_installation"]) .args(["--", "--data-dir", &format!("{}", data_dir.display())]) + .args(&flags.args) .run() .map_err(anyhow::Error::new) }) diff --git a/zellij-server/src/lib.rs b/zellij-server/src/lib.rs index bae871f441..ac1b268d47 100644 --- a/zellij-server/src/lib.rs +++ b/zellij-server/src/lib.rs @@ -42,7 +42,7 @@ use zellij_utils::{ cli::CliArgs, consts::{DEFAULT_SCROLL_BUFFER_SIZE, SCROLL_BUFFER_SIZE}, data::{Event, PluginCapabilities}, - errors::{ContextType, ErrorInstruction, FatalError, ServerContext}, + errors::{prelude::*, ContextType, ErrorInstruction, FatalError, ServerContext}, input::{ command::{RunCommand, TerminalAction}, get_mode_info, @@ -150,7 +150,21 @@ macro_rules! remove_client { macro_rules! send_to_client { ($client_id:expr, $os_input:expr, $msg:expr, $session_state:expr) => { let send_to_client_res = $os_input.send_to_client($client_id, $msg); - if let Err(_) = send_to_client_res { + if let Err(e) = send_to_client_res { + // Try to recover the message + let context = match e.downcast_ref::() { + Some(ZellijError::ClientTooSlow { .. }) => { + format!( + "client {} is processing server messages too slow", + $client_id + ) + }, + _ => { + format!("failed to route server message to client {}", $client_id) + }, + }; + // Log it so it isn't lost + Err::<(), _>(e).context(context).non_fatal(); // failed to send to client, remove it remove_client!($client_id, $os_input, $session_state); } @@ -552,6 +566,9 @@ pub fn start_server(mut os_input: Box, socket_path: PathBuf) { // If `None`- Send an exit instruction. This is the case when a user closes the last Tab/Pane. if let Some(output) = &serialized_output { for (client_id, client_render_instruction) in output.iter() { + // TODO: When a client is too slow or unresponsive, the channel fills up + // and this call will disconnect the client in turn. Should this be + // changed? send_to_client!( *client_id, os_input, diff --git a/zellij-server/src/os_input_output.rs b/zellij-server/src/os_input_output.rs index 07bd7c0f42..9c214b7052 100644 --- a/zellij-server/src/os_input_output.rs +++ b/zellij-server/src/os_input_output.rs @@ -14,6 +14,7 @@ use signal_hook::consts::*; use sysinfo::{ProcessExt, ProcessRefreshKind, System, SystemExt}; use zellij_utils::{ async_std, channels, + channels::TrySendError, data::Palette, errors::prelude::*, input::command::{RunCommand, TerminalAction}, @@ -343,15 +344,14 @@ struct ClientSender { impl ClientSender { pub fn new(client_id: ClientId, mut sender: IpcSenderWithContext) -> Self { - let (client_buffer_sender, client_buffer_receiver) = channels::bounded(50); + let (client_buffer_sender, client_buffer_receiver) = channels::bounded(5000); std::thread::spawn(move || { let err_context = || format!("failed to send message to client {client_id}"); for msg in client_buffer_receiver.iter() { - let _ = sender.send(msg).with_context(err_context); + sender.send(msg).with_context(err_context).non_fatal(); } - let _ = sender.send(ServerToClientMsg::Exit(ExitReason::Error( - "Buffer full".to_string(), - ))); + // If we're here, the message buffer is broken for some reason + let _ = sender.send(ServerToClientMsg::Exit(ExitReason::Disconnect)); }); ClientSender { client_id, @@ -359,9 +359,24 @@ impl ClientSender { } } pub fn send_or_buffer(&self, msg: ServerToClientMsg) -> Result<()> { - let err_context = || format!("Client {} send buffer full", self.client_id); + let err_context = || { + format!( + "failed to send or buffer message for client {}", + self.client_id + ) + }; + self.client_buffer_sender .try_send(msg) + .or_else(|err| { + if let TrySendError::Full(_) = err { + log::warn!( + "client {} is processing server messages too slow", + self.client_id + ); + } + Err(err) + }) .with_context(err_context) } } diff --git a/zellij-utils/src/channels.rs b/zellij-utils/src/channels.rs index 26a2716710..4fdbc8bcae 100644 --- a/zellij-utils/src/channels.rs +++ b/zellij-utils/src/channels.rs @@ -4,7 +4,9 @@ use async_std::task_local; use std::cell::RefCell; use crate::errors::{get_current_ctx, ErrorContext}; -pub use crossbeam::channel::{bounded, unbounded, Receiver, RecvError, Select, SendError, Sender}; +pub use crossbeam::channel::{ + bounded, unbounded, Receiver, RecvError, Select, SendError, Sender, TrySendError, +}; /// An [MPSC](mpsc) asynchronous channel with added error context. pub type ChannelWithContext = (Sender<(T, ErrorContext)>, Receiver<(T, ErrorContext)>); diff --git a/zellij-utils/src/errors.rs b/zellij-utils/src/errors.rs index 7fbed99ed4..b56bd79a16 100644 --- a/zellij-utils/src/errors.rs +++ b/zellij-utils/src/errors.rs @@ -469,6 +469,9 @@ open an issue on GitHub: #[error("an error occured")] GenericError { source: anyhow::Error }, + + #[error("Client {client_id} is too slow to handle incoming messages")] + ClientTooSlow { client_id: u16 }, } #[cfg(not(target_family = "wasm"))] diff --git a/zellij-utils/src/ipc.rs b/zellij-utils/src/ipc.rs index b264f48e52..81d9e6c896 100644 --- a/zellij-utils/src/ipc.rs +++ b/zellij-utils/src/ipc.rs @@ -117,6 +117,7 @@ pub enum ExitReason { NormalDetached, ForceDetached, CannotAttach, + Disconnect, Error(String), } @@ -133,6 +134,31 @@ impl Display for ExitReason { f, "Session attached to another client. Use --force flag to force connect." ), + Self::Disconnect => { + let session_tip = match crate::envs::get_session_name() { + Ok(name) => format!("`zellij attach {}`", name), + Err(_) => "see `zellij ls` and `zellij attach`".to_string(), + }; + write!( + f, + " +Your zellij client lost connection to the zellij server. + +As a safety measure, you have been disconnected from the current zellij session. +However, the session should still exist and none of your data should be lost. + +This usually means that your terminal didn't process server messages quick +enough. Maybe your system is currently under high load, or your terminal +isn't performant enough. + +There are a few things you can try now: + - Reattach to your previous session and see if it works out better this + time: {session_tip} + - Try using a faster terminal. GPU-accelerated terminals such as Kitty + or Alacritty are cross-platform and known to work well with zellij. + " + ) + }, Self::Error(e) => write!(f, "Error occurred in server:\n{}", e), } }