Node-mssql: msnodesqlv8 bug: Connection is busy with results for another command

[davazp]

I have found an issue when executing a stored procedure which raises an error concurrently using the msnodesqlv8 driver. Actually, it happens if the procedure does not exist as well.

I am not sure if it is a bug in mssql or in msnodesqlv8 itself. I report it here as my code is not using msnodesqlv8 directly. The problem is with the versions

mssql: 3.3.0
msnodesqlv8: 0.2.x (I have tried with the latest and many other releases)

However, it works with msnodesqlv8 0.1.46. So it seems that it was introduced in the 0.2.x releases.

I have narrowed the problem to the following minimal code:

'use strict';

const sql = require('mssql');

sql.connect({
  driver: 'msnodesqlv8',
  server: 'localhost',
  database: 'master',
  options: { trustedConnection: true }
}).then(db=>{
  let array = [];

  for (let i=0; i<100; i++){
    array.push(i);
  }

  return Promise.all(array.map(i=>{
    let request = db.request();
    // test should raise an error, or just don't exist at all
    return request.execute('test').catch(err=>{
      console.log(err);
    });
  }));

}).then(()=>{
  process.exit(0);
}).catch(err=>{
  console.log(err);
  process.exit(-1);
});

When it works, you expect the errors to be like

{ [RequestError: error]
  name: 'RequestError',
  message: 'error',
  code: 'EREQUEST',
  number: 50000,
  lineNumber: undefined,
  state: '42000',
  class: undefined,
  serverName: undefined,
  procName: undefined }

(or whatever error the procedure throws)

However, with the latest version, I get many instances of the following error:

{ [RequestError: Connection is busy with results for another command]
  name: 'RequestError',
  message: 'Connection is busy with results for another command',
  code: 'EREQUEST',
  number: 0,
  lineNumber: undefined,
  state: 'HY000',
  class: undefined,
  serverName: undefined,
  procName: undefined }

I do not know if this this could affect to concurrent operations to the faulty one that do not raise any errors.

[patriksimek]

It seems more like a msnodesqlv8 related problem. I will do some tests later. Meantime, you should post this to msnodesqlv8 issues.

[TimelordUK]

hi guys. I had a go last night at reproducing this issue and so far have not been able to do so.

It would be very useful to have a test that demonstrates the problem directly with msnodesqlv8.

I include my test code below - which is pretty much what you have but running directly.

https://github.com/patriksimek/node-mssql/issues/414

however, if we cannot do that i shall try to debug as you are running it i.e. in node-mssql.

The c++ driver does have to hang on for output and return parameters before submitting again so it is possible the next invocation is being sent in too early but I was expecting my test in the link to reproduce the exact same problem unless I am not understanding correctly?

[davazp]

@TimelordUK I think that is the wrong link for your demo. I can give it a try as well, but your demo would be a good starting point.

[TimelordUK]

my apologies yes the snippet i wrote last night is in your issue raised with msnodesqlv8

https://github.com/TimelordUK/node-sqlserver-v8/issues/34

the easiest thing may be for me to just check the test in to trunk but it may be enough for you to start testing. I basically added this to

https://github.com/TimelordUK/node-sqlserver-v8/blob/master/DriverModuleTest.ts

and ran the transpiled code through node.

Hopefully we can track it down soon.

the latest version including this test is now checked into trunk :-

https://github.com/TimelordUK/node-sqlserver-v8/blob/master/DriverModuleTest.js

[ratanaklun]

I've been receiving this error too, but with just normal queries. I think it may be related to multiple statements and errors ocurring within them. Maybe to do with multiple results?

This is what I'm using to reproduce the problem:

let query = `
RAISERROR(`Error`, 14, 1);
SELECT 1+1;
`;

conn.queryRaw(query, (err, results) => {
  console.log({ error: err, results: results });
  conn.queryRaw(query, (e, r) => {
    console.log({ error: e, results: r });
  });
});

The second query produces a Connection is busy with results for another command error.

Removing the SELECT statement works as expected showing that it's not necessarily an error result alone that causes the problem.

[TimelordUK]

Do you see this issue running direct on msnodesqlv8 or via node-msssql?

Sent from my iPad

On 8 Mar 2017, at 19:58, Ratanak Lun [e-mail] wrote:

I've been receiving this error too, but with just normal queries. I think it may be related to multiple statements and errors ocurring within them. Maybe to do with multiple results?

This is what I'm using to reproduce the problem:

let query = RAISERROR(Error, 14, 1); SELECT 1+1; ;

conn.queryRaw(query, (err, results) => {
console.log({ error: err, results: results });
conn.queryRaw(query, (e, r) => {
console.log({ error: e, results: r });
});
});
The second query produces a Connection is busy with results for another command error.

Removing the SELECT statement works as expected showing that it's not necessarily an error result alone that causes the problem.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

[ratanaklun]

This is using msnodesqlv8 directly.

[TimelordUK]

Ok thanks I'll run your test and see if I can reproduce the issue.

Sent from my iPad

On 8 Mar 2017, at 20:05, Ratanak Lun [e-mail] wrote:

This is using msnodesqlv8 directly.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

[TimelordUK]

Out of interest if you add more Boolean at end of callback and only call next when more is false does this then work ? As in add another parameter and check that status.

Sent from my iPad

On 8 Mar 2017, at 20:05, Ratanak Lun [e-mail] wrote:

This is using msnodesqlv8 directly.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

[ratanaklun]

The 3rd argument to the callback is undefined. Also sorry, didn't realize this was node-mssql repo.

[TimelordUK]

can you try a patch to driverMgr.js around line 470, the onInvoke. I was not cleaning up the statement. Please note that your test will now not produce the busy connection error however it looks to be a much more complex fix to get the driver to go on and read the results of the select. The statement seems to terminated at the point the error is raised. I strongly suspect this is how the driver has worked, this error crept in when i was working on some memory corruption.

I will have to extensively test the driver before releasing this fix, I would appreciate if you could let me know if this helps.

-- the only change you should need is inside the if err { } block.

function onInvoke(err, results, params) {

        outputParams = params;

        if (err) {
            invokeObject.end(queryId, outputParams, function() {
                routeStatementError(err, callback, notify);
            }, results, false);
            nextOp();
            return;
        }

        meta = results;
        if (meta.length > 0) {
            notify.emit('meta', meta);
            //noinspection JSUnresolvedFunction
            native.readRow(queryId, onReadRow);
        }
        else {
            //noinspection JSUnresolvedFunction
            native.nextResult(queryId, onNextResult)
        }
    }

[ratanaklun]

I tried your suggested change and it is not producing the busy error anymore. Looks like this is on its way to being fixed. Thanks!

[TimelordUK]

I have released 02.16 which should now fix this issue. Thanks for bringing to my attention and for the test code.

[davazp]

This worked for me as well 👍 @patriksimek, I think you can close it now.
Thanks @TimelordUK.

[kyle-apex]

@TimelordUK Thanks for your work on msnodesqlv8. I have an old setup where updating to 0.2.16 avoids connection is busy when severity is 14 (ex: constraint issue) as described; however, it there something I can override so that severity 16 issues don't cause it as well? Any attempted addition over the specified column length (severity 16) causes connection is busy for us. We're using the loopback framework with msnodesqlv8 as the driver.

Thanks,
Kyle

[TimelordUK]

That is interesting. I will take a look at this issue to see why 16 does not terminate a statement correctly.

I’ll report back when I know more. Can I reproduce this issue with a raise error 16 statement ?

Sent from my iPhone

On 20 Dec 2018, at 16:31, Kyle Hoskins [e-mail] wrote:

@TimelordUK Thanks for your work on msnodesqlv8. I have an old setup where updating to 0.2.16 avoids connection is busy when severity is 14 (ex: constraint issue) as described; however, it there something I can override so that severity 16 issues don't cause it as well? Any attempted addition over the specified column length (severity 16) causes connection is busy for us. We're using the loopback framework with msnodesqlv8 as the driver.

Thanks,
Kyle

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.