tediousjs / node-mssql

Microsoft SQL Server client for Node.js
https://tediousjs.github.io/node-mssql
MIT License
2.23k stars 468 forks source link

Did node-mssql change its response shape between 6.3.0 and 6.4.0 / 7.2.1 and 7.3.0? #1391

Closed ShaunB-SQ closed 2 years ago

ShaunB-SQ commented 2 years ago

I am trying to update my node package from 5.1.0 to newest and on the first change to 6.4.1 its breaking immediately and I can't figure out what I need to do to fix it.

It's like the shape of the response obj is changed.

 const fetchQueuedReport = async () => {
  try {
    const request = new sql.Request();
    const response = await request.query(SELECT_QUEUED);
    console.dir(response, { depth: null, colors: true });
    const [report] = response.recordset;
    return report;
  } catch (error) {
    console.error(error);
    throw error;
  }
};

In 5.1.0 the response I get as I am expecting is.

{ recordsets: [ [] ], recordset: [], output: {}, rowsAffected: [ 0 ] } In 6.4.1 the response I am getting is

Mon, 09 May 2022 19:55:27 GMT tedious deprecated The default value for `config.options.enableArithAbort` will change from `false` to `true` in the next major version of `tedious`. Set the value to `true` or `false` explicitly to silence this message. at node_modules\mssql\lib\tedious\connection-pool.js:61:23
{ recordsets: null, recordset: null, output: {}, rowsAffected: [ 0 ] }

Notice recordsets and recordset are null now and no longer giving me the arrays I am expecting.

I can't find any documentation on what is wrong here or how to fix it so I get the results back that I am expecting from the DB.

Walking up the version tree it seems that it works until 6.3.2 and then at 6.4.0 it breaks on me.

Even 7.2.1 works for me and it was released before 6.4.0 release branch, but 7.3.0 released at the same time as 6.4.0 breaks for me the same way.

{ recordsets: null, recordset: null, output: {}, rowsAffected: [ 0 ] }

dhensby commented 2 years ago

There hasn't been any intentional change to the response shape for those releases.

Both release 6.4.0 and 7.3.0 introduce the same fix (https://github.com/tediousjs/node-mssql/pull/1338) where by transactions/prepared statements will correctly inherit config settings from the ConnectionPool (eg: stream or arrayRowMode), so that's a clue.

You haven't provided the connection config, nor how you're connecting to the pool, so it's hard to say what's going on. I think the most likely explanation is that your global pool is connected in stream mode and when you create a new Request instance, it will use the global pool if one is not passed to it. The fixes in 6.4. and 7.3 mean that the parent connection's (ie: the global pool in this instance) configuration regarding streaming will be used whereas before it was not being carried over from the global pool.

Please check your global pool config and see if you're using stream mode, if you are, I suspect that is the problem and you'll need to stop using stream mode or set stream mode to false on the particular requests where you don't want to use stream mode.

ShaunB-SQ commented 2 years ago

connection code is:

const sql = require('mssql');
const { fetchConfigV3 } = require('./AwsClient');

const ONE_HOUR = 60 * 60 * 1000;

let connection = null;

const connectToDb = async () => {
  try {
    if (connection) {
      return connection;
    }

    const configV3 = await fetchConfigV3();
    const { username, password, host, name } = configV3.database;
    const sqlConfig = {
      user: username,
      password,
      server: host,
      database: name,
      requestTimeout: ONE_HOUR,
      stream: true,
      options: {
        trustServerCertificate: true,
      },
    };

    connection = await sql.connect(sqlConfig);
    return connection;
  } catch (error) {
    console.error(error);
    throw error;
  }
};

module.exports = {
  connectToDb,
};
ShaunB-SQ commented 2 years ago

Setting stream: false did fix the issue and the output changed back to the arrays we were expecting.

dhensby commented 2 years ago

great news, I'm glad you got to the bottom of it