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 <aram@poor.dev>

8 files changed, 106 insertions, 23 deletions

diff --git a/src/errors.rs b/src/errors.rs
new file mode 100644
index 00000000..26e0010d
--- /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<AppInstruction>) {
+    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::<String>().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 00ed0725..8c7bd88d 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<ScreenInstruction>,
     send_pty_instructions: Sender<PtyInstruction>,
-    send_app_instructions: Sender<AppInstruction>,
+    send_app_instructions: SyncSender<AppInstruction>,
 }
 
 impl InputHandler {
@@ -22,7 +22,7 @@ impl InputHandler {
         command_is_executing: CommandIsExecuting,
         send_screen_instructions: Sender<ScreenInstruction>,
         send_pty_instructions: Sender<PtyInstruction>,
-        send_app_instructions: Sender<AppInstruction>,
+        send_app_instructions: SyncSender<AppInstruction>,
     ) -> Self {
         InputHandler {
             mode: InputMode::Normal,
@@ -241,7 +241,7 @@ pub fn input_loop(
     command_is_executing: CommandIsExecuting,
     send_screen_instructions: Sender<ScreenInstruction>,
     send_pty_instructions: Sender<PtyInstruction>,
-    send_app_instructions: Sender<AppInstruction>,
+    send_app_instructions: SyncSender<AppInstruction>,
 ) {
     let _handler = InputHandler::new(
         os_input,
diff --git a/src/main.rs b/src/main.rs
index 08bc37c3..ab40805f 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<dyn OsApi>, opts: Opt) {
@@ -113,9 +115,9 @@ pub fn start(mut os_input: Box<dyn OsApi>, opts: Opt) {
         Receiver<PtyInstruction>,
     ) = channel();
     let (send_app_instructions, receive_app_instructions): (
-        Sender<AppInstruction>,
+        SyncSender<AppInstruction>,
         Receiver<AppInstruction>,
-    ) = 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<dyn OsApi>, 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<dyn OsApi>, 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<dyn OsApi>, 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 56bb82d2..5e52b4d0 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<ScreenInstruction>,
     max_panes: Option<usize>,
     send_pty_instructions: Sender<PtyInstruction>,
-    send_app_instructions: Sender<AppInstruction>,
+    send_app_instructions: SyncSender<AppInstruction>,
     full_screen_ws: PositionAndSize,
     terminals: BTreeMap<RawFd, TerminalPane>, // BTreeMap because we need a predictable order when changing focus
     panes_to_hide: HashSet<RawFd>,
@@ -88,7 +88,7 @@ impl Screen {
     pub fn new(
         receive_screen_instructions: Receiver<ScreenInstruction>,
         send_pty_instructions: Sender<PtyInstruction>,
-        send_app_instructions: Sender<AppInstruction>,
+        send_app_instructions: SyncSender<AppInstruction>,
         full_screen_ws: &PositionAndSize,
         os_api: Box<dyn OsApi>,
         max_panes: Option<usize>,
diff --git a/src/terminal_pane/scroll.rs b/src/terminal_pane/scroll.rs
index bfd6bd31..d66c022e 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 c17d03fc..f20f30ce 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 efc7d8b3..e65cf01e 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 7f5bd742..a0966383 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;