No handlers for SASL errors

[sehrope]

Leaving a note here as a reminder to take another look at the SASL error handling. At first glance I don't think the errors would bubble up as they're thrown synchronously in the helper functions and the caller does not catch or emit them up to the client.

node-postgres/lib/sasl.js

Lines 4 to 17 in 0acaf9d

	function startSession (mechanisms) {
	if (mechanisms.indexOf('SCRAM-SHA-256') === -1) {
	throw new Error('SASL: Only mechanism SCRAM-SHA-256 is currently supported')
	}
	const clientNonce = crypto.randomBytes(18).toString('base64')
	return {
	mechanism: 'SCRAM-SHA-256',
	clientNonce,
	response: 'n,,n=*,r=' + clientNonce,
	message: 'SASLInitialResponse'
	}
	}

node-postgres/lib/client.js

Lines 142 to 146 in 0acaf9d

	con.on('authenticationSASL', checkPgPass(function (msg) {
	saslSession = sasl.startSession(msg.mechanisms)
	con.sendSASLInitialResponseMessage(saslSession.mechanism, saslSession.response)
	}))

[gabegorelick]

At first glance I don't think the errors would bubble up as they're thrown synchronously in the helper functions and the caller does not catch or emit them up to the client.

This is still an issue as of pg 8.7.1.

To reproduce, attempt to connect to a PG instance that uses scram and don't specify a password when connecting:

const client = new Client();
try {
  await client.connect();
} catch (err) {
  // this is never reached
}

You'll get an uncaught exception: Error: SASL: SCRAM-SERVER-FIRST-MESSAGE: client password must be a string. But this really should be raised by client.connect and not be unhandled.

[rpominov]

I’m building a CLI app that uses node-postgres.

When I leave all connection arguments empty as a user of the app, it crashes with this very confusing error.

As a developer of the app, I don’t see any way to catch this error to provide any context to the user as to why it crashed and exit gracefully.

If it’s possible to prioritise fixing this, I would appreciate it greatly.

[charmander]

@rpominov Check if it’s missing before making a connection attempt.

[rpominov]

I think postgres allows to setup the server so that you can connect without a password.

Also checking the password is not trivial, as it can come from at least 4 places that I'm aware of:

• password option,
• connectionString option,
• env variable,
• .pgpass file.

In general, there seem to be a lot of authentication methods that node-postgres supports that I don't fully understand. Ideally, I want to just pass the options to node-postgres and show any errors that it produces back to the user, while stating in the documentation that they can use whatever authentication method that the library supports.

[ephys]

We're encountering this issue over at Sequelize after updating our CI to use postgres 14 (PR sequelize/sequelize#14737)

We have a test that checks that an error is raised is the password is invalid (in this case null), but because the error is not sent to Client.connect, we end up with an unhandled exception