Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle errors gracefully in main() #99

Open
lauralt opened this issue Mar 3, 2021 · 4 comments
Open

Handle errors gracefully in main() #99

lauralt opened this issue Mar 3, 2021 · 4 comments

Comments

@lauralt
Copy link
Collaborator

lauralt commented Mar 3, 2021

We are currently panicking if VMM::try_from() or vmm.run() fail. We could instead handle those errors gracefully (by logging an error, exiting the process etc.).

@jcvenegas
Copy link

I can help with this

jcvenegas added a commit to jcvenegas/vmm-reference that referenced this issue May 24, 2021
Handle calls to VMM::try_from() or vmm.run().

This commit adds anyhow and thiserror crates to
handle errors.

The `anyhow` crate is used in the main binary to add
extra context and give a pretty view of error trace.

The create `thiserror` was used to easily
derive `std::error::Error` for errors provided by libs.

The new crates pulls the following extra crates:
```diff
+name = "proc-macro2"
+name = "quote"
+name = "syn"
+name = "thiserror-impl"
+name = "unicode-xid"
```

New error format:
examples:
```sh
$ vmm-reference --kernel \
    path=/path/vmlinuz-5.10.25
Error: Failed to create VMM from configurations

Caused by:
    Error issuing an ioctl to KVM.

$ echo $?
1
```

```sh
$ vmm-reference --kernel path
Error: Failed to parse CLI options

Caused by:
    Failed to parse cli Invalid input for kernel: Missing required
    argument: path
```

```
$vmm-reference
Error: Failed to parse CLI options

Caused by:
    Failed to parse cli error: The following required arguments were not provided:
        --kernel <kernel>

    USAGE:
        vmm-reference [OPTIONS] --kernel <kernel>

    For more information try --help
```

Fixes: rust-vmm#99

Signed-off-by: Carlos Venegas <[email protected]>
jcvenegas added a commit to jcvenegas/vmm-reference that referenced this issue May 24, 2021
Handle calls to VMM::try_from() or vmm.run().

This commit adds anyhow and thiserror crates to
handle errors.

The `anyhow` crate is used in the main binary to add
extra context and give a pretty view of error trace.

The create `thiserror` was used to easily
derive `std::error::Error` for errors provided by libs.

The new crates pulls the following extra crates:
```diff
+name = "proc-macro2"
+name = "quote"
+name = "syn"
+name = "thiserror-impl"
+name = "unicode-xid"
```

New error format:
examples:
```sh
$ vmm-reference --kernel \
    path=/path/vmlinuz-5.10.25
Error: Failed to create VMM from configurations

Caused by:
    Error issuing an ioctl to KVM.

$ echo $?
1
```

```sh
$ vmm-reference --kernel path
Error: Failed to parse CLI options

Caused by:
    Failed to parse cli Invalid input for kernel: Missing required
    argument: path
```

```
$vmm-reference
Error: Failed to parse CLI options

Caused by:
    Failed to parse cli error: The following required arguments were not provided:
        --kernel <kernel>

    USAGE:
        vmm-reference [OPTIONS] --kernel <kernel>

    For more information try --help
```

Fixes: rust-vmm#99

Signed-off-by: Carlos Venegas <[email protected]>
jcvenegas added a commit to jcvenegas/vmm-reference that referenced this issue May 24, 2021
Handle calls to VMM::try_from() or vmm.run().

This commit adds anyhow and thiserror crates to
handle errors.

The `anyhow` crate is used in the main binary to add
extra context and give a pretty view of error trace.

The create `thiserror` was used to easily
derive `std::error::Error` for errors provided by libs.

The new crates pulls the following extra crates:
```diff
+name = "proc-macro2"
+name = "quote"
+name = "syn"
+name = "thiserror-impl"
+name = "unicode-xid"
```

New error format:
examples:
```sh
$ vmm-reference --kernel \
    path=/path/vmlinuz-5.10.25
Error: Failed to create VMM from configurations

Caused by:
    Error issuing an ioctl to KVM.

$ echo $?
1
```

