Emitting `done` after transaction failing

I'm working with mssql lib
"msnodesqlv8": "2.0.6",
"mssql": "6.2.3",

I get transaction is getting stuck if it fails - on foreign key constraint for example
In tediousjs/node-mssql#77 thread I saw it can happen because of missing _this.emit('done') call after _this.emit('error', e)
How can I ensure it happens correctly?

if you update to latest version and in your local version change reader.js end function to include emitDone - this is called when an error is found in work flow when reading the results of the cpp driver.

does this help?

      function end (err) {
        invokeObject.end(notify, outputParams, () => {
          if (!Array.isArray(err)) {
            err = [err]
          }
          routeStatementError(err, callback, notify)
        }, null, false)
        emitDone()
        close()
      }

I'll try, thanks!
For the record:

node-sqlserver-v8/lib/reader.js

Line 204 in 03a0a2c

function end (err) {

Ok, it didn't help - the transaction just continues and commits, without failing or rollbacking
@TimelordUK , do you an idea how we can fail the mssql transaction?

does it make any difference if done is moved to after the raise error call

  function end (err) {
        invokeObject.end(notify, outputParams, () => {
          if (!Array.isArray(err)) {
            err = [err]
          }
          routeStatementError(err, callback, notify)
          emitDone()
        }, null, false)
        // emitDone()
        close()
      }

have you got an actual simple example to reproduce issue I have no idea how mssql is handling transactions I guess would have to debug code to see what they are looking for to rollback. - what is trigger their side?

const sql = require('mssql')
(async () => {
    try {
        await sql.connect('mssql://username:password@localhost/database')
        const result = await sql.query`insert into purchases (user_id) values (-1)` // lets say client_id is identuty - natural number, forbidden by previously created foreign key to  
        // hangs 
    } catch (err) {
        // nothing here
    }
})()

hello,

OK I have finally got to look into this issue, thanks for your patience.

There is a bug in this library when dispatching errors that shows up in this condition - namely the driver receives 2 errors from the ODBC, one is treated as info the other an actual error. This means when using the stream based api you are told more are to follow, when actually only one error event is raised and the other is an info event.

In any case I have corrected this and added the following test case

note the error / info management is now

        const q = theConnection.query(insertSql, [1, 'sprinting'])
        const errors = []
        q.on('error', (err, more) => {
          errors.push(err)
          if (more) return
          const msgs = errors.map(e => e.message)
          assert.deepStrictEqual(1, msgs.length)
          assert(msgs[0].includes('Violation of PRIMARY KEY'))
        })

        q.on('info', i => {
          const msg = i.message
          assert(msg.includes('statement has been terminated'))
        })

and in this case the 'free' event is raised by native library when the resources for the statement are unallocated.

I intend to release shortly hopefully this will help the mssql wrapper handle the situation better.

test('begin a transaction and use streaming with error on constraint to trigger rollback detection', done => {
    const tableName = 'testsales'
    const dropTableSql = `IF OBJECT_ID('${tableName}', 'U') IS NOT NULL 
  DROP TABLE ${tableName};`

    const createTableSql = `CREATE TABLE ${tableName} (
      activity_id INT PRIMARY KEY,
      activity_name VARCHAR (255) NOT NULL
  );`

    const insertSql = `INSERT INTO ${tableName} (activity_id, activity_name) VALUES (?, ?)`
    const selectSql = `select activity_id, activity_name from ${tableName}`

    const fns = [

      asyncDone => {
        theConnection.query(dropTableSql, (err) => {
          assert.ifError(err)
          asyncDone()
        })
      },

      asyncDone => {
        theConnection.query(createTableSql, (err) => {
          assert.ifError(err)
          asyncDone()
        })
      },

      asyncDone => {
        theConnection.beginTransaction(err => {
          assert.ifError(err)
          asyncDone()
        })
      },

      asyncDone => {
        theConnection.query(insertSql, [1, 'jogging'], (err) => {
          assert.ifError(err)
          asyncDone()
        })
      },

      asyncDone => {
        const q = theConnection.query(insertSql, [1, 'sprinting'])
        const errors = []
        q.on('error', (err, more) => {
          errors.push(err)
          if (more) return
          const msgs = errors.map(e => e.message)
          assert.deepStrictEqual(1, msgs.length)
          assert(msgs[0].includes('Violation of PRIMARY KEY'))
        })

        q.on('info', i => {
          const msg = i.message
          assert(msg.includes('statement has been terminated'))
        })

        q.on('free', () => {
          theConnection.rollback(err => {
            assert.ifError(err)
            asyncDone()
          })
        })
      },

      asyncDone => {
        theConnection.queryRaw(selectSql, (err, results) => {
          assert.ifError(err)
          assert.deepStrictEqual(results.rows, [])
          asyncDone()
        })
      }
    ]
    async.series(fns, () => {
      done()
    })
  })

Oh, thank you, it will help a lot!

now released as 2.0.9