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.

Sign up for GitHub

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

Extra help when config incorrect #1187

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+27 −7
Open

Extra help when config incorrect #1187

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5155799
Extra help when config incorrect
Dhghomon Dec 12, 2023
94964f8
Update cargo.lock
Dhghomon Dec 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 20 additions & 3 deletions src/connect.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use bytes::Bytes;
use tokio::time::sleep;
use tokio_stream::Stream;

use edgedb_errors::{Error, ErrorKind, ResultExt};
use edgedb_errors::{Error, ErrorKind, ResultExt, AuthenticationError};
use edgedb_errors::{NoDataError, ProtocolEncodingError, ClientError};
use edgedb_protocol::QueryResult;
use edgedb_protocol::client_message::{State, CompilationOptions};
Expand Down Expand Up @@ -176,7 +176,12 @@ impl Connector {
_ =>
format!("EdgeDB instance at {}", cfg.display_addr())
};
format!("Connecting to {}...", desc)
let wait = cfg.wait_until_available();
let wait_msg = match wait.as_secs() {
seconds if seconds < 1 => format!("{}ms", wait.as_millis()),
seconds => format!("{seconds}s")
};
format!("Connecting to {desc} (will try up to {wait_msg})...\n")
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this telling me the number of retries or the time? I assume it's the time since wait_msg is set to wait.as_secs(), but it's not entirely clear based on the message. I.e., is the next word "times" or "seconds?"

Maybe wait_msg includes "seconds," and if so, ignore this suggestion.

Suggested change
format!("Connecting to {desc} (will try up to {wait_msg})...\n")
format!("Connecting to {desc} (will try for up to {wait_msg} seconds)...\n")

Copy link
Contributor Author

@Dhghomon Dhghomon Dec 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, it'll be seconds or milliseconds (though far more likely to be seconds), written s or ms (ms if under one second). Could change s to seconds though.

Interestingly during the 30 seconds you can see it trying over and over again if you set the RUST_LOG to something like debug so we could technically put those messages in if it makes sense. It's noisier but the advantage would be that the user gets to see the problem within five seconds instead of twiddling thumbs until the 30 seconds have passed / killing the process.

Here's the output with debug, could pull out those debug messages and print them a bit nicer:

PS C:\rust\edgedb-cli> cargo run
    Finished dev [optimized + debuginfo] target(s) in 0.45s
     Running `target\debug\edgedb.exe`
[2023-12-12T13:58:31Z DEBUG edgedb::version_check] Cached version None
[2023-12-12T13:58:31Z INFO  edgedb_tokio::raw::connection] Connecting via TCP localhost:10757
[2023-12-12T13:58:31Z DEBUG rustyline] PathInfo("C:\\Users\\mithr\\AppData\\Local\\edgedb\\edgeql.history", SystemTime { intervals: 133468273415897816 }, 100)
Connecting to EdgeDB instance 'edgedb_cli' at localhost:10757 (will try up to 30s)...
[2023-12-12T13:58:35Z DEBUG edgedb_tokio::raw::connection] Temporary connection error: ClientConnectionError: No connection could be made because the target machine actively refused it. (os error 10061)
[2023-12-12T13:58:39Z DEBUG edgedb_tokio::raw::connection] Temporary connection error: ClientConnectionError: No connection could be made because the target machine actively refused it. (os error 10061)
[2023-12-12T13:58:43Z DEBUG edgedb_tokio::raw::connection] Temporary connection error: ClientConnectionError: No connection could be made because the target machine actively refused it. (os error 10061)
[2023-12-12T13:58:47Z DEBUG edgedb_tokio::raw::connection] Temporary connection error: ClientConnectionError: No connection could be made because the target machine actively refused it. (os error 10061)
[2023-12-12T13:58:51Z DEBUG edgedb_tokio::raw::connection] Temporary connection error: ClientConnectionError: No connection could be made because the target machine actively refused it. (os error 10061)
[2023-12-12T13:58:56Z DEBUG edgedb_tokio::raw::connection] Temporary connection error: ClientConnectionError: No connection could be made because the target machine actively refused it. (os error 10061)
[2023-12-12T13:59:00Z DEBUG edgedb_tokio::raw::connection] Temporary connection error: ClientConnectionError: No connection could be made because the target machine actively refused it. (os error 10061)
[2023-12-12T13:59:04Z DEBUG edgedb_tokio::raw::connection] Temporary connection error: ClientConnectionError: No connection could be made because the target machine actively refused it. (os error 10061)
edgedb error: ClientConnectionFailedError: cannot establish connection for 30s: No connection could be made because the target machine actively refused it. (os error 10061)
error: process didn't exit successfully: `target\debug\edgedb.exe` (exit code: 1)```

}

async fn print_warning(&self, cfg: &Config, interactive: bool)
Expand All @@ -198,8 +203,20 @@ impl Connector {

impl Connection {
pub async fn connect(cfg: &Config) -> Result<Connection, Error> {
let connection = match raw::Connection::connect(&cfg).await {
Ok(conn) => conn,
Err(err) => {
if err.is::<AuthenticationError>() {
eprintln!("Failed to authenticate.\
\nHint: Use `edgedb info` to find and check the config for this instance");
return Err(err)
} else {
return Err(err)
}
}
};
Ok(Connection {
inner: raw::Connection::connect(&cfg).await?,
inner: connection,
state: State::empty(),
server_version: None,
config: cfg.clone(),
Expand Down
3 changes: 3 additions & 0 deletions src/options.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -647,7 +647,10 @@ impl Options {
}
);

let extra_help = "Hint: Setting RUST_LOG to `info`, `debug`, or `trace` can provide extra debugging info when encountering an issue.\n";

let app = clap::Command::new("edgedb")
.after_help(extra_help)
.term_width(term_width())
.args(deglobalized);

Expand Down
Loading