[CONJS-170] Pool.query/execute/batch(undefined) never release connect…

…ion #163 pool wasn't releasing connection An error must be thrown immediately, and never stall a connection
mariadb-corporation · Sep 15, 2021 · caf783c · caf783c
1 parent 677a465
commit caf783c
Show file tree

Hide file tree

Showing 3 changed files with 84 additions and 58 deletions.
diff --git a/lib/misc/errors.js b/lib/misc/errors.js
@@ -109,6 +109,7 @@ module.exports.ER_PING_TIMEOUT = 45042;
 module.exports.ER_BAD_PARAMETER_VALUE = 45043;
 module.exports.ER_CANNOT_RETRIEVE_RSA_KEY = 45044;
 module.exports.ER_MINIMUM_NODE_VERSION_REQUIRED = 45045;
+module.exports.ER_POOL_UNDEFINED_SQL = 45049;
 
 const keys = Object.keys(module.exports);
 const errByNo = {};

diff --git a/lib/pool-base.js b/lib/pool-base.js
@@ -207,6 +207,22 @@ function PoolBase(options, processTask, createConnectionPool, pingPromise) {
    * @return {*}
    */
   const addRequest = function (pool, sql, values, isBatch) {
+    if (isBatch != undefined && !sql) {
+      // request for query/batch without sql
+      return Promise.reject(
+          Errors.createError(
+              'sql parameter is mandatory',
+              null,
+              false,
+              null,
+              'HY000',
+              Errors.ER_POOL_UNDEFINED_SQL,
+              undefined,
+              false
+          )
+      );
+    }
+
     if (closed) {
       return Promise.reject(
         Errors.createError(

diff --git a/test/integration/test-pool.js b/test/integration/test-pool.js
@@ -108,6 +108,36 @@ describe('Pool', () => {
     pool.end();
   });
 
+  it('undefined query', async function () {
+    const pool = base.createPool({ connectionLimit: 1 });
+    try {
+      await pool.query(undefined);
+      throw new Error('must have thrown an error');
+    } catch (err) {
+      assert(err.message.includes('sql parameter is mandatory'));
+      assert.equal(err.sqlState, 'HY000');
+      assert.equal(err.errno, 45049);
+      assert.equal(err.code, 'ER_POOL_UNDEFINED_SQL');
+    } finally {
+      await pool.end();
+    }
+  });
+
+  it('undefined batch', async function () {
+    const pool = base.createPool({ connectionLimit: 1 });
+    try {
+      await pool.batch(undefined);
+      throw new Error('must have thrown an error');
+    } catch (err) {
+      assert(err.message.includes('sql parameter is mandatory'));
+      assert.equal(err.sqlState, 'HY000');
+      assert.equal(err.errno, 45049);
+      assert.equal(err.code, 'ER_POOL_UNDEFINED_SQL');
+    } finally {
+      await pool.end();
+    }
+  });
+
   it('query with null placeholder', async function () {
     const pool = base.createPool({ connectionLimit: 1 });
     let rows = await pool.query('select ? as a', [null]);
@@ -185,72 +215,51 @@ describe('Pool', () => {
       });
   });
 
-  it('pool with wrong authentication connection', function (done) {
+  it('pool with wrong authentication connection', async function () {
     if (
       process.env.srv === 'maxscale' ||
       process.env.srv === 'skysql' ||
       process.env.srv === 'skysql-ha'
     )
       this.skip();
     this.timeout(10000);
-    const pool = base.createPool({
-      acquireTimeout: 4000,
-      initializationTimeout: 2000,
-      user: 'wrongAuthentication'
-    });
-    pool
-      .getConnection()
-      .then(() => {
-        pool.end();
-        done(new Error('must have thrown error'));
-      })
-      .catch((err) => {
-        assert.isTrue(
-          err.errno === 1524 ||
-            err.errno === 1045 ||
-            err.errno === 1698 ||
-            err.errno === 45028 ||
-            err.errno === 45025 ||
-            err.errno === 45044,
-          err.message
-        );
-        pool
-          .getConnection()
-          .then(() => {
-            pool.end();
-            done(new Error('must have thrown error'));
-          })
-          .catch((err) => {
-            pool.end();
-            assert.isTrue(
-              err.errno === 1524 ||
-                err.errno === 1045 ||
-                err.errno === 1698 ||
-                err.errno === 45028 ||
-                err.errno === 45025 ||
-                err.errno === 45044,
-              err.message
-            );
-            done();
-          });
-      });
-    pool
-      .getConnection()
-      .then(() => {
-        pool.end();
-        done(new Error('must have thrown error'));
-      })
-      .catch((err) => {
-        assert.isTrue(
-          err.errno === 1524 ||
-            err.errno === 1045 ||
-            err.errno === 1698 ||
-            err.errno === 45028 ||
-            err.errno === 45025 ||
-            err.errno === 45044,
-          err.message
-        );
+    let err;
+    let pool;
+    try {
+      pool = base.createPool({
+        acquireTimeout: 4000,
+        initializationTimeout: 2000,
+        user: 'wrongAuthentication'
       });
+      await pool.getConnection();
+      throw new Error('must have thrown error');
+    } catch (err) {
+      assert.isTrue(
+        err.errno === 1524 ||
+          err.errno === 1045 ||
+          err.errno === 1698 ||
+          err.errno === 45028 ||
+          err.errno === 45025 ||
+          err.errno === 45044,
+        err.message
+      );
+    }
+    try {
+      await pool.getConnection();
+      throw new Error('must have thrown error');
+    } catch (err) {
+      assert.isTrue(
+        err.errno === 1524 ||
+          err.errno === 1045 ||
+          err.errno === 1698 ||
+          err.errno === 45028 ||
+          err.errno === 45025 ||
+          err.errno === 45044,
+        err.message
+      );
+    } finally {
+      pool.end();
+    }
   });
 
   it('create pool', async function () {