Skip to content

Improve error reporting by printing error chain sources #1277

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Johan-Liebert1
Copy link

While working on integrating composefs-rs, received this error

ERROR Installing to disk: Creating composefs deployment: Failed to create image proxy: error

The actual error occurred deep into the containers-image-proxy crate and was a bit difficult to track down. After this patch below is what the error reporting looks like

ERROR Installing to disk: Creating composefs deployment: Failed to create image proxy: error
0: Error {
    context: "Failed to create image proxy",
    source: Other(
        "skopeo proxy unexpectedly exited during request method Initialize: exit status: 125\nError: please use unshare with rootless\n",
    ),
}
1: Other(
    "skopeo proxy unexpectedly exited during request method Initialize: exit status: 125\nError: please use unshare with rootless\n",
)

This is more for better dev-ex, so please let me know if this should be behind some sort of flag, like something similar to BOOTC_ERR_TRACE=1

@cgwalters
Copy link
Collaborator

Thanks for your patch! A minor procedural thing but this project tends to use "topic prefixes", so can you add e.g. cli: or so as a prefix? Also hopefully most patches are improvements, and the "imperative mood" is preferred so this could be condensed to just e.g.:

cli: Print error chain sources

Anyways on the actual code change. Yeah, today we use the "alternative printer" for anyhow which formats errors as a single line. I have personally felt in the past this is more readable than the default, especially as we use the #[context] attribute somewhat liberally.

In this specific case...I'm confused as to why we're only getting error at the end and not the full source. It maybe relates to our use of thiserror in the proxy crate?

This is more for better dev-ex, so please let me know if this should be behind some sort of flag, like something similar to BOOTC_ERR_TRACE=1

Perhaps when -v is used (ref #1154 ) we could also change how errors are printed to be more verbose.

In this specific case though it seems to me we should just fix the proxy code right?

Offhand I wonder if we should change https://github.com/containers/containers-image-proxy-rs/blob/832795839fb71ae9aba11dc1a8d86f029adb7f1a/src/imageproxy.rs#L35C1-L35C48 to use #[from] instead?

@cgwalters cgwalters added triaged This looks like a valid issue area/client Related to the client/CLI labels Apr 16, 2025
Copy link
Collaborator

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

Forgot to mark as requested-changes per above

Currently only the top level error is printed which makes it hard to
pinpoint the origin of the error, this patch prints the error chain
similar to a backtrace.

Signed-off-by: Pragyan Poudyal <[email protected]>
@Johan-Liebert1
Copy link
Author

Johan-Liebert1 commented Apr 17, 2025

Thanks for the review. Updated the commit message, I read the docs about commit message format but forgot them while actually committing.

In this specific case...I'm confused as to why we're only getting error at the end and not the full source

Seems like this is intentional behaviour on anyhow's end https://docs.rs/anyhow/1.0.28/anyhow/struct.Error.html#display-representations

The docs say {:?} will print a backtrace, but I guess that's only true in case of a panic?

Perhaps when -v is used

#1154 seems to be WIP as of now, but yes we could definitely have more verbose error reporting.

In this specific case though it seems to me we should just fix the proxy code right?

But, the fact still remains that anyhow will only print the error message on the top of the stack, so we will still end up missing out on printing the root cause.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/client Related to the client/CLI triaged This looks like a valid issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants