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

Add hint when authentication fails, rv interactive #1169

Closed
wants to merge 3 commits into from

Conversation

Dhghomon
Copy link
Contributor

@Dhghomon Dhghomon commented Nov 1, 2023

Part of this is to finish up the error output here: #1125

Very small change, because at the moment the warning is displaying on the same line as the connecting message:

Connecting to EdgeDB instance 'anything' at localhost:10700...[2023-11-01T00:49:47Z WARN  edgedb_tokio::raw::connection] Error received from server: database 'edgedbb' does not exist. Severity: Error. Code: 0x4030005
edgedb error: ClientConnectionFailedError: end of stream while reading message
error: process didn't exit successfully: `target\debug\edgedb.exe -I anything` (exit code: 1)

so a change to eprintln! will end up with this display which should be sufficient:

Connecting to EdgeDB instance 'anything' at localhost:10700...
[2023-11-01T00:49:47Z WARN  edgedb_tokio::raw::connection] Error received from server: database 'edgedbb' does not exist. Severity: Error. Code: 0x4030005
edgedb error: ClientConnectionFailedError: end of stream while reading message
error: process didn't exit successfully: `target\debug\edgedb.exe -I anything` (exit code: 1)

So that's when the database indicated doesn't exist.

After that I had a look through the other parameters that can possibly fail inside the credentials directory, they are:

  • port: hangs up for 30 seconds and times out if wrong, (note: now added a message informing how long it will try before giving up)
  • tls_cert_data and tlc_ca: these fail immediately if wrong and show exactly what the problem is so good enough,
  • user and password: this output for these isn't very helpful, just a traceback:
edgedb error: AuthenticationError: authentication failed
  Server traceback:
      Traceback (most recent call last):
        File "edb/server/protocol/frontend.pyx", line 336, in edb.server.protocol.frontend.FrontendConnection.main
        File "edb/server/protocol/binary.pyx", line 1054, in authenticate
        File "edb/server/protocol/binary.pyx", line 284, in auth
        File "edb/server/protocol/frontend.pyx", line 572, in _authenticate
        File "edb/server/protocol/frontend.pyx", line 681, in _auth_scram
      edb.errors.AuthenticationError: authentication failed
error: process didn't exit successfully: `target\debug\edgedb.exe -I anything` (exit code: 1)

This 'authentication failed' message can take place in a lot of points during the authentication:

https://github.com/edgedb/edgedb/blob/286e015786326f3ce615a405f2cd0eba88642406/edb/server/protocol/frontend.pyx#L561

But looks like they all have something to do with user or password issues so maybe the easiest change at the moment is to stick in a hint that maybe the problem is inside the config file:

Failed to authenticate.
Hint: Use `edgedb info` to find and check the config for this instance

Finally, if println! is being used whether interactive or not then there's no need for a few of the functions that pass on the interactive bool so those get deleted.

Edit: now that edgedb/edgedb-rust#284 is merged I've added a note when connecting on how long the CLI will try before giving up. Also a note to the general help output that changing RUST_LOG can help with debugging when something has gone wrong.

@Dhghomon Dhghomon marked this pull request as ready for review November 1, 2023 02:52
@Dhghomon Dhghomon changed the title +hint when authentication fails, rv interactive Add hint when authentication fails, rv interactive Nov 1, 2023
@Dhghomon Dhghomon closed this Dec 12, 2023
@Dhghomon Dhghomon deleted the failed-authentication-help branch February 9, 2024 01:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant