docs: Describe how to handle Options as errors (#1805)

* docs: Describe how to handle Options as errors

* CONTRIBUTING: Add tips for code contributions

which will be home to condensed tips and best-practices around the
zellij code. Currently explains to prefer returning `Result` types
instead of `unwrap`ing on them.

The tips in here are meant to be short, concise guides that allow the
user to get started without a lot of reading. The individual tips can
(and should) be supplemented with links to "further reading" where the
topic at hand is explained in greater detail.
This commit is contained in:
har7an 2022-10-18 14:14:38 +00:00 committed by GitHub
parent 12d35bded5
commit 0711591ad3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 59 additions and 0 deletions

View File

@ -113,6 +113,21 @@ something interesting to work on and guide through.
[discord-invite-link]: https://discord.gg/feHDHahHCz
[good-first-issue]: https://github.com/zellij-org/zellij/labels/good%20first%20issue
## Tips for Code Contributions
### Prefer returning `Result`s instead of `unwrap`ing
- Add `use zellij_utils::errors::prelude::*;` to the file
- Make the function return `Result<T>`, with an appropriate `T` (Use `()` if there's nothing to return)
- Append `.context()` to any `Result` you get with a sensible error description (see [the docs][error-docs-context])
- Generate ad-hoc errors with `anyhow!(<SOME MESSAGE>)`
- *Further reading*: [See here][error-docs-result]
[error-docs-context]: https://github.com/zellij-org/zellij/blob/main/docs/ERROR_HANDLING.md#attaching-context
[error-docs-result]: https://github.com/zellij-org/zellij/blob/main/docs/ERROR_HANDLING.md#converting-a-function-to-return-a-result-type
## Filing Issues
Bugs and enhancement suggestions are tracked as GitHub issues.

View File

@ -101,6 +101,12 @@ the following text.
## Converting a function to return a `Result` type
> **TL;DR**
> - Add `use zellij_utils::errors::prelude::*;` to the file
> - Make the function return `Result<T>`, with an appropriate `T` (Often `()`)
> - Append `.context()` to any `Result` you get with a sensible error description (see below)
> - Generate ad-hoc errors with `anyhow!(<SOME MESSAGE>)`
Here's an example of the `Screen::render` function as it looked before:
```rust
@ -321,6 +327,44 @@ the application (i.e. crash zellij). Since we made sure to attach context
messages to the errors on their way up, we will see these messages in the
resulting output!
## Error handling for `Option` types
Beyond what's described in "Choosing helpful context messages" above, `Option`
types benefit from extra handling. That is because a `Option` containing a
`None` where a value is expected doesn't carry an error message: It doesn't
tell us why the `None` is bad (i.e. equivalent to an Error) in this case.
In situations where a call to `unwrap()` or similar on a `Option` type is to be
converted for error handling, it is a good idea to attach an additional short
context. An example from the zellij codebase is shown below:
```rust
let destination_tab = self.get_indexed_tab_mut(destination_tab_index)
.context("failed to get destination tab by index")
.with_context(|| format!("failed to move clients from tab {source_tab_index} to tab {destination_tab_index}"))?;
```
Here the call to `self.get_indexed_tab_mut(destination_tab_index)` will return
a `Option`. The surrounding code, however, doesn't know what to do with a
`None` value, so it is considered an error.
Here you see that we attach two contexts:
```rust
.context("failed to get destination tab by index")
```
Because the `None` type itself doesn't tell us what the "error" means, we
attach a description manually. The second context:
```rust
.with_context(|| format!("failed to move clients from tab {source_tab_index} to tab {destination_tab_index}"))?;
```
then describes what the surrounding function was trying to achieve (See
descriptions above).
[tracking_issue]: https://github.com/zellij-org/zellij/issues/1753
[handle_panic]: https://docs.rs/zellij-utils/latest/zellij_utils/errors/fn.handle_panic.html
[miette]: https://crates.io/crates/miette