```sh
$ vmm-reference --kernel path
Error: Failed to parse CLI options

Caused by:
    Failed to parse cli Invalid input for kernel: Missing required
    argument: path
```

```
$vmm-reference
Error: Failed to parse CLI options

Caused by:
    Failed to parse cli error: The following required arguments were not provided:
        --kernel <kernel>

    USAGE:
        vmm-reference [OPTIONS] --kernel <kernel>

    For more information try --help
```

Fixes: rust-vmm#99

Signed-off-by: Carlos Venegas <[email protected]>
jcvenegas added a commit to jcvenegas/vmm-reference that referenced this issue May 24, 2021
Handle calls to `CLI`, `VMM::try_from()` and `vmm.run()`.

This commit adds anyhow and thiserror crates to
handle errors.

The `anyhow` crate is used in the main binary to add
extra context and give a pretty view of error trace.

The create `thiserror` was used to easily
derive `std::error::Error` for errors provided by libs.

The new crates pulls the following extra crates:
```diff
+name = "proc-macro2"
+name = "quote"
+name = "syn"
+name = "thiserror-impl"
+name = "unicode-xid"
```

New error format:
examples:
```sh
$ vmm-reference --kernel \
    path=/path/vmlinuz-5.10.25
Error: Failed to create VMM from configurations

Caused by:
    Error issuing an ioctl to KVM.

$ echo $?
1
```

```sh
$ vmm-reference --kernel path
Error: Failed to parse CLI options

Caused by:
    Failed to parse cli Invalid input for kernel: Missing required
    argument: path
```

```
$vmm-reference
Error: Failed to parse CLI options

Caused by:
    Failed to parse cli error: The following required arguments were not provided:
        --kernel <kernel>

    USAGE:
        vmm-reference [OPTIONS] --kernel <kernel>

    For more information try --help
```

Fixes: rust-vmm#99

Signed-off-by: Carlos Venegas <[email protected]>
jcvenegas added a commit to jcvenegas/vmm-reference that referenced this issue May 24, 2021
Handle calls to `CLI`, `VMM::try_from()` and `vmm.run()`.

This commit adds anyhow and thiserror crates to
handle errors.

The `anyhow` crate is used in the main binary to add
extra context and give a pretty view of error trace.

The create `thiserror` was used to easily
derive `std::error::Error` for errors provided by libs.

The new crates pulls the following extra crates:
```diff
+name = "proc-macro2"
+name = "quote"
+name = "syn"
+name = "thiserror-impl"
+name = "unicode-xid"
```

New error format:
examples:
```sh
$ vmm-reference --kernel \
    path=/path/vmlinuz-5.10.25
Error: Failed to create VMM from configurations

Caused by:
    Error issuing an ioctl to KVM.

$ echo $?
1
```

```sh
$ vmm-reference --kernel path
Error: Failed to parse CLI options

Caused by:
    Failed to parse cli Invalid input for kernel: Missing required
    argument: path
```

```
$vmm-reference
Error: Failed to parse CLI options

Caused by:
    Failed to parse cli error: The following required arguments were not provided:
        --kernel <kernel>

    USAGE:
        vmm-reference [OPTIONS] --kernel <kernel>

    For more information try --help
```

Fixes: rust-vmm#99

Signed-off-by: Carlos Venegas <[email protected]>
jcvenegas added a commit to jcvenegas/vmm-reference that referenced this issue May 24, 2021
Handle calls to `CLI`, `VMM::try_from()` and `vmm.run()`.

This commit adds anyhow and thiserror crates to
handle errors.

The `anyhow` crate is used in the main binary to add
extra context and give a pretty view of error trace.

The create `thiserror` was used to easily
derive `std::error::Error` for errors provided by libs.

The new crates pulls the following extra crates:
```diff
+name = "proc-macro2"
+name = "quote"
+name = "syn"
+name = "thiserror-impl"
+name = "unicode-xid"
```

New error format:
examples:
```sh
$ vmm-reference --kernel \
    path=/path/vmlinuz-5.10.25
Error: Failed to create VMM from configurations

Caused by:
    Error issuing an ioctl to KVM.

$ echo $?
1
```

