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

"Caused By" error on missing relative dependency could be clearer #14924

Open
spease opened this issue Dec 11, 2024 · 4 comments
Open

"Caused By" error on missing relative dependency could be clearer #14924

spease opened this issue Dec 11, 2024 · 4 comments
Labels
A-diagnostics Area: Error and warning messages generated by Cargo itself. A-manifest Area: Cargo.toml issues C-bug Category: bug S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.

Comments

@spease
Copy link

spease commented Dec 11, 2024

Problem

When attempting to cargo install an application from a git repo on another computer

cargo install --git "ssh://spease@host/home/spease/projects/backuplib" backup-check

it failed with a series of "failed to load manifest errors". The issue was that an underlying dependency was outside the project tree as I had to modify a third-party library and the PR never got accepted or rejected, so I never updated the dependency to a permanent value. However even though this was simple, I didn't immediately catch this due to the displayed error.

error: failed to load manifest for workspace member `/Users/spease/.cargo/git/checkouts/backuplib-a3cf582f8b68cb05/681a95d/backup-check`
referenced by workspace at `/Users/spease/.cargo/git/checkouts/backuplib-a3cf582f8b68cb05/681a95d/Cargo.toml`

Caused by:
  failed to load manifest for dependency `backuplib`

Caused by:
  failed to load manifest for dependency `twox-hash`

Caused by:
  failed to read `/Users/spease/.cargo/git/checkouts/backuplib-a3cf582f8b68cb05/twox-hash/Cargo.toml`

Caused by:
  No such file or directory (os error 2)

Because each item was separated by "Caused by:", I interpreted it as a list of errors rather than a cause-and-effect / inverted stack trace. So I started working down the list, when I needed to go from the bottom and work up.

I also interpreted "Failed to load" as meaning that the path specified for the dependency was missing, when in actuality it just seems to mean that an error was encountered.

Finally, the path specified for the dependency that was outside the project tree used a generated path that was clearly on the local machine (macOS home folder vs Linux home folder), so in scanning the rest of the "caused by" it didn't immediately stand out as a broken relative path that wasn't able to be copied over due to being outside the project folder.

Steps

No response

Possible Solution(s)

For something like this, I think the following error message structure would be much clearer:

Error:
    `No such file or directory (os error 2)` when opening path `../../twox-hash` for dependency `twox-hash` needed by `backuplib`

Absolute path:
    /Users/spease/.cargo/git/checkouts/backuplib-a3cf582f8b68cb05/twox-hash/Cargo.toml

Needed by:
    /Users/spease/.cargo/git/checkouts/backuplib-a3cf582f8b68cb05/681a95d/backuplib/Cargo.toml
    /Users/spease/.cargo/git/checkouts/backuplib-a3cf582f8b68cb05/681a95d/backup-check/Cargo.toml

In workspace:
    /Users/spease/.cargo/git/checkouts/backuplib-a3cf582f8b68cb05/681a95d/Cargo.toml

Notes

It looks to me like relative path notation when doing a remote checkout can end up capturing something outside of the generated folder for that workspace. That doesn't seem great.

Version

cargo 1.80.0 (376290515 2024-07-16)
release: 1.80.0
commit-hash: 37629051518c3df9ac2c1744589362a02ecafa99
commit-date: 2024-07-16
host: aarch64-apple-darwin
libgit2: 1.7.2 (sys:0.18.3 vendored)
libcurl: 8.7.1 (sys:0.4.72+curl-8.6.0 system ssl:(SecureTransport) LibreSSL/3.3.6)
ssl: OpenSSL 1.1.1w  11 Sep 2023
os: Mac OS 15.1.1 [64-bit]
@spease spease added C-bug Category: bug S-triage Status: This issue is waiting on initial triage. labels Dec 11, 2024
@epage epage added A-diagnostics Area: Error and warning messages generated by Cargo itself. A-manifest Area: Cargo.toml issues labels Dec 11, 2024
@epage
Copy link
Contributor

epage commented Dec 11, 2024

Thanks for sharing your thought process through an error so we can find ways to improve it!

For something like this, I think the following error message structure would be much clearer:

FYI these "error chains" are something you will frequently run across in the Rust ecosystem as we've (imo) hit a local maxima of adding context to errors by wrapping them in another error.

It would be interesting to move away from error chaining but we'd be having to treat a lot of new ground in doing so and the errors are likely to have other usability problems.

Because each item was separated by "Caused by:", I interpreted it as a list of errors rather than a cause-and-effect / inverted stack trace.

So you saw

error: failed to load manifest for workspace member `/Users/spease/.cargo/git/checkouts/backuplib-a3cf582f8b68cb05/681a95d/backup-check`
referenced by workspace at `/Users/spease/.cargo/git/checkouts/backuplib-a3cf582f8b68cb05/681a95d/Cargo.toml`

Caused by:
  failed to load manifest for dependency `backuplib`

Caused by:
  failed to load manifest for dependency `twox-hash`

Caused by:
  failed to read `/Users/spease/.cargo/git/checkouts/backuplib-a3cf582f8b68cb05/twox-hash/Cargo.toml`

Caused by:
  No such file or directory (os error 2)

and thought it means there were multiple causes?

I wonder what are alternative ways of showing the chain that could help clarify that these are cascading, rather than parallel.

Maybe lowercase?

error: failed to load manifest for workspace member `/Users/spease/.cargo/git/checkouts/backuplib-a3cf582f8b68cb05/681a95d/backup-check`
referenced by workspace at `/Users/spease/.cargo/git/checkouts/backuplib-a3cf582f8b68cb05/681a95d/Cargo.toml`

caused by:
  failed to load manifest for dependency `backuplib`

caused by:
  failed to load manifest for dependency `twox-hash`

caused by:
  failed to read `/Users/spease/.cargo/git/checkouts/backuplib-a3cf582f8b68cb05/twox-hash/Cargo.toml`

caused by:
  No such file or directory (os error 2)

Or using lines to connect them (though this may be taken as parallel nodes, rather than a chain)

error: failed to load manifest for workspace member `/Users/spease/.cargo/git/checkouts/backuplib-a3cf582f8b68cb05/681a95d/backup-check`
referenced by workspace at `/Users/spease/.cargo/git/checkouts/backuplib-a3cf582f8b68cb05/681a95d/Cargo.toml`
|
|- Caused by:
|  failed to load manifest for dependency `backuplib`
| 
|- Caused by:
|   failed to load manifest for dependency `twox-hash`
| 
|- Caused by:
|   failed to read `/Users/spease/.cargo/git/checkouts/backuplib-a3cf582f8b68cb05/twox-hash/Cargo.toml`
| 
|- Caused by:
    No such file or directory (os error 2)

I also interpreted "Failed to load" as meaning that the path specified for the dependency was missing, when in actuality it just seems to mean that an error was encountered.

For me, failing to "load" can mean a lot of things and not just a read error of that specific file. However, if people have ideas for better wording, I'd be interesting in hearing it.

Finally, the path specified for the dependency that was outside the project tree used a generated path that was clearly on the local machine (macOS home folder vs Linux home folder), so in scanning the rest of the "caused by" it didn't immediately stand out as a broken relative path that wasn't able to be copied over due to being outside the project folder.

I wonder if we have enough information for identifying when a path dependency walks outside of a git repo and error more specifically about it. When writing that, it sounds important on the surface but I have a feeling it might be a rare case with the impact mostly being on the error message, that I wonder how much this work would pay off. This requires editing a local path (and not patching in uncommitted config), pushing the commit, there being no CI checking it, and then using it as a git dependency.

@epage epage added S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. and removed S-triage Status: This issue is waiting on initial triage. labels Dec 11, 2024
@spease
Copy link
Author

spease commented Dec 11, 2024

Backtraces have it right, and compiler errors do a better job of using formatting to show lowlevel cause underneath a high-level description. In this case it's mostly that there is a clear headline error at the top, but it's the last thing you should be looking at, and the cause-and-effect goes bottom-to-top rather than top-to-bottom.

