Skip to content

CLI: improve error reporting#659

Merged
imobachgs merged 6 commits intoagama-project:masterfrom
imobachgs:improve-error-reporting
Jul 13, 2023
Merged

CLI: improve error reporting#659
imobachgs merged 6 commits intoagama-project:masterfrom
imobachgs:improve-error-reporting

Conversation

@imobachgs
Copy link
Copy Markdown
Contributor

@imobachgs imobachgs commented Jul 12, 2023

Problem

CLI error reporting is rather poor. In some cases, the errors are pretty hard to read (e.g., when it cannot connect to the D-Bus server) and, in other cases, the problem is not properly reported (e.g., when lshw or jsonnet are missing or when the jsonnet invocation fails).

The anyhow crate helps with reporting the problems, including a backtrace if supported. We should make use of its reporting capabilities.

Errors to focus on:

  • ServiceError, which is anyone coming from the D-Bus layer.
  • Missing binaries (lshw, jsonnet).
  • Cannot download a profile (from libcurl).
  • Failing commands (e.g., running jsonnet).

Solution

Better CLI errors

agama-cli uses now anyhow to report the errors. Let's see some examples:

# agama profile download http://missing-domain.net/profile.jsonnet
Could not read the profile

Caused by:
    [6] Couldn't resolve host name (Could not resolve host: missing-domain.net)

# agama profile evaluate bad.jsonnet
Could not evaluate the profile

Caused by:
    Jsonnet evaluation failed:
    profile.jsonnet:39:15-30 Unknown variable: findBiggestDisk
    
            name: findBiggestDisk(agama.disks),

# agama config show
Could not connect to Agama bus at 'unix:path=/run/agama/bus'

Caused by:
    0: I/O error: No such file or directory (os error 2)
    1: No such file or directory (os error 2)

Exit code

agama-cli now returns an exit code which is especially important when using the CLI in scripts. As usual, it returns 0 for success and 1 for errors.

Unknown product 'Unknown'. Available products: '["ALP-Bedrock", "ALP-Micro", "Tumbleweed", "Leap16"]'
$ echo $?
1

Stop using &str as error types

In Rust, anything can be used as an error. Nothing stops you from using, e.g., a string slice (&str) as your error type. We used this approach when handling settings from the command line. This PR introduces a new SettingsError enum instead.

Partially get rid of Box<dyn Error>

We have replaced Box<dyn Error> with concrete error types.

Testing

  • Tested manually

@coveralls
Copy link
Copy Markdown

Coverage Status

coverage: 72.398% (-0.03%) from 72.432% when pulling e0d3ba6 on imobachgs:improve-error-reporting into a108b23 on openSUSE:master.

@imobachgs imobachgs marked this pull request as ready for review July 12, 2023 15:15
@mvidner
Copy link
Copy Markdown
Contributor

mvidner commented Jul 13, 2023

Thanks for pointing out Luca Palmieri's Error Handling In Rust - A Deep Dive as a detailed background piece

Copy link
Copy Markdown
Contributor

@mvidner mvidner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks!

@imobachgs imobachgs merged commit f33d147 into agama-project:master Jul 13, 2023
@imobachgs imobachgs deleted the improve-error-reporting branch July 13, 2023 08:30
@imobachgs imobachgs mentioned this pull request Aug 2, 2023
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 this pull request may close these issues.

3 participants