```sh
$ vmm-reference --kernel path
Error: Failed to parse CLI options

Caused by:
    Failed to parse cli Invalid input for kernel: Missing required
    argument: path
```

```
$vmm-reference
Error: Failed to parse CLI options

Caused by:
    Failed to parse cli error: The following required arguments were not provided:
        --kernel <kernel>

    USAGE:
        vmm-reference [OPTIONS] --kernel <kernel>

    For more information try --help
```

Fixes: rust-vmm#99

Signed-off-by: Carlos Venegas <[email protected]>
jcvenegas added a commit to jcvenegas/vmm-reference that referenced this issue May 24, 2021
Handle calls to `CLI`, `VMM::try_from()` and `vmm.run()`.

This commit adds anyhow and thiserror crates to handle errors.

The `anyhow` crate is used in the main binary to add extra context and
give a pretty view of error trace.

The create `thiserror` was used to easily derive `std::error::Error` for
errors provided by libs.

The new crates pulls the following extra crates: ```diff +name =
"proc-macro2" +name = "quote" +name = "syn" +name = "thiserror-impl"
+name = "unicode-xid" ```

New error format: examples: ```sh $ vmm-reference --kernel \
path=/path/vmlinuz-5.10.25 Error: Failed to create VMM from
configurations

Caused by: Error issuing an ioctl to KVM.

$ echo $?  1 ```

```sh $ vmm-reference --kernel path Error: Failed to parse CLI options

Caused by: Failed to parse cli Invalid input for kernel: Missing
required argument: path ```

``` $vmm-reference Error: Failed to parse CLI options

Caused by: Failed to parse cli error: The following required arguments
were not provided: --kernel <kernel>

    USAGE: vmm-reference [OPTIONS] --kernel <kernel>

    For more information try --help ```

Fixes: rust-vmm#99

Signed-off-by: Carlos Venegas <[email protected]>
jcvenegas added a commit to jcvenegas/vmm-reference that referenced this issue May 24, 2021
Handle calls to `CLI`, `VMM::try_from()` and `vmm.run()`.

This commit adds anyhow and thiserror crates to handle errors.

The `anyhow` crate is used in the main binary to add extra context and
give a pretty view of error trace.

The create `thiserror` was used to easily derive `std::error::Error` for
errors provided by libs.

The new crates pulls the following extra crates: ```diff +name =
"proc-macro2" +name = "quote" +name = "syn" +name = "thiserror-impl"
+name = "unicode-xid" ```

New error format: examples: ```sh $ vmm-reference --kernel \
path=/path/vmlinuz-5.10.25 Error: Failed to create VMM from
configurations

Caused by: Error issuing an ioctl to KVM.

$ echo $?  1 ```

```sh $ vmm-reference --kernel path Error: Failed to parse CLI options

Caused by: Failed to parse cli Invalid input for kernel: Missing
required argument: path ```

``` $vmm-reference Error: Failed to parse CLI options

Caused by: Failed to parse cli error: The following required arguments
were not provided: --kernel <kernel>

    USAGE: vmm-reference [OPTIONS] --kernel <kernel>

    For more information try --help ```

Fixes: rust-vmm#99

Signed-off-by: Carlos Venegas <[email protected]>
@jcvenegas
Copy link

jcvenegas commented May 24, 2021

I have opened #122 , there I add some error handling using two external crates, thiserror and anyhow. There I refactor the main binary crate, given that is using some external creates is OK? or to much as I dont see to many external creates used, I can just limit to handle the unwrap in main if thiserror and anyhow is something is not wanted.

@lauralt
Copy link
Collaborator Author

lauralt commented May 25, 2021

Hi, @jcvenegas! You're right, we are indeed trying to limit the number of external dependencies. I think it should be fine to just handle the unwraps, without adding thiserror etc. (an example in this commit). For now we could just use println!s, since there is also this PR which is replacing them with the log crate.
Thanks for helping with this!

@jcvenegas
Copy link

@lauralt I will send a second PR as al alternative proposal

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants