vapor / postgres-nio

🐘 Non-blocking, event-driven Swift client for PostgreSQL.
https://api.vapor.codes/postgresnio/documentation/postgresnio/
MIT License
327 stars 75 forks source link

PostgresConnection.authenticate responds with `uncleanShutdown` error when wrong credentials are passed #160

Open avolokhov opened 3 years ago

avolokhov commented 3 years ago

Describe the bug

When I mess up my credentials and try to send a random gibberish as a password, the error I get in return is obscure and not very helpful. I would expect PostgresConnection.authenticate to fail with a clean auth failed error, but I get PostgresError.protocol("Unexpected connection close") instead.

To Reproduce

  1. set up a working postgres instance
  2. connect to it via PostgresConnection.connect().authenticate() using a random gibberish for the password
  3. check the error message

Expected behavior

I'd expect a clean auth failed error.

Environment

vapor 4.45.2 postgres-nio 1.5.2

avolokhov commented 3 years ago

When I debug it locally, I see we're hitting this error case in ConnectionStateMachine#186:

   mutating func closed() -> ConnectionAction {
        switch self.state {
        case .initialized:
            preconditionFailure("How can a connection be closed, if it was never connected.")
        case .closed:
            preconditionFailure("How can a connection be closed, if it is already closed.")
        case .authenticated,
             .sslRequestSent,
             .sslNegotiated,
             .sslHandlerAdded,
             .waitingToStartAuthentication,
             .authenticating,
             .readyForQuery,
             .extendedQuery,
             .prepareStatement,
             .closeCommand:
            return self.errorHappened(.uncleanShutdown) // <----- Here
        case .error, .closing:
            self.state = .closed
            self.quiescingState = .notQuiescing
            return .fireChannelInactive
        case .modifying:
            preconditionFailure("Invalid state")
        }
    }

In debugger I see self.state has an information that we're currently in authenticating/passwordAuthenticationSent. I think if we were able to bubble this state up and log/throw it appropriately, this would be a huge help in investigating such problems.