Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature proposal #9

Closed
3 tasks done
patriksimek opened this issue Dec 18, 2015 · 10 comments
Closed
3 tasks done

Feature proposal #9

patriksimek opened this issue Dec 18, 2015 · 10 comments

Comments

@patriksimek
Copy link

patriksimek commented Dec 18, 2015

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)
@TimelordUK
Copy link
Owner

// 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);
});

@patriksimek
Copy link
Author

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.

@TimelordUK
Copy link
Owner

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);
    });
});

@TimelordUK
Copy link
Owner

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

@ratanaklun
Copy link

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.

@TimelordUK
Copy link
Owner

OK, This should be supported on latest version.

@TimelordUK
Copy link
Owner

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);
        });
    });
});

@TimelordUK
Copy link
Owner

closing let me know what else is required.

@patriksimek
Copy link
Author

@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!

@TimelordUK
Copy link
Owner

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 }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants