Better build error messages

[konstin]

Build failures are one of the most common user facing failures that aren't "obivous" errors (such as typos) or resolver errors. Currently, they show more technical details than being focussed on this being an error in a subprocess that is either on the side of the package or - more likely - in the build environment, e.g. the user needs to install a dev package or their python version is incompatible.

The new error message clearly delineates the part that's important (this is a build backend problem) from the internals (we called this hook) and is consistent about which part of the dist building stage failed. We have to calibrate the exact wording of the error message some more. Most of the implementation is working around the orphan rule, (this)error rules and trait rules, so it came out more of a refactoring than intended.

Example:

[zanieb]

Might need to rope in @BurntSushi to review this implementation detail

[BurntSushi]

One thing that would be useful here is a little more docs like, "If the orphan-rule didn't exist, we would do ... instead."

[zanieb]

AnyBuildError?

[konstin]

It's more like "anyhow::Error, but with an extra trait for build error querying on top".

[zanieb]

Would it also be feasible to explain why we're building a package? Like are we doing it for install or just to read its metadata? Is that already obvious?

[BurntSushi]

I think this is probably fine. At least, I don't immediately see a simpler solution. But some extra docs (see comments) might help clear up some things.

[BurntSushi]

Do you mean IsBuildBackendError?

[BurntSushi]

Also, I would mention that AsDynError is an internal thing in thiserror. It took me a bit to figure that out as I was confused about it not being in thiserror's public API docs.

[BurntSushi]

Would it be possible to spell out the concrete cycle here? I think that would aide understanding what specifically this trait is trying to do.

[konstin]

I'll leave this here as context for the update later.

[BurntSushi]

One thing that would be useful here is a little more docs like, "If the orphan-rule didn't exist, we would do ... instead."

[BurntSushi]

Same as below, but this might be dropping any chain that might have been on anyhow::Error? Not 100% sure

[konstin]

AnyErrorBuild preserves the chain, it passes .source() calls through to its wrapped type.

[BurntSushi]

I mean its Display impl. The Display impl for a Box<dyn Error> doesn't emit the chain. But a anyhow::Error will.

[konstin]

afaik all the error types only show their inner message, but not the chain, over which you have to iterate manually. When testing,

use thiserror::Error;

#[derive(Error, Debug)]
#[error("Wrapper Err")]
struct WrappedErr;

fn main() {
    let err: anyhow::Error = WrappedErr.into();
    println!("{}", err);
    println!("{}", err.context("2"));
}

shows

Wrapper Err
2

and we don't seem to be losing and context in our snapshot tests.

[konstin]

anyhow does show the chain, but only in alternate mode:

use thiserror::Error;

#[derive(Error, Debug)]
#[error("Inner Err")]
struct InnerErr;

#[derive(Error, Debug)]
#[error("Wrapper Err")]
struct WrappedErr(#[source] InnerErr);

fn main() {
    let err = WrappedErr(InnerErr);
    println!("{:#}", err);
    let err: anyhow::Error = WrappedErr(InnerErr).into();
    println!("{:#}", err);
    println!("{:#}", err.context("2"));
}

shows

Wrapper Err
Wrapper Err: Inner Err
2: Wrapper Err: Inner Err

[BurntSushi]

Ah okay, so anyhow::Error's Display impl is the same as Box<dyn Error>. Interesting. All righty.

And yeah, I realize the snapshot tests indicate nothing untoward is happening here. I don't have a full grasp of the surrounding code and I wasn't sure how easy it would be to break something in the future. But I think you've demonstrated my concern is unfounded. Thank you. :)

[BurntSushi]

Thank you!

[BurntSushi]

Nice thank you!

[zanieb]

Awesome. Sorry for the delay!