Merge pull request #1849 from dhensby/fix/ps-execute-stream

fix: prevent TypeError in PreparedStatement.execute() when streaming without callback

Author: dhensby

lib/base/prepared-statement.js

@@ -356,11 +356,10 @@ class PreparedStatement extends EventEmitter {
       }
     }
 
-    req.execute('sp_execute', (err, result) => {
-      if (err) return callback(err)
-
-      callback(null, result)
-    })
+    req.execute('sp_execute', typeof callback === 'function'
+      ? callback
+      : () => {}
+    )
 
     return req
   }

test/common/tests.js

@@ -922,6 +922,26 @@ module.exports = (sql, driver) => {
       }).catch(done)
     },
 
+    'prepared statement with streaming and no callback' (done) {
+      const ps = new TestPreparedStatement()
+      ps.stream = true
+      ps.input('num', sql.Int)
+      ps.prepare('select @num as number').then(() => {
+        const req = ps.execute({ num: 123 })
+        const rows = []
+
+        req.on('row', row => rows.push(row))
+        req.on('error', err => {
+          ps.unprepare(() => done(err))
+        })
+        req.on('done', () => {
+          assert.strictEqual(rows.length, 1)
+          assert.strictEqual(rows[0].number, 123)
+          ps.unprepare(done)
+        })
+      }).catch(done)
+    },
+
     'transaction with rollback' (done) {
       let tbegin = false
       let tcommit = false

test/common/unit.js

@@ -1402,4 +1402,24 @@ describe('connection string auth - tedious', () => {
       })
     })
   })
+
+  describe('PreparedStatement streaming', () => {
+    const BasePreparedStatement = require('../../lib/base/prepared-statement')
+    const ConnectionPool = require('../../lib/tedious/connection-pool')
+
+    it('execute does not throw when stream=true and no callback provided', (done) => {
+      const pool = new ConnectionPool({ server: 'localhost' })
+      const ps = new BasePreparedStatement(pool)
+      ps.stream = true
+      ps.prepared = true
+      ps._handle = 1
+
+      // Should not throw TypeError — returns the inner Request
+      const req = ps.execute({})
+      assert.ok(req, 'execute() should return the inner Request')
+      // The request will error (no connection) — that's fine,
+      // the point is it doesn't throw TypeError
+      req.on('error', () => done())
+    })
+  })
 })

test/msnodesqlv8/msnodesqlv8.js

@@ -82,6 +82,7 @@ describe('msnodesqlv8', function () {
     it('prepared statement with affected rows', done => TESTS['prepared statement with affected rows'](done))
     it('prepared statement in transaction', done => TESTS['prepared statement in transaction'](done))
     it('prepared statement with duplicate output column names', done => TESTS['prepared statement with duplicate output column names'](done))
+    it('prepared statement with streaming and no callback', done => TESTS['prepared statement with streaming and no callback'](done))
     it('transaction with rollback', done => TESTS['transaction with rollback'](done))
     it('transaction with commit', done => TESTS['transaction with commit'](done))
     it('transaction throws on bad isolation level', done => TESTS['transaction throws on bad isolation level'](done))

test/tedious/tedious.js

@@ -89,6 +89,7 @@ describe('tedious', () => {
     it('prepared statement with affected rows', done => TESTS['prepared statement with affected rows'](done))
     it('prepared statement in transaction', done => TESTS['prepared statement in transaction'](done))
     it('prepared statement with duplicate output column names', done => TESTS['prepared statement with duplicate output column names'](done))
+    it('prepared statement with streaming and no callback', done => TESTS['prepared statement with streaming and no callback'](done))
     it('transaction with rollback', done => TESTS['transaction with rollback'](done))
     it('transaction with commit', done => TESTS['transaction with commit'](done))
     it('transaction throws on bad isolation level', done => TESTS['transaction throws on bad isolation level'](done))