Tracking issue to improve error handling

[har7an]

The goal

We are looking to improve the general state of error handling in zellij. As the zellij code-base changed, a lot of the previous places where unwrap made sense can now potentially cause errors which we'd like to handle. We want to eliminate calls to unwrap and use Result types instead, so we can handle errors where appropriate.

You can read more about the background and the tools we have to achieve this in the docs.

Help wanted

The zellij codebase has become very large, and few functions use Result types to propagate errors up to calling functions. We are looking for contributors to help us out with successively converting the code to change this.

If you want to make a contribution, this may be the ideal start for you. While working on error handling you:

• get familiar with different parts of the code, without having to integrate new features while you're at it,
• get a feeling for how individual portions of the code interact and
• help users and developers alike by
  • making more sense of application crashes, or
  • preventing crashes from occurring in the first place

Contributing

You want to help us out with error handling? That's great!

You can get going right away. Keep in mind while working on this that it's very important to understand the piece of code you are changing. A good way to help you understand what is happening would be to try to run the application, reach that place in the code and see what happens. You can also try to manually generate errors in different places (for example with the anyhow::bail! macro) to see if you're on the right track.

In order to help us coordinate efforts, we would ask you to indicate that you're working on some aspect of error handling by either:

• leaving us a comment here in this issue, or
• opening a Pull Request ahead of time and mentioning this issue from there.

This allows us to keep the list below up-to-date, and prevents you from working on something that another person has picked up before you.

If you want to help us, but aren't sure where to start or what to do, you can either:

• leave a comment in this issue, or
• reach out to us via Discord or Matrix.

We'll help you find a suitable topic to work on and guide you as questions arise.

Thanks in advance!

List of efforts regarding error handling

• Server: Remove panics in tab module #1748
• zellij-server/src/screen: improve error handling #1770
• zellij-server: improve thread_bus error handling #1775
• improve error handling in route #1808
• errors: Don't panic in wasm_vm #1827
• plugins: Improve error handling on plugin version mismatch #1838
• improve error handling in pty #1840
• improve error handling in ui module #1870
• errors: Don't unwrap in server::ui::boundaries #1871
• errors: Don't unwrap in server::pty_writer #1872
• errors: Don't unwrap in server::terminal_bytes #1876
• errors: Don't unwrap in zellij_server::output #1878
• errors: Remove log::error in server #1881
• errors: Don't unwrap in server::os_input_output #1895
• Improve client disconnect handling #2068
• Errors: less unwrap in server #2069
• errors: handle missing plugin caches #2093

[naosense]

I want to work on this, any hints or suggestions where I can start.

[har7an]

Hi @naosense,

glad to hear it! Here's a bunch of suggestions (along with the number of calls to unwrap()):

• zellij-server/src/route.rs (77): This one should be mostly self-contained I think (Doesn't require lots of changes in other files across the codebase)
• zellij-server/src/pty.rs (33): This may require changes in other files
• zellij-server/src/panes/grid.rs (27): This one will require many changes in other files

You can pick whichever suits you most. You can also look for other files/modules to work on. Personally I checkout the latest main, go into the zellij submodule that interests me the most (We're currently focusing on zellij_server, but feel free to pick what suits you best), and do something like:

$ rg -c "\.unwrap\(" -g '*.rs' -g '!*tests*'

to get an overview of how many calls to unwrap() there are in general. From there on you can pick almost any file you like. If you need further help, I suggest you open a pull request and we continue the discussion there.

[naosense]

ok, I'll try it

[alaricljs]

I'd like to call out an example of how error handling could be improved that is easily reproduced.

I recently messed with my filesystem with specific goals in mind and a side effect that I missed was my user's .cache directory getting owned by root. Everything I previously used could still function since those .cache artifacts already existed and were still owned by me. zellij was the first new thing I tried since this mistake.

When starting zellij for the first time it crashed with a pretty verbose file not found and permission denied error but no specifics as to the file. I then tried the recommended RUST env var to get more info, but the backtrace was unusable as it was all <unknown> entries.

The only reason I found /tmp/zellij-1000/zellig-log was because I also tried strace against zellij. That log is the only place I got usable info to solve my problem as it specifically stated failed to create datadir in ".../.cache/...". So the quick and easy improvement is to mention the log file in the error messages. The other improvement would be getting the same information that's in the log file into the error message that's displayed to the user.