fix: settle msnodesqlv8 bulk loads when the table lookup fails

lib/msnodesqlv8/request.js

@@ -245,7 +245,12 @@ class Request extends BaseRequest {
 
           const go = () => {
             const tm = connection.tableMgr()
-            return tm.bind(table.path.replace(/\[|\]/g, ''), mgr => {
+            // the table manager reports a failed lookup as (null, err), not error first
+            return tm.bind(table.path.replace(/\[|\]/g, ''), (mgr, err) => {
+              if (err) {
+                return done(err)
+              }
+
               if (mgr.columns.length === 0) {
                 return done(new RequestError('Table was not found on the server.', 'ENAME'))
               }

test/common/tests.js

@@ -1275,6 +1275,39 @@ module.exports = (sql, driver) => {
       })
     },
 
+    'bulk load releases the connection when the table lookup fails' (name, done) {
+      // without create, the table is looked up on the borrowed connection before any row is
+      // sent. A database that does not exist makes that lookup fail outright, rather than
+      // find no table, and the borrow has to come back with the error. The name is used by
+      // no other test, so nothing the driver remembers about it can reach another load.
+      const t = new sql.Table(`notexistingdb.dbo.${name}`)
+      t.create = false
+      t.columns.add('a', sql.Int, { nullable: true })
+      t.rows.add(1)
+
+      const req = new TestRequest()
+      const pool = req.parent
+      const free = () => pool.pool.numFree()
+      const before = free()
+
+      req.bulk(t).then(() => {
+        done(new Error('bulk() should reject a table in a database that does not exist'))
+      }, err => {
+        try {
+          assert.strictEqual(pool.pool.numUsed(), 0, 'the borrowed connection should have been released')
+          assert.strictEqual(free(), before, 'the borrowed connection should have gone back to the pool')
+          assert.ok(err instanceof sql.RequestError, 'the rejection should be a RequestError')
+          assert.strictEqual(err.code, 'EREQUEST', `the rejection should be a request error, got ${err.code}`)
+        } catch (e) {
+          return done(e)
+        }
+        new sql.Request().query('select 1 as v').then(result => {
+          assert.strictEqual(result.recordset[0].v, 1, 'the pool should still serve queries afterwards')
+          done()
+        }).catch(done)
+      })
+    },
+
     'bulk load with varchar-max field' (name, done) {
       const t = new sql.Table(name)
       t.create = true

test/msnodesqlv8/msnodesqlv8.js

@@ -194,6 +194,7 @@ describe('msnodesqlv8', function () {
     it('bulk converts dates', done => TESTS['bulk converts dates'](done))
     it('bulk load rejects unsafe column names without leaking a connection', done => TESTS['bulk load rejects unsafe column names without leaking a connection']('bulk_table_unsafe', done))
     it('bulk load releases the connection when a column name changes after the check', done => TESTS['bulk load releases the connection when a column name changes after the check']('bulk_table_mutated', done))
+    it('bulk load releases the connection when the table lookup fails', done => TESTS['bulk load releases the connection when the table lookup fails']('bulk_table_lookup', done))
     it('bulk load into an existing table rejects an unsafe column name', done => TESTS['bulk load into an existing table rejects an unsafe column name']('bulk_table', done))
     it.skip('bulk insert of a value that is not a string into a string column throws (table) (not supported by msnodesqlv8)', done => TESTS['bulk insert of a value that is not a string into a string column throws']('bulk_table3', done))
     it('bulk insert with length option as string other than max throws (table)', done => TESTS['bulk insert with length option as string other than max throws']('bulk_table4', done))

test/tedious/tedious.js

@@ -190,6 +190,7 @@ describe('tedious', () => {
     it('bulk converts dates', done => TESTS['bulk converts dates'](done))
     it('bulk load rejects unsafe column names without leaking a connection', done => TESTS['bulk load rejects unsafe column names without leaking a connection']('bulk_table_unsafe', done))
     it('bulk load releases the connection when a column name changes after the check', done => TESTS['bulk load releases the connection when a column name changes after the check']('bulk_table_mutated', done))
+    it('bulk load releases the connection when the table lookup fails', done => TESTS['bulk load releases the connection when the table lookup fails']('bulk_table_lookup', done))
     it('bulk load into an existing table rejects an unsafe column name', done => TESTS['bulk load into an existing table rejects an unsafe column name']('bulk_table', done))
     it('bulk insert of a value that is not a string into a string column throws (table)', done => TESTS['bulk insert of a value that is not a string into a string column throws']('bulk_table3', done))
     it('bulk insert with length option as string other than max throws (table)', done => TESTS['bulk insert with length option as string other than max throws']('bulk_table4', done))