fix: make sure recycled connections is truly idle

chore: typo

revert: drop Driver#recycleConnections due to poor interoperability

package.json

@@ -62,7 +62,7 @@
     "jsdoc": "^3.6.3",
     "mocha": "^8.2.1",
     "mysql": "^2.17.1",
-    "mysql2": "^1.7.0",
+    "mysql2": "^2.3.0",
     "nyc": "^15.1.0",
     "pg": "^8.5.1",
     "sinon": "^10.0.0",

src/drivers/abstract/index.js

@@ -16,31 +16,6 @@ module.exports = class AbstractDriver {
     this.options = opts;
   }
 
-  closeConnection(connection) {
-    throw new Error('not implemented');
-  }
-
-  recycleConnections() {
-    const acquiredAt = new Map();
-    const timeout = this.idleTimeout * 1000;
-
-    this.pool.on('acquire', function onAcquire(connection) {
-      acquiredAt.set(connection, Date.now());
-    });
-
-    const checkIdle = () => {
-      const now = Date.now();
-      for (const [ connection, timestamp ] of acquiredAt) {
-        if (now - timestamp > timeout) {
-          this.closeConnection(connection);
-          acquiredAt.delete(connection);
-        }
-      }
-      setTimeout(checkIdle, timeout);
-    };
-    checkIdle();
-  }
-
   /**
    * Cast raw values from database to JavaScript types. When the raw packet is fetched from database, `Date`s and special numbers are transformed by drivers already. This method is used to cast said values to custom types set by {@link Bone.attribute}, such as `JSON`.
    * @private

src/drivers/mysql/index.js

@@ -25,7 +25,6 @@ class MysqlDriver extends AbstractDriver {
     this.type = 'mysql';
     this.pool = this.createPool(opts);
     this.escape = this.pool.escape.bind(this.pool);
-    this.recycleConnections();
   }
 
   get escapeId() {
@@ -69,11 +68,6 @@ class MysqlDriver extends AbstractDriver {
     });
   }
 
-  closeConnection(connection) {
-    connection.release();
-    connection.destroy();
-  }
-
   async query(query, values, opts = {}) {
     const { logger } = this;
     const connection = opts.connection || await this.getConnection();

src/drivers/postgres/index.js

@@ -109,7 +109,6 @@ class PostgresDriver extends AbstractDriver {
     super(opts);
     this.type = 'postgres';
     this.pool = this.createPool(opts);
-    this.recycleConnections();
   }
 
   createPool(opts) {
@@ -121,11 +120,6 @@ class PostgresDriver extends AbstractDriver {
     return await this.pool.connect();
   }
 
-  async closeConnection(client) {
-    client.release();
-    await client.end();
-  }
-
   async query(query, values, spell = {}) {
     const { sql, nestTables } = typeof query === 'string' ? { sql: query } : query;
     const { text } = parameterize(sql, values);

src/drivers/sqlite/index.js

@@ -75,7 +75,7 @@ class Connection {
     this.pool.releaseConnection(this);
   }
 
-  async destroy() {
+  async end() {
     const { connections } = this.pool;
     const index = connections.indexOf(this);
     if (index >= 0) connections.splice(index, 1);
@@ -138,7 +138,6 @@ class SqliteDriver extends AbstractDriver {
     super(opts);
     this.type = 'sqlite';
     this.pool = this.createPool(opts);
-    this.recycleConnections();
   }
 
   createPool(opts) {
@@ -149,11 +148,6 @@ class SqliteDriver extends AbstractDriver {
     return await this.pool.getConnection();
   }
 
-  async closeConnection(connection) {
-    connection.release();
-    await connection.destroy();
-  }
-
   async query(query, values, opts = {}) {
     const connection = opts.connection || await this.getConnection();
 

test/unit/drivers/abstract/index.test.js

@@ -1,6 +1,5 @@
 'use strict';
 
-const EventEmitter = require('events');
 const assert = require('assert').strict;
 const dayjs = require('dayjs');
 const { Logger } = require('../../../..');
@@ -56,23 +55,3 @@ describe('=> AbstractDriver#logger', function() {
     assert.ok(driver.logger instanceof CustomLogger);
   });
 });
-
-describe('=> AbstractDriver#recycleConnections', function() {
-  it('should close idle connections', async function() {
-    const driver = new AbstractDriver({ idleTimeout: 0.01 });
-    driver.pool = new EventEmitter();
-    let released;
-    let destroyed;
-    driver.recycleConnections();
-    driver.closeConnection = function() {
-      released = true;
-      destroyed = true;
-    };
-    driver.pool.emit('acquire', {});
-    assert.ok(!released);
-    assert.ok(!destroyed);
-    await new Promise(resolve => setTimeout(resolve, 30));
-    assert.ok(released);
-    assert.ok(destroyed);
-  });
-});

test/unit/drivers/mysql/index.test.js

@@ -74,33 +74,4 @@ describe('=> MySQL driver', () => {
     await driver.truncateTable('notes');
     assert.equal((await driver.query('SELECT count(*) AS count FROM notes')).rows[0].count, 0);
   });
-
-  it('driver.recycleConnections()', async function() {
-    const driver2 = new MysqlDriver({
-      ...options,
-      idleTimeout: 0.01,
-    });
-    let released;
-    driver2.pool.on('release', function() {
-      released = true;
-    });
-    const connection = await driver2.getConnection();
-    await new Promise(function(resolve, reject) {
-      connection.query('SELECT 1', function(err, row) {
-        if (err) reject(err);
-        resolve({ row });
-      });
-    });
-    assert.ok(!released);
-    await new Promise(resolve => setTimeout(resolve, 30));
-    assert.ok(released);
-    await assert.rejects(async function() {
-      await new Promise(function(resolve, reject) {
-        connection.query('SELECT 1', function(err, row) {
-          if (err) reject(err);
-          resolve({ row });
-        });
-      });
-    }, /Error: Cannot enqueue Query after being destroyed./);
-  });
 });

test/unit/drivers/postgres/index.test.js

@@ -111,17 +111,4 @@ describe('=> PostgreSQL driver', () => {
     await driver.query(`INSERT INTO notes (title) VALUES ('Untitled')`);
     assert.equal((await driver.query('SELECT id FROM notes')).rows[0].id, '1');
   });
-
-  it('driver.recycleConnections()', async function() {
-    const driver2 = new PostgresDriver({
-      ...options,
-      idleTimeout: 0.01,
-    });
-    const connection = await driver2.getConnection();
-    await connection.query('SELECT 1');
-    await new Promise(resolve => setTimeout(resolve, 30));
-    await assert.rejects(async function() {
-      await connection.query('SELECT 1');
-    }, /Error: Client was closed and is not queryable/);
-  });
 });

test/unit/drivers/sqlite/index.test.js

@@ -187,25 +187,4 @@ describe('=> SQLite driver.pool', function() {
     ]);
     assert.equal(driver2.pool.connections.length, 1);
   });
-
-  it('driver.recycleConnections()', async function() {
-    const driver2 = new SqliteDriver({
-      ...options,
-      idleTimeout: 0.01,
-    });
-    const connection = await driver2.getConnection();
-    await connection.query('SELECT 1');
-    await new Promise(resolve => setTimeout(resolve, 30));
-    await assert.rejects(async function() {
-      await connection.query('SELECT 1');
-    }, /Error: SQLITE_MISUSE: Database is closed/);
-
-    // should remove connection from pool when destroy
-    assert.equal(driver2.pool.connections.length, 0);
-
-    // should still be able to create new connection
-    await assert.doesNotReject(async function() {
-      await driver2.query('SELECT 1');
-    });
-  });
 });