From 56c53d24870d7470a6aa336788093860ea05c9e7 Mon Sep 17 00:00:00 2001 From: Kunal Mohan <44079328+kunalmohan@users.noreply.github.com> Date: Thu, 3 Dec 2020 20:05:16 +0530 Subject: [PATCH] feat(infra): add custom panic hook. Print backtrace and thread, error info on panic. (#75) * Add custom panic hook. Print backtrace and thread, error info on panic. * use sync_channel and SyncSender * nit fixes and cleanup * disable custom panic hook while running tests * make separate errors.rs file and address other review comments * improve panic message * debug: does increasing time between snapshots make tests pass? (this is temporary) * fix(tests): suspend before sending quit command * fix(tests): add missing use * style(format): commas are important apparently * fix(tests): can we get away with reducing the QUIT suspense time? * fix(tests): can we get away with 50? Co-authored-by: Aram Drevekenin --- Cargo.lock | 1 + Cargo.toml | 19 +++++----- src/errors.rs | 56 ++++++++++++++++++++++++++++ src/input.rs | 8 ++-- src/main.rs | 28 ++++++++++++-- src/screen.rs | 6 +-- src/terminal_pane/scroll.rs | 4 +- src/tests/fakes.rs | 23 ++++++++---- src/tests/integration/layouts.rs | 2 +- src/tests/integration/resize_down.rs | 2 +- 10 files changed, 117 insertions(+), 32 deletions(-) create mode 100644 src/errors.rs diff --git a/Cargo.lock b/Cargo.lock index d05c5a32e..3c7bf2c6d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -862,6 +862,7 @@ name = "mosaic" version = "0.1.0" dependencies = [ "async-std", + "backtrace", "bincode", "futures", "insta", diff --git a/Cargo.toml b/Cargo.toml index 72d828cd8..41dd31753 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,19 +7,20 @@ edition = "2018" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] -termios = "0.3" +backtrace = "0.3.55" +bincode = "1.3.1" +futures = "0.3.5" libc = "0.2" nix = "0.17.0" -signal-hook = "0.1.10" -unicode-width = "0.1.8" -unicode-truncate = "0.1.1" -vte = "0.8.0" -futures = "0.3.5" serde = { version = "1.0", features = ["derive"] } -bincode = "1.3.1" -structopt = "0.3" -serde_yaml = "0.8" serde_json = "1.0" +serde_yaml = "0.8" +signal-hook = "0.1.10" +structopt = "0.3" +termios = "0.3" +unicode-truncate = "0.1.1" +unicode-width = "0.1.8" +vte = "0.8.0" [dependencies.async-std] version = "1.3.0" diff --git a/src/errors.rs b/src/errors.rs new file mode 100644 index 000000000..26e0010dc --- /dev/null +++ b/src/errors.rs @@ -0,0 +1,56 @@ +use crate::AppInstruction; +use backtrace::Backtrace; +use std::panic::PanicInfo; +use std::sync::mpsc::SyncSender; +use std::{process, thread}; + +pub fn handle_panic(info: &PanicInfo<'_>, send_app_instructions: &SyncSender) { + let backtrace = Backtrace::new(); + let thread = thread::current(); + let thread = thread.name().unwrap_or("unnamed"); + + let msg = match info.payload().downcast_ref::<&'static str>() { + Some(s) => Some(*s), + None => info.payload().downcast_ref::().map(|s| &**s), + }; + + let backtrace = match (info.location(), msg) { + (Some(location), Some(msg)) => { + format!( + "\nthread '{}' panicked at '{}': {}:{}\n{:?}", + thread, + msg, + location.file(), + location.line(), + backtrace + ) + } + (Some(location), None) => { + format!( + "\nthread '{}' panicked: {}:{}\n{:?}", + thread, + location.file(), + location.line(), + backtrace + ) + } + (None, Some(msg)) => { + format!( + "\nthread '{}' panicked at '{}'\n{:?}", + thread, msg, backtrace + ) + } + (None, None) => { + format!("\nthread '{}' panicked\n{:?}", thread, backtrace) + } + }; + + if thread == "main" { + println!("{}", backtrace); + process::exit(1); + } else { + send_app_instructions + .send(AppInstruction::Error(backtrace)) + .unwrap(); + } +} diff --git a/src/input.rs b/src/input.rs index 00ed0725b..8c7bd88d1 100644 --- a/src/input.rs +++ b/src/input.rs @@ -1,5 +1,5 @@ /// Module for handling input -use std::sync::mpsc::Sender; +use std::sync::mpsc::{Sender, SyncSender}; use crate::os_input_output::OsApi; use crate::pty_bus::PtyInstruction; @@ -13,7 +13,7 @@ struct InputHandler { command_is_executing: CommandIsExecuting, send_screen_instructions: Sender, send_pty_instructions: Sender, - send_app_instructions: Sender, + send_app_instructions: SyncSender, } impl InputHandler { @@ -22,7 +22,7 @@ impl InputHandler { command_is_executing: CommandIsExecuting, send_screen_instructions: Sender, send_pty_instructions: Sender, - send_app_instructions: Sender, + send_app_instructions: SyncSender, ) -> Self { InputHandler { mode: InputMode::Normal, @@ -241,7 +241,7 @@ pub fn input_loop( command_is_executing: CommandIsExecuting, send_screen_instructions: Sender, send_pty_instructions: Sender, - send_app_instructions: Sender, + send_app_instructions: SyncSender, ) { let _handler = InputHandler::new( os_input, diff --git a/src/main.rs b/src/main.rs index 08bc37c32..ab40805f5 100644 --- a/src/main.rs +++ b/src/main.rs @@ -3,6 +3,7 @@ mod tests; mod boundaries; mod command_is_executing; +mod errors; mod input; mod layout; mod os_input_output; @@ -14,7 +15,7 @@ mod utils; use std::io::Write; use std::os::unix::net::UnixStream; use std::path::PathBuf; -use std::sync::mpsc::{channel, Receiver, Sender}; +use std::sync::mpsc::{channel, sync_channel, Receiver, Sender, SyncSender}; use std::thread; use serde::{Deserialize, Serialize}; @@ -95,6 +96,7 @@ pub fn main() { pub enum AppInstruction { Exit, + Error(String), } pub fn start(mut os_input: Box, opts: Opt) { @@ -113,9 +115,9 @@ pub fn start(mut os_input: Box, opts: Opt) { Receiver, ) = channel(); let (send_app_instructions, receive_app_instructions): ( - Sender, + SyncSender, Receiver, - ) = channel(); + ) = sync_channel(0); let mut screen = Screen::new( receive_screen_instructions, send_pty_instructions.clone(), @@ -132,6 +134,15 @@ pub fn start(mut os_input: Box, opts: Opt) { ); let maybe_layout = opts.layout.map(Layout::new); + #[cfg(not(test))] + std::panic::set_hook({ + use crate::errors::handle_panic; + let send_app_instructions = send_app_instructions.clone(); + Box::new(move |info| { + handle_panic(info, &send_app_instructions); + }) + }); + active_threads.push( thread::Builder::new() .name("pty".to_string()) @@ -419,7 +430,6 @@ pub fn start(mut os_input: Box, opts: Opt) { .spawn({ let send_screen_instructions = send_screen_instructions.clone(); let send_pty_instructions = send_pty_instructions.clone(); - let send_app_instructions = send_app_instructions.clone(); let os_input = os_input.clone(); move || { input_loop( @@ -443,6 +453,16 @@ pub fn start(mut os_input: Box, opts: Opt) { let _ = send_pty_instructions.send(PtyInstruction::Quit); break; } + AppInstruction::Error(backtrace) => { + os_input.unset_raw_mode(0); + println!("{}", backtrace); + let _ = send_screen_instructions.send(ScreenInstruction::Quit); + let _ = send_pty_instructions.send(PtyInstruction::Quit); + for thread_handler in active_threads { + let _ = thread_handler.join(); + } + std::process::exit(1); + } } } diff --git a/src/screen.rs b/src/screen.rs index 56bb82d27..5e52b4d05 100644 --- a/src/screen.rs +++ b/src/screen.rs @@ -1,7 +1,7 @@ use std::collections::{BTreeMap, HashSet}; use std::io::Write; use std::os::unix::io::RawFd; -use std::sync::mpsc::{Receiver, Sender}; +use std::sync::mpsc::{Receiver, Sender, SyncSender}; use crate::boundaries::Boundaries; use crate::layout::Layout; @@ -75,7 +75,7 @@ pub struct Screen { pub receiver: Receiver, max_panes: Option, send_pty_instructions: Sender, - send_app_instructions: Sender, + send_app_instructions: SyncSender, full_screen_ws: PositionAndSize, terminals: BTreeMap, // BTreeMap because we need a predictable order when changing focus panes_to_hide: HashSet, @@ -88,7 +88,7 @@ impl Screen { pub fn new( receive_screen_instructions: Receiver, send_pty_instructions: Sender, - send_app_instructions: Sender, + send_app_instructions: SyncSender, full_screen_ws: &PositionAndSize, os_api: Box, max_panes: Option, diff --git a/src/terminal_pane/scroll.rs b/src/terminal_pane/scroll.rs index bfd6bd311..d66c022e7 100644 --- a/src/terminal_pane/scroll.rs +++ b/src/terminal_pane/scroll.rs @@ -1,5 +1,5 @@ -use ::std::collections::VecDeque; -use ::std::fmt::{self, Debug, Formatter}; +use std::collections::VecDeque; +use std::fmt::{self, Debug, Formatter}; use crate::terminal_pane::terminal_character::{ CharacterStyles, TerminalCharacter, EMPTY_TERMINAL_CHARACTER, diff --git a/src/tests/fakes.rs b/src/tests/fakes.rs index c17d03fc4..f20f30ce2 100644 --- a/src/tests/fakes.rs +++ b/src/tests/fakes.rs @@ -1,16 +1,18 @@ use crate::terminal_pane::PositionAndSize; -use ::std::collections::{HashMap, VecDeque}; -use ::std::io::Write; -use ::std::os::unix::io::RawFd; -use ::std::path::PathBuf; -use ::std::sync::atomic::{AtomicBool, Ordering}; -use ::std::sync::{Arc, Mutex}; -use ::std::time::{Duration, Instant}; +use std::collections::{HashMap, VecDeque}; +use std::io::Write; +use std::os::unix::io::RawFd; +use std::path::PathBuf; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::{Arc, Mutex}; +use std::time::{Duration, Instant}; use crate::os_input_output::OsApi; use crate::tests::possible_tty_inputs::{get_possible_tty_inputs, Bytes}; +use crate::tests::utils::commands::QUIT; const MIN_TIME_BETWEEN_SNAPSHOTS: Duration = Duration::from_millis(50); +const WAIT_TIME_BEFORE_QUITTING: Duration = Duration::from_millis(50); #[derive(Clone)] pub enum IoEvent { @@ -207,7 +209,12 @@ impl OsApi for FakeInputOutput { } } match self.stdin_commands.lock().unwrap().pop_front() { - Some(command) => command, + Some(command) => { + if command == QUIT { + std::thread::sleep(WAIT_TIME_BEFORE_QUITTING); + } + command + } None => { // what is happening here? // diff --git a/src/tests/integration/layouts.rs b/src/tests/integration/layouts.rs index efc7d8b36..e65cf01e0 100644 --- a/src/tests/integration/layouts.rs +++ b/src/tests/integration/layouts.rs @@ -1,4 +1,4 @@ -use ::insta::assert_snapshot; +use insta::assert_snapshot; use std::path::PathBuf; use crate::terminal_pane::PositionAndSize; diff --git a/src/tests/integration/resize_down.rs b/src/tests/integration/resize_down.rs index 7f5bd7427..a09663835 100644 --- a/src/tests/integration/resize_down.rs +++ b/src/tests/integration/resize_down.rs @@ -1,4 +1,4 @@ -use ::insta::assert_snapshot; +use insta::assert_snapshot; use crate::terminal_pane::PositionAndSize; use crate::tests::fakes::FakeInputOutput;