cpjreynolds / rustty

A terminal UI library
https://docs.rs/rustty
MIT License
153 stars 14 forks source link

Errors are suppressed when a crash occurs #21

Closed Syntaf closed 8 years ago

Syntaf commented 8 years ago

It looks like errors aren't properly forwarded to be displayed at the end of the program. Any ideas on how to fix this?

Reproducible example

On the circle example here , change the type to a u8, 10u8. This will cause an arithmetic overflow if you increase the radius beyond 15. The exception is never displayed however and is lost in space, there is no way of knowing you're actually getting this exception beyond testing this without the use of rustty (which I had to do originally)

ghost commented 8 years ago

It's a tricky problem because our Terminal flushes the screen on Drop (see https://github.com/cpjreynolds/rustty/blob/504145d564d2593960cf80094d5ca2c3d8755df4/src/core/terminal.rs#L675 ), which happens at every panic.

When I debug, I comment out the self.flush().unwrap(); line so I can see panic messages.

There's two solutions I see:

  1. Find a way to catch panics (I don't know Rust enough to know if it's possible) and skip flushing based on a debug flag we could set on the terminal.
  2. Don't implement Drop and require the user of the library to clean after herself manually.
Syntaf commented 8 years ago

I just found thread::panicking , which can detect if a panic is occurring. I changed flush inside terminal.rs to

fn flush(&mut self) -> Result<(), Error> {
    if thread::panicking() {
        Ok(())
    } else {
        try!(self.tty.write_all(&self.outbuffer));
        self.outbuffer.clear();
        Ok(())
    }
}

If an exception occurs, the terminal would break and not properly clear the buffer. If an exception does not occur, everything will continue as normal.

I don't want to create a PR for this because I think there's still a much more elegant solution, some way to clear the screen while keeping that exception there.

Side note: have you thought of enabling labels for the issue tracker @cpjreynolds? , it would be useful for classifying issues related to portability, bugs, and improvements

Syntaf commented 8 years ago

Found an interesting function here . Although it is hidden and unstable it could possibly be used to redirect sterr to a buffer that can then be written to the screen at the end of drop? I'm not sure if it is nightly only however, I'll need to check that. Even then, unstable and hidden isn't exactly the nicest thing

EDIT: It's nightly only, RIP. back to square 1

cpjreynolds commented 8 years ago

Closing as #22 appears to have fixed the issue.