Node-mssql: Bad unique identifier in query exhausts connection pool

[srglovin]

Passing in a bad uuid for a uniqueidentifier field (for example 'foobar') throws a NaN exception and more importantly does not release the connection back to the pool. Multiple calls will exhaust connection pool stopping subsequent sql processing.

Expected behaviour:

I expect an exception to be thrown AND the connection to be released so that we don't run out of connections in the connection pool.

Actual behaviour:

Simple schema:

CREATE TABLE TestUniqueId(
  id UNIQUEIDENTIFIER NOT NULL PRIMARY KEY,
  description varchar(50))

Query that causes exception:

  select * from TestUniqueId where id = 'foobar';

Here is the exception:

error TypeError: Cannot read property 'NaN' of undefined
    at Object.guidToArray (/Users/e10113940/learn-node/node_modules/tedious/lib/guid-parser.js:26:42)
    at Object.writeParameterData (/Users/e10113940/learn-node/node_modules/tedious/lib/data-types/uniqueidentifier.js:23:49)
    at new RpcRequestPayload (/Users/e10113940/learn-node/node_modules/tedious/lib/rpcrequest-payload.js:82:12)
    at Connection.execSql (/Users/e10113940/learn-node/node_modules/tedious/lib/connection.js:1449:49)
    at parent.acquire (/Users/e10113940/learn-node/node_modules/mssql/lib/tedious.js:855:63)
    at _acquire.then.connection (/Users/e10113940/learn-node/node_modules/mssql/lib/base.js:175:42)
    at processTicksAndRejections (internal/process/next_tick.js:81:5)

Configuration:

// paste relevant config here

Software versions

• NodeJS: V11.10.0
• node-mssql: 5.0.3
• SQL Server: SQL Server 12.0.2000.8

[dhensby]

I've looked into this and can't produce the same error or the behaviour described in this ticket. So whilst there is an error (somewhat expected) it doesn't deplete the pool.

Are you able to provide a script that replicates the problem?

I've created the simple schema as described, executed the supplied query using the following code:

const sql = require('mssql')

const sqlConfig = {
  password: '***',
  database: '***',
  // connectionTimeout: undefined,
  // requestTimeout: 30000,
  stream: false,
  options: { encrypt: true },
  port: 1433,
  user: '***',
  server: 'localhost',
  pool: {
    acquireTimeoutMillis: 1000
  }
}

let count = 0

function badUUIDReq (pool) {
  const req = pool.request()
  return req.query("select * from TestUniqueId where id = 'foobar';").catch((e) => {
    if (count++ < 10) {
      return badUUIDReq(pool)
    } else {
      throw e
    }
  })
}

function main () {
  sql.connect(sqlConfig).then((connection) => {
    return badUUIDReq(connection)
  }).then((result) => {
    console.dir(result)
  }).then(() => sql.close()).catch(() => {
    return sql.close()
  })
}

main()

Executing that test script gives me repeated errors of:

RequestError: Conversion failed when converting from a character string to uniqueidentifier.
    at handleError (./node-mssql/lib/tedious.js:566:15)
    at Connection.emit (events.js:189:13)
    at Parser.tokenStreamParser.on.token (./node-mssql/node_modules/tedious/lib/connection.js:716:12)
    at Parser.emit (events.js:189:13)
    at Parser.parser.on.token (./node-mssql/node_modules/tedious/lib/token/token-stream-parser.js:27:14)
    at Parser.emit (events.js:189:13)
    at addChunk (./node-mssql/node_modules/tedious/node_modules/readable-stream/lib/_stream_readable.js:296:12)
    at readableAddChunk (./node-mssql/node_modules/tedious/node_modules/readable-stream/lib/_stream_readable.js:278:11)
    at Parser.Readable.push (./node-mssql/node_modules/tedious/node_modules/readable-stream/lib/_stream_readable.js:239:10)
    at Parser.Transform.push (./node-mssql/node_modules/tedious/node_modules/readable-stream/lib/_stream_transform.js:139:32)
  code: 'EREQUEST',
  number: 8169,
  lineNumber: 1,
  state: 2,
  class: 16,
  serverName: 'd5db4d2286f6',
  procName: '',
  originalError:
   { Error: Conversion failed when converting from a character string to uniqueidentifier.
       at handleError (./node-mssql/lib/tedious.js:564:19)
       at Connection.emit (events.js:189:13)
       at Parser.tokenStreamParser.on.token (./node-mssql/node_modules/tedious/lib/connection.js:716:12)
       at Parser.emit (events.js:189:13)
       at Parser.parser.on.token (./node-mssql/node_modules/tedious/lib/token/token-stream-parser.js:27:14)
       at Parser.emit (events.js:189:13)
       at addChunk (./node-mssql/node_modules/tedious/node_modules/readable-stream/lib/_stream_readable.js:296:12)
       at readableAddChunk (./node-mssql/node_modules/tedious/node_modules/readable-stream/lib/_stream_readable.js:278:11)
       at Parser.Readable.push (./node-mssql/node_modules/tedious/node_modules/readable-stream/lib/_stream_readable.js:239:10)
       at Parser.Transform.push (./node-mssql/node_modules/tedious/node_modules/readable-stream/lib/_stream_transform.js:139:32)
     info:
      { number: 8169,
        state: 2,
        class: 16,
        message:
         'Conversion failed when converting from a character string to uniqueidentifier.',
        serverName: 'd5db4d2286f6',
        procName: '',
        lineNumber: 1,
        name: 'ERROR',
        event: 'errorMessage' } },
  name: 'RequestError',
  precedingErrors: []

The debug output is:

$ DEBUG=* node test2.js 
  mssql:base pool(1): created +0ms
  mssql:base pool(1): connecting +2ms
  mssql:tedi pool(1): connection #1 created +0ms
  mssql:tedi connection(1): establishing +0ms
  mssql:tedi connection(1): established +73ms
  mssql:base pool(1): connected +100ms
  mssql:tedi connection(1): destroying +0ms
  mssql:tedi connection(1): destroyed +1ms
  mssql:base request(1): created +2ms
  mssql:base pool(1): attempting to create connection resource(0), attempt #0 +1ms
  mssql:tedi pool(1): connection #2 created +3ms
  mssql:tedi connection(2): establishing +0ms
  mssql:tedi connection(2): established +46ms
  mssql:tedi connection(2): borrowed to request #1 +2ms
  mssql:tedi request(1): query select * from TestUniqueId where id = 'foobar'; +0ms
  mssql:base connection(2): released +55ms
  mssql:tedi request(1): failed { [error] } +6ms
  mssql:base request(2): created +3ms
  mssql:tedi connection(2): borrowed to request #2 +3ms
  mssql:tedi request(2): query select * from TestUniqueId where id = 'foobar'; +0ms
  mssql:base connection(2): released +4ms
  mssql:tedi request(2): failed { [error] } +4ms
  mssql:base request(3): created +0ms
  mssql:tedi connection(2): borrowed to request #3 +0ms
  mssql:tedi request(3): query select * from TestUniqueId where id = 'foobar'; +0ms
  mssql:base connection(2): released +2ms
  mssql:tedi request(3): failed { [error] } +2ms
  mssql:base request(4): created +0ms
  mssql:tedi connection(2): borrowed to request #4 +1ms
  mssql:tedi request(4): query select * from TestUniqueId where id = 'foobar'; +0ms
  mssql:base connection(2): released +2ms
  mssql:tedi request(4): failed { [error] } +1ms
  mssql:base request(5): created +0ms
  mssql:tedi connection(2): borrowed to request #5 +0ms
  mssql:tedi request(5): query select * from TestUniqueId where id = 'foobar'; +0ms
  mssql:base connection(2): released +2ms
  mssql:tedi request(5): failed { [error] } +2ms
  mssql:base request(6): created +0ms
  mssql:tedi connection(2): borrowed to request #6 +0ms
  mssql:tedi request(6): query select * from TestUniqueId where id = 'foobar'; +0ms
  mssql:base connection(2): released +2ms
  mssql:tedi request(6): failed { [error] } +2ms
  mssql:base request(7): created +0ms
  mssql:tedi connection(2): borrowed to request #7 +0ms
  mssql:tedi request(7): query select * from TestUniqueId where id = 'foobar'; +0ms
  mssql:base connection(2): released +2ms
  mssql:tedi request(7): failed { [error] } +2ms
  mssql:base request(8): created +0ms
  mssql:tedi connection(2): borrowed to request #8 +0ms
  mssql:tedi request8): query select * from TestUniqueId where id = 'foobar'; +0ms
  mssql:base connection(2): released +2ms
  mssql:tedi request(8): failed { [error] } +2ms
  mssql:base request(9): created +0ms
  mssql:tedi connection(2): borrowed to request #9 +0ms
  mssql:tedi request(9): query select * from TestUniqueId where id = 'foobar'; +0ms
  mssql:base connection(2): released +2ms
  mssql:tedi request(9): failed { [error] } +2ms
  mssql:base request(10): created +0ms
  mssql:tedi connection(2): borrowed to request #10 +0ms
  mssql:tedi request(10): query select * from TestUniqueId where id = 'foobar'; +0ms
  mssql:base connection(2): released +2ms
  mssql:tedi request(10): failed { [error] } +2ms
  mssql:base request(11): created +0ms
  mssql:tedi connection(2): borrowed to request #11 +0ms
  mssql:tedi request(11): query select * from TestUniqueId where id = 'foobar'; +0ms
  mssql:base connection(2): released +2ms
  mssql:tedi request(11): failed { [error] } +2ms
  mssql:tedi connection(2): destroying +2ms
  mssql:tedi connection(2): destroyed +0ms

[srglovin]

I realized I misreported how to reproduce the error. Instead of passing in the sql “select * from TestUniqueId where id = ‘foobar’”, I actually set ‘foobar' as a request input parameter. I updated badUUIDReq() function below to reproduce the problem. I also changed the count to 15 as the pool size is 10.

Steve

function badUUIDReq (pool) {
  const req = pool.request()
  req.input('id', sql.UniqueIdentifier, 'foobar')
  return req.query("select * from TestUniqueId where id = @id").catch((e) => {
    console.log('Request threw exception',e)
    if (count++ < 15) {
      return badUUIDReq(pool)
    } else {
      throw e
    }
  })
}

[dhensby]

Thanks @srglovin, I've now been able to verify this problem and have created a fix at #842

This is a problem in v4 and v5, so I'll patch both

[dhensby]

Fixed in v4.3.7, v5.0.5, and v6.0.0-alpha.8