feature: add hints to errors when returning errors from the server

[NoNameProvided]

I have tried to create a bulk insert and struggled more hours than I willing to admit to understand why I am receiving the following error on every attempt:

The incoming tabular data stream (TDS) protocol stream is incorrect. The stream ended unexpectedly.

This is a very general error message (from the MS SQL server) and doesn't include any details why the data stream is invalid. After searching the issues here and after Googling I still couldn't find any discussion about it besides some irrelevant Microsoft knowledgebase articles.

The root of the problem was that one of the inserted fields used an Text type instead of Char(7).

- table.columns.add('myColumn', sql.Text, { nullable: false });
+ table.columns.add('myColumn', sql.Char(7), { nullable: false });

Expected behaviour:

I am sure I am not the first one to run into this issue. I would have expected that the error message contains hints about what can go wrong when this messages is received.

I would either expect some direct hints inlined in the error or a link directing me to a FAQ page where the common problems are listed.

Actual behaviour:

No hints are provided why this error is happening.

Configuration:

Code re reproduce

require('dotenv-safe').config();
const sql = require('mssql');

const pool = new sql.ConnectionPool({
  user: process.env['DATABASE_USER'],
  password: process.env['DATABASE_PASSWORD'],
  database: process.env['DATABASE_NAME'],
  server: process.env['DATABASE_SERVER'],
  port: Number.parseInt(process.env['DATABASE_PORT']) || 1433,
  debug: true,
  options: {
    encrypt: true,
  },
});

pool.on('debug', (connection, message) => {
  console.log(message);
});

pool
  .connect()
  .then(connection => {
    const table = new sql.Table('[MY_DB].[my_schema].[my_table]');
    const request = new sql.Request(connection);

    table.columns.add('UTCLogDateTime', sql.DateTime2(2), { nullable: false });
    table.columns.add('myColumn1', sql.SmallInt, { nullable: false });
    table.columns.add('myColumn2', sql.SmallInt, { nullable: false });
    // make sure myColumn3 is actually a Char(7) column
    table.columns.add('myColumn3', sql.Text, { nullable: false });

    table.rows.add(new Date('2030-01-20T08:00:00.000Z'), 42, 42, 'value2');

    return request.bulk(table, { keepNulls: true });
  })
  .catch(e => {
    console.error('Completed with error!');
    console.error(e);
    pool.close().then(() => process.exit(1));
  });

Software versions

• NodeJS: LTS (v12.7.0)
• node-mssql: 6.0.1
• SQL Server: Microsoft SQL Azure (RTM) - 12.0.2000.8

[dhensby]

I think this is a good idea, however the error is most likely coming from the tedious driver as it's the one parsing the TDS stream, this library just wraps it.

It may be better to bring this up on the tedious repo instead?

[NoNameProvided]

I think this is a good idea, however the error is most likely coming from the tedious driver as it's the one parsing the TDS stream, this library just wraps it.

The error comes from the server when it tries to parse the sent data stream. (So the server fails to parse not the tedious driver.)

It may be better to bring this up on the tedious repo instead?

I would not expect a low level driver to make guesses what happened as it has no context about the desired operation, I would expect that from the higher level abstractions (like this library).

For example: This error is only affected when bulk inserting data. (single insert via request works with wrong data type specified). The driver itself doesn't know we are trying to bulk insert it just parses and sends the received data. However (I assume) the library is aware that we are bulk inserting when the error is received so it can show tips based on desired operation.

[dhensby]

I see what you're saying, but the drivers also know when a bulk operation is happening because they expose this. However, because we are providing a common interface for more than one driver, it probably would be less effort for us to implement this than pushing the responsibility onto 2 (or more) drivers.

@NoNameProvided is this something you'd be willing to contribute to the library?

[NoNameProvided]

Hi!

Sorry for the late reply. I am not sure I have the all around expertise to "discover" all the possible edge-cases. (Read as I am a very basic user or MSSQL, we only develop in it for a customer, we use Postgres otherwise.) If you like to I can write a note section about this in the bulk insert part of the documentation or if we discuss the proposed way to handle this from code, I am down to contribute.

[dhensby]

I don't think we need to discover all the edge cases - just filling them in as we come across them is fine

[NoNameProvided]

What is your opinion should we add this as documentation or included it in the code? I see four options here:

• extend the original error for specific error types and return the extended error with some extra data
• always wrap the errors in a custom error
• log hints to the console when a known error is raised
• create a FAQ of common errors

The first two I don't like personally as it changes the original error what is not desirable. The 3rd option can work but polluting the global console without allowing the user to disable it is a no-go, so some minimal log handler must be added for it which the user can enable/disable. (If we go this direction it can be combined with #840 to remove the dependency on the debug package and provide an optional callback where the lib outputs the generated log messages.)

The fourth option is the simplest but requires the user to search for the issue instead of being presented on the spot.