node-postgres icon indicating copy to clipboard operation
node-postgres copied to clipboard

pg-connection-string ConnectionOptions interface is not compatible with PoolConfig

Open hjr3 opened this issue 5 years ago • 4 comments

There are a few issues:

  1. pg-connection-string ConnectionOptions has database: string | null | undefined vs PoolConfig has database?: string; (which effectively means database: string | undefined).
  2. pg-connection-string ConnectionOptions has ssl?: boolean | string vs PoolConfig has ssl?: boolean | ConnectionOptions

hjr3 avatar Jul 16 '20 01:07 hjr3

I originally thought about making a short term fix to include tls.ConnectionOption as part of pg-connection-string. This may break some TypeScript code though if people were doing something like:

if (config.ssl === '1') {
   // some logic
} else {
   // implicitly thinking it is now a bool
}

My recommendation is:

  • Replace ConnectionOptions with PoolConfig
  • Release pg-connection-string to 3.0.0 with this breaking change

hjr3 avatar Jul 16 '20 01:07 hjr3

@brianc I just encountered this in the Mastodon codebase, and the only solution I had for working around it was this:

    const parsedUrl = dbUrlToConfig(env.DATABASE_URL);

    // The result of dbUrlToConfig from pg-connection-string is not type
    // compatible with pg.PoolConfig, since parts of the connection URL may be
    // `null` when pg.PoolConfig expects `undefined`, as such we have to
    // manually create the baseConfig object from the properties of the
    // parsedUrl.
    //
    // For more information see: https://github.com/brianc/node-postgres/issues/2280
    if (typeof parsedUrl.password === 'string') baseConfig.password = parsedUrl.password;
    if (typeof parsedUrl.host === 'string') baseConfig.host = parsedUrl.host;
    if (typeof parsedUrl.user === 'string') baseConfig.user = parsedUrl.user;
    if (typeof parsedUrl.port === 'string') baseConfig.port = parseInt(parsedUrl.port, 10);
    if (typeof parsedUrl.database === 'string') baseConfig.database = parsedUrl.database;
    if (typeof parsedUrl.options === 'string') baseConfig.options = parsedUrl.options;

    // The pg-connection-string type definition isn't correct, as parsedUrl.ssl
    // can absolutely be an Object, this mess is to work around these incorrect types:
    if (typeof parsedUrl.ssl === 'boolean') {
      baseConfig.ssl = parsedUrl.ssl;
    } else if (typeof parsedUrl.ssl === 'object') {
      /** @type {Record<string, any>} */
      const sslOptions = parsedUrl.ssl;
      baseConfig.ssl = {};

      baseConfig.ssl.cert = sslOptions.cert;
      baseConfig.ssl.key = sslOptions.key;
      baseConfig.ssl.ca = sslOptions.ca;
      baseConfig.ssl.rejectUnauthorized = sslOptions.rejectUnauthorized;
    }

Which is obviously quite a mess, but the only way to get the two to play nicely together.

ThisIsMissEm avatar Jan 15 '24 23:01 ThisIsMissEm

Maybe I can make a compat function in pg-connection-string to convert it into the correct type.

hjr3 avatar Jan 16 '24 01:01 hjr3

@ThisIsMissEm i opened https://github.com/brianc/node-postgres/pull/3128 to resolve this issue.

hjr3 avatar Jan 17 '24 16:01 hjr3