If you wanted it to remain generic, you could just draw the user's eye to the bottom by emphasizing the final error, and then it more naturally flows bottom-to-top:

Error chain:
├ failed to load manifest for workspace member `/Users/spease/.cargo/git/checkouts/backuplib-a3cf582f8b68cb05/681a95d/backup-check`
referenced by workspace at `/Users/spease/.cargo/git/checkouts/backuplib-a3cf582f8b68cb05/681a95d/Cargo.toml`
├ failed to load manifest for dependency `backuplib`
├ failed to load manifest for dependency `twox-hash`
└ failed to read `/Users/spease/.cargo/git/checkouts/backuplib-a3cf582f8b68cb05/twox-hash/Cargo.toml`

**Error: No such file or directory (os error 2)**

(and ideally make Error red like the compiler output)

"No such file or directory" is an infuriating error message by itself that I see all too often (tell me the damn file!) but I recognize you want to just stringify the error code. And then the actual file causing it is directly adjacent to it, right above it, and this looks more like a stack trace of related errors since they're grouped together rather than separated by separate headings.

Alternative:

Error chain:
  4: failed to load manifest for workspace member `/Users/spease/.cargo/git/checkouts/backuplib-a3cf582f8b68cb05/681a95d/backup-check`
referenced by workspace at `/Users/spease/.cargo/git/checkouts/backuplib-a3cf582f8b68cb05/681a95d/Cargo.toml`
  3: failed to load manifest for dependency `backuplib`
  2: failed to load manifest for dependency `twox-hash`
  1: failed to read `/Users/spease/.cargo/git/checkouts/backuplib-a3cf582f8b68cb05/twox-hash/Cargo.toml`

**Error: No such file or directory (os error 2)**

This makes it even more clear that it's steps in a cause-and-effect chain imho, but I'm unsure if the numbers add any more function than that. The numbers can be reversed to be line numbers too, but I think having reverse order helps immediately emphasize what's going on.

You could also reverse this format:

**Error: No such file or directory (os error 2)**

Error stack:
  1: failed to read `/Users/spease/.cargo/git/checkouts/backuplib-a3cf582f8b68cb05/twox-hash/Cargo.toml`
  2: failed to load manifest for dependency `twox-hash`
  3: failed to load manifest for dependency `backuplib`
  4: failed to load manifest for workspace member `/Users/spease/.cargo/git/checkouts/backuplib-a3cf582f8b68cb05/681a95d/backup-check`
referenced by workspace at `/Users/spease/.cargo/git/checkouts/backuplib-a3cf582f8b68cb05/681a95d/Cargo.toml`

and in isolation this is probably the most readable, but on the console having the most specific error emphasized and at the very bottom means that it's the first thing that you see after running the command.

@epage
Copy link
Contributor

epage commented Dec 11, 2024

In terms of what order they are shown, that depends on the error and the user. Some parts of the top or bottom may be generic (e.g. "no such file or directory") and we should start with the most noteworthy part first with the question of which part is noteworthy.

As for rendering with numbers, I lean away from that because that feels more raw like backtraces while these errors are for users.

Looking around for inspiration,

miette uses a tree with arrows with the root error last:

color-eyre does numbered entries with root error last:

EDIT: anyhow does numbered entries with root error last:

Error: a

Caused by:
    0: b
    1: c
    2: d

@weihanglo
Copy link
Member

The current style Cargo chose is pretty similar to the default style the popular error handling crate anyhow is doing. See this example. In Cargo we kinda follow the pattern—using the shallowest error as the headline part, though it is not enforced. It may even have a rich error message from anywhere of the stack.

I feel like it is a general issue of Cargo's error handles. #10160 is a bit relevant as by flattening the trace stack we could remove some unimportant entries. It doesn't apply to this case though.

The code printing Caused by: lives here btw.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Error and warning messages generated by Cargo itself. A-manifest Area: Cargo.toml issues C-bug Category: bug S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.
Projects
None yet
Development

No branches or pull requests

3 participants