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

Better offline error message #2110

Merged
merged 2 commits into from
Mar 4, 2024
Merged

Conversation

konstin
Copy link
Member

@konstin konstin commented Mar 1, 2024

Error for uv pip compile scripts/requirements/jupyter.in without internet:

Before

error: error sending request for url (https://pypi.org/simple/jupyter/): error trying to connect: dns error: failed to lookup address information: No such host is known. (os error 11001)
  Caused by: error trying to connect: dns error: failed to lookup address information: No such host is known. (os error 11001)
  Caused by: dns error: failed to lookup address information: No such host is known. (os error 11001)
  Caused by: failed to lookup address information:  No such host is known. (os error 11001)

After

error: Could not connect, are you offline?
  Caused by: error sending request for url (https://pypi.org/simple/django/): error trying to connect: dns error: failed to lookup address information: Temporary failure in name resolution
  Caused by: error trying to connect: dns error: failed to lookup address information: Temporary failure in name resolution
  Caused by: dns error: failed to lookup address information: Temporary failure in name resolution
  Caused by: failed to lookup address information: Temporary failure in name resolution

On linux, it would be "Temporary failure in name resolution" instead of "No such host is known. (os error 11001)".

The implementation checks for "dne error" stringly as hyper errors are opaque. The danger is that this breaks with a hyper update. We still get the complete error trace since reqwest eagerly inlines errors (seanmonstar/reqwest#2147).

No test since i wouldn't know how to simulate this in cargo test.

Fixes #1971

Error for `uv pip compile scripts/requirements/jupyter.in` without internet:

**Before**

```
error: error sending request for url (https://pypi.org/simple/jupyter/): error trying to connect: dns error: failed to lookup address information: Temporary failure in name resolution
  Caused by: error trying to connect: dns error: failed to lookup address information: Temporary failure in name resolution
  Caused by: dns error: failed to lookup address information: Temporary failure in name resolution
  Caused by: failed to lookup address information: Temporary failure in name resolution
```

**After**

```
error: error sending request for url (https://pypi.org/simple/jupyter/): error trying to connect: dns error: No such host is known. (os error 11001)
  Caused by: Could not connect, are you offline?
```

The implementation checks for "dne error" stringly as hyper errors are opaque. The danger is that this breaks with a hyper update. We still get the complete error trace since reqwest eagerly inlines errors (seanmonstar/reqwest#2147).

No test since i wouldn't know how to simulate this in cargo test.

Fixes #1971
@konstin konstin added the error messages Messaging when something goes wrong label Mar 1, 2024
@zanieb
Copy link
Member

zanieb commented Mar 1, 2024

Isn't the error / caused by out of order here?

@konstin
Copy link
Member Author

konstin commented Mar 1, 2024

Yeah, will switch. The feature i really want is to attach hints to error messages that are shown below the error chain themselves. I also want that for build errors (#2108). We could do that by defining our own ErrorHint trait and downcasting when we print the chain in main.

@konstin
Copy link
Member Author

konstin commented Mar 4, 2024

I've switched the error hierarchy, it's verbose again.

I've experimented a bit with adding help messages but i don't think it's possible without either adding that trait to our entire error hierarchy (the miette approach) or losing the error type in a struct ErrorWithHelp { help: String, source: Box<dyn Error + Send + Sync + 'static> }.

@konstin konstin requested review from zanieb and removed request for charliermarsh March 4, 2024 11:03
@konstin konstin merged commit 898c3f6 into main Mar 4, 2024
7 checks passed
@konstin konstin deleted the konsti/better-offline-error-message branch March 4, 2024 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
error messages Messaging when something goes wrong
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No internet error message is bad
2 participants