TimelordUK/node-sqlserver-v8

Feature proposal

patriksimek opened this issue · 10 comments

Just a small withlist. You know... it's Christmas :)

  • Connection timeout (documentation says it can't be set via connection string)
  • Request cancellation (required to support request timeouts)
  • Multiple errors handling (atm only the first error that occurs is reported)

// connection timeout, can set the attribute on ODBC driver connect :-
// note people have reported this attribute does not cover all cases, I have tested with no server // listening, and do receive error back within the timeout period. Can use conn_str directly as before.

   var connectionObj = {
         conn_str: connStr,
        conn_timeout: 2
      };
msnodesql.open(connectionObj, function (err, conn) {
    if (err) {
        console.error(err);
        process.exit();
    }
    ;
    done(conn);
});

Thanks, it's working, but as you said, it's not very reliable. Real timeout varies according to the type of test.

  • I got 7s real timeout when connecting to unreachable host with timeout set to 1s.
  • I got 14s real timeout when connecting to unreachable host with timeout set to 2s.
  • I got 21s real timeout when connecting to reachable host not running SQL Server with timeout set to 1s.

i added query timeout via the API, this seems to be more exact in terms of behavior.

I will take a look at adding query request cancel.

open(function (conn1) {

    var queryObj = {
        query_str :   "waitfor delay \'00:00:15\';",
        query_timeout : 20
    };
    conn1.query(queryObj, function(err, res) {
        console.log(res);
        console.log(err);
    });
});

Can you think of a simple example which should raise more than one error so I can take a look at this request.

With the recent error handling changes, I think handling multiple errors would be a next logical step. With the following query, only the first error returns an error to the query callback:

RAISERROR('First Error', 1, 1);
RAISERROR('Second Error', 1, 1);

Interestingly, the callback does get called two times with a null error.

OK, This should be supported on latest version.

I have cancel on a branch checked in and appears to be working. It is a fairly substantial change so I will test over coming days before merging it across and releasing. You can now cancel a stored proc call, prepared query or vanilla query,

test('cancel single query from notifier using tmp connection - expect Operation canceled', function (test_done) {

    var q = sql.query(conn_str, "waitfor delay \'00:00:20\';", function (err) {
        assert(err);
        assert(err.message.indexOf('Operation canceled') > 0);
        test_done();
    });
    q.on('submitted', function () {
        q.cancelQuery(function (err) {
            assert(!err);
        });
    });
});

closing let me know what else is required.

@TimelordUK Can you please help me figure out how to handle multiple errors? This is my code:

sql.open(connectionString, function (err, con) {
  if (err) {
    console.log('failed to open ' + err.message)
  }
  const req = con.query('select a;select b;', (err) => {
    console.log("done", err)
  })

  req.on("info", (msg) => {
    console.log("info", msg)
  })

  req.on("error", (msg) => {
    console.log("error", msg)
  })
})

All I get is:

done { Error: [Microsoft][SQL Server Native Client 11.0][SQL Server]Invalid column name 'a'. sqlstate: '42S22', code: 207 }

Running the comamnd via Tedious or SQL Server Management Studio gives me this:

Msg 207, Level 16, State 1, Line 1
Invalid column name 'a'.
Msg 207, Level 16, State 1, Line 1
Invalid column name 'b'.

Thank you. Great job on adding all the missing features btw!

ill release a fix for this

const sql = require('msnodesqlv8')

const connectionString = 'Driver={SQL Server Native Client 11.0}; Server=np:\\\\.\\pipe\\LOCALDB#E086FCD9\\tsql\\query; Database={master}; Trusted_Connection=Yes;'

sql.open(connectionString, function (err, con) {
  if (err) {
    console.log('failed to open ' + err.message)
  }
  const req = con.query('select a;select b;')
  req.on("error", (msg) => {
    console.log("error", msg)
  })
})

error { [Error: [Microsoft][SQL Server Native Client 11.0][SQL Server]Invalid column name 'a'.] sqlstate: '42S22', code: 207 }
error { [Error: [Microsoft][SQL Server Native Client 11.0][SQL Server]Invalid column name 'b'.] sqlstate: '42S22', code: 207 }