Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

zellij-server: improve thread_bus error handling #1775

Merged
merged 3 commits into from
Oct 7, 2022
Merged

zellij-server: improve thread_bus error handling #1775

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4f985c9
zellij-server: improve thread_bus error handling
arriven Oct 4, 2022
5382dde
zellij-server/thread_bus: get rid of option.unwrap
arriven Oct 6, 2022
f30b618
zellij-utils/errors.rs: generic error in to_anyhow
arriven Oct 6, 2022
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
3 changes: 0 additions & 3 deletions zellij-server/src/screen.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -971,7 +971,6 @@ impl Screen {
Some(*client_id),
Event::TabUpdate(tab_data),
))
.to_anyhow()
.context("failed to update tabs")?;
}
Ok(())
Expand Down Expand Up @@ -1167,7 +1166,6 @@ impl Screen {
self.bus
.senders
.send_to_server(ServerInstruction::UnblockInputThread)
.context("failed to send message to server")
.context("failed to unblock input")
}
}
Expand Down Expand Up @@ -1871,7 +1869,6 @@ pub(crate) fn screen_thread_main(
.bus
.senders
.send_to_server(*instruction)
.context("failed to send message to server")
.context("failed to confirm prompt")?;
}
screen.unblock_input()?;
Expand Down
6 changes: 0 additions & 6 deletions zellij-server/src/tab/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,6 @@ impl Tab {
let pane_title = run.location.to_string();
self.senders
.send_to_plugin(PluginInstruction::Load(pid_tx, run, tab_index, client_id))
.to_anyhow()
.with_context(err_context)?;
let pid = pid_rx.recv().with_context(err_context)?;
let mut new_plugin = PluginPane::new(
Expand Down Expand Up @@ -588,7 +587,6 @@ impl Tab {
Some(*client_id),
Event::ModeUpdate(mode_info.clone()),
))
.to_anyhow()
.with_context(|| {
format!(
"failed to update plugins with mode info {:?}",
Expand Down Expand Up @@ -1169,7 +1167,6 @@ impl Tab {
for key in parse_keys(&input_bytes) {
self.senders
.send_to_plugin(PluginInstruction::Update(Some(pid), None, Event::Key(key)))
.to_anyhow()
.with_context(err_context)?;
}
},
Expand Down Expand Up @@ -2390,7 +2387,6 @@ impl Tab {
None,
Event::CopyToClipboard(self.clipboard_provider.as_copy_destination()),
))
.to_anyhow()
.with_context(|| {
format!("failed to inform plugins about copy selection for client {client_id}")
})
Expand Down Expand Up @@ -2424,7 +2420,6 @@ impl Tab {
};
self.senders
.send_to_plugin(PluginInstruction::Update(None, None, clipboard_event))
.to_anyhow()
.context("failed to notify plugins about new clipboard event")
.non_fatal();

Expand Down Expand Up @@ -2466,7 +2461,6 @@ impl Tab {
None,
Event::Visible(visible),
))
.to_anyhow()
.with_context(|| format!("failed to set visibility of tab to {visible}"))?;
}
Ok(())
Expand Down
61 changes: 36 additions & 25 deletions zellij-server/src/thread_bus.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use crate::{
os_input_output::ServerOsApi, pty::PtyInstruction, pty_writer::PtyWriteInstruction,
screen::ScreenInstruction, wasm_vm::PluginInstruction, ServerInstruction,
};
use zellij_utils::errors::prelude::*;
use zellij_utils::{channels, channels::SenderWithContext, errors::ErrorContext};

/// A container for senders to the different threads in zellij on the server side
Expand All @@ -20,10 +21,7 @@ pub(crate) struct ThreadSenders {
}

impl ThreadSenders {
pub fn send_to_screen(
&self,
instruction: ScreenInstruction,
) -> Result<(), channels::SendError<(ScreenInstruction, ErrorContext)>> {
pub fn send_to_screen(&self, instruction: ScreenInstruction) -> Result<()> {
if self.should_silently_fail {
let _ = self
.to_screen
Expand All @@ -32,14 +30,16 @@ impl ThreadSenders {
.unwrap_or_else(|| Ok(()));
Ok(())
} else {
self.to_screen.as_ref().unwrap().send(instruction)
self.to_screen
.as_ref()
.context("failed to get screen sender")?
.send(instruction)
.to_anyhow()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, initially I implemented it as a crutch for the changes in tab, as you noticed correctly. Thank you for removing it in tab!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, it refused to compile otherwise since anyhow::Result<T> != Result<T, crossbeam::channel::SendError> but thanks! 😅

.context("failed to send message to screen")
}
}

pub fn send_to_pty(
&self,
instruction: PtyInstruction,
) -> Result<(), channels::SendError<(PtyInstruction, ErrorContext)>> {
pub fn send_to_pty(&self, instruction: PtyInstruction) -> Result<()> {
if self.should_silently_fail {
let _ = self
.to_pty
Expand All @@ -48,14 +48,16 @@ impl ThreadSenders {
.unwrap_or_else(|| Ok(()));
Ok(())
} else {
self.to_pty.as_ref().unwrap().send(instruction)
self.to_pty
.as_ref()
.context("failed to get pty sender")?
.send(instruction)
.to_anyhow()
.context("failed to send message to pty")
}
}

pub fn send_to_plugin(
&self,
instruction: PluginInstruction,
) -> Result<(), channels::SendError<(PluginInstruction, ErrorContext)>> {
pub fn send_to_plugin(&self, instruction: PluginInstruction) -> Result<()> {
if self.should_silently_fail {
let _ = self
.to_plugin
Expand All @@ -64,14 +66,16 @@ impl ThreadSenders {
.unwrap_or_else(|| Ok(()));
Ok(())
} else {
self.to_plugin.as_ref().unwrap().send(instruction)
self.to_plugin
.as_ref()
.context("failed to get plugin sender")?
.send(instruction)
.to_anyhow()
.context("failed to send message to plugin")
}
}

pub fn send_to_server(
&self,
instruction: ServerInstruction,
) -> Result<(), channels::SendError<(ServerInstruction, ErrorContext)>> {
pub fn send_to_server(&self, instruction: ServerInstruction) -> Result<()> {
if self.should_silently_fail {
let _ = self
.to_server
Expand All @@ -80,13 +84,15 @@ impl ThreadSenders {
.unwrap_or_else(|| Ok(()));
Ok(())
} else {
self.to_server.as_ref().unwrap().send(instruction)
self.to_server
.as_ref()
.context("failed to get server sender")?
.send(instruction)
.to_anyhow()
.context("failed to send message to server")
}
}
pub fn send_to_pty_writer(
&self,
instruction: PtyWriteInstruction,
) -> Result<(), channels::SendError<(PtyWriteInstruction, ErrorContext)>> {
pub fn send_to_pty_writer(&self, instruction: PtyWriteInstruction) -> Result<()> {
if self.should_silently_fail {
let _ = self
.to_pty_writer
Expand All @@ -95,7 +101,12 @@ impl ThreadSenders {
.unwrap_or_else(|| Ok(()));
Ok(())
} else {
self.to_pty_writer.as_ref().unwrap().send(instruction)
self.to_pty_writer
.as_ref()
.context("failed to get pty writer sender")?
.send(instruction)
.to_anyhow()
.context("failed to send message to pty writer")
}
}

Expand Down
2 changes: 1 addition & 1 deletion zellij-utils/src/errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,7 @@ mod not_wasm {
Err(e) => {
let (msg, context) = e.into_inner();
Err(
crate::anyhow::anyhow!("failed to send message to client: {:#?}", msg)
crate::anyhow::anyhow!("failed to send message to channel: {:#?}", msg)
.context(context.to_string()),
)
},
Expand Down