Add connect_timeout with a default of 30 seconds

porsager · porsager · commit 2e01c2b3039e · 2020-04-07T02:00:58.000+02:00
diff --git a/README.md b/README.md
@@ -44,25 +44,26 @@ You can use either a `postgres://` url connection string or the options to defin
 
 ```js
 const sql = postgres('postgres://username:password@host:port/database', {
-  host        : '',         // Postgres ip address or domain name
-  port        : 5432,       // Postgres server port
-  path        : '',         // unix socket path (usually '/tmp')
-  database    : '',         // Name of database to connect to
-  username    : '',         // Username of database user
-  password    : '',         // Password of database user
-  ssl         : false,      // True, or options for tls.connect
-  max         : 10,         // Max number of connections
-  idle_timeout: 0,          // Idle connection timeout in seconds
-  types       : [],         // Array of custom types, see more below
-  onnotice    : fn          // Defaults to console.log
-  onparameter : fn          // (key, value) when server param change
-  debug       : fn          // Is called with (connection, query, parameters)
-  transform   : {
+  host            : '',         // Postgres ip address or domain name
+  port            : 5432,       // Postgres server port
+  path            : '',         // unix socket path (usually '/tmp')
+  database        : '',         // Name of database to connect to
+  username        : '',         // Username of database user
+  password        : '',         // Password of database user
+  ssl             : false,      // True, or options for tls.connect
+  max             : 10,         // Max number of connections
+  idle_timeout    : 0,          // Idle connection timeout in seconds
+  connect_timeout : 30,         // Connect timeout in seconds
+  types           : [],         // Array of custom types, see more below
+  onnotice        : fn          // Defaults to console.log
+  onparameter     : fn          // (key, value) when server param change
+  debug           : fn          // Is called with (connection, query, params)
+  transform       : {
     column            : fn, // Transforms incoming column names
     value             : fn, // Transforms incoming row values
     row               : fn  // Transforms entire rows
   },
-  connection  : {
+  connection      : {
     application_name  : 'postgres.js', // Default application_name
     ...                                // Other connection parameters
   }
@@ -578,6 +579,11 @@ This error is thrown if the user has called [`sql.end()`](#sql_end) and performe
 
 This error is thrown for any queries that were pending when the timeout to [`sql.end({ timeout: X })`](#sql_destroy) was reached.
 
+##### CONNECTION_CONNECT_TIMEOUT
+> write CONNECTION_CONNECT_TIMEOUT host:port
+
+This error is thrown if the startup phase of the connection (tcp, protocol negotiation and auth) took more than the default 30 seconds or what was specified using `connect_timeout` or `PGCONNECT_TIMEOUT`.
+
 ## Migration tools
 
 Postgres.js doesn't come with any migration solution since it's way out of scope, but here are some modules that supports Postgres.js for migrations:
diff --git a/lib/connection.js b/lib/connection.js
@@ -14,6 +14,7 @@ function Connection(options = {}) {
     onparameter,
     transform,
     idle_timeout,
+    connect_timeout,
     onnotify,
     onnotice,
     parsers
@@ -27,6 +28,7 @@ function Connection(options = {}) {
   let open = false
   let ready = false
   let statements = {}
+  let connect_timer
 
   const queries = Queue()
       , id = count++
@@ -128,7 +130,17 @@ function Connection(options = {}) {
 
     open
       ? socket.write(buffer)
-      : (messages.push(buffer), socket.connect())
+      : (messages.push(buffer), connect())
+  }
+
+  function connect() {
+    connect_timeout && (connect_timer = setTimeout(connectTimedOut, connect_timeout * 1000))
+    socket.connect()
+  }
+
+  function connectTimedOut() {
+    error(errors.connection('CONNECT_TIMEOUT', options))
+    socket.destroy()
   }
 
   function simple(str, query) {
@@ -161,6 +173,7 @@ function Connection(options = {}) {
   }
 
   function onready(err) {
+    connect_timer && (clearTimeout(connect_timer), connect_timer = null)
     if (err) {
       if (backend.query) {
         err.stack += backend.query.origin.replace(/.*\n/, '\n')
@@ -210,6 +223,7 @@ function Connection(options = {}) {
   }
 
   function close() {
+    connect_timer && (clearTimeout(connect_timer), connect_timer = null)
     error(errors.connection('CLOSED', options))
     statements = {}
     messages = []
diff --git a/lib/index.js b/lib/index.js
@@ -490,19 +490,20 @@ function parseOptions(a, b) {
   return Object.assign({
     host,
     port,
-    path        : o.path || host.indexOf('/') > -1 && host + '/.s.PGSQL.' + port,
-    database    : o.database || o.db || (url.pathname || '').slice(1) || env.PGDATABASE || 'postgres',
-    user        : o.user || o.username || auth[0] || env.PGUSERNAME || env.PGUSER || osUsername(),
-    pass        : o.pass || o.password || auth[1] || env.PGPASSWORD || '',
-    max         : o.max || url.query.max || 10,
-    types       : o.types || {},
-    ssl         : o.ssl || url.ssl || false,
-    idle_timeout: o.idle_timeout || warn(o.timeout, 'The timeout option is deprecated, use idle_timeout instead'),
-    onnotice    : o.onnotice,
-    onparameter : o.onparameter,
-    transform   : Object.assign({}, o.transform),
+    path            : o.path || host.indexOf('/') > -1 && host + '/.s.PGSQL.' + port,
+    database        : o.database || o.db || (url.pathname || '').slice(1) || env.PGDATABASE || 'postgres',
+    user            : o.user || o.username || auth[0] || env.PGUSERNAME || env.PGUSER || osUsername(),
+    pass            : o.pass || o.password || auth[1] || env.PGPASSWORD || '',
+    max             : o.max || url.query.max || 10,
+    types           : o.types || {},
+    ssl             : o.ssl || url.ssl || false,
+    idle_timeout    : o.idle_timeout || url.query.idle_timeout || env.PGIDLE_TIMEOUT || warn(o.timeout, 'The timeout option is deprecated, use idle_timeout instead'),
+    connect_timeout : o.connect_timeout || url.query.connect_timeout || env.PGCONNECT_TIMEOUT || 30,
+    onnotice        : o.onnotice,
+    onparameter     : o.onparameter,
+    transform       : Object.assign({}, o.transform),
     connection  : Object.assign({ application_name: 'postgres.js' }, o.connection),
-    debug       : o.debug
+    debug           : o.debug
   },
     mergeUserTypes(o.types)
   )
diff --git a/tests/index.js b/tests/index.js
@@ -937,3 +937,31 @@ t('Query parameters are not enumerable', async() => [
   -1,
   (await sql`selec ${ 1 }`.catch(err => Object.keys(err).indexOf('parameters')))
 ])
+
+t('connect_timeout throws proper error', async() => [
+  'CONNECTION_CONNECT_TIMEOUT',
+  await postgres({
+    ...options,
+    ...login_scram,
+    connect_timeout: 0.001
+  })`select 1`.catch(e => e.code)
+])
+
+t('requests works after single connect_timeout', async() => {
+  let first = true
+
+  const sql = postgres({
+    ...options,
+    ...login_scram,
+    connect_timeout: { valueOf() { return first ? (first = false, 0.001) : 1 } }
+  })
+
+  return [
+    'CONNECTION_CONNECT_TIMEOUT,,1',
+    [
+      await sql`select 1 as x`.catch(x => x.code),
+      await new Promise(r => setTimeout(r, 10)),
+      (await sql`select 1 as x`)[0].x
+    ].join(',')
+  ]
+})
