tediousjs / tedious

Node TDS module for connecting to SQL Server databases.
http://tediousjs.github.io/tedious/
MIT License
1.58k stars 439 forks source link

feat: provide better error messages when input param is invalid #1603

Closed jtomaszewski closed 3 months ago

jtomaszewski commented 9 months ago

Currently when I run a DB procedure that requires a TVP input that has many columns and rows, if at least one value of that is incorrect (e.g. fails with a The number NaN cannot be converted to a BigInt because it is not an integer error), I only get that error message - without highlighting which param value is the faulty one. It looks like this:

RequestError: The number NaN cannot be converted to a BigInt because it is not an integer
  File "/home/site/wwwroot/node_modules/mssql/lib/tedious/request.js", line 811, col 19, in Request.userCallback
    err = new RequestError(err, 'EREQUEST')
  File "/home/site/wwwroot/node_modules/tedious/lib/request.js", line 239, col 14, in Request.callback
    this.userCallback(err, rowCount, rows);
  File "/home/site/wwwroot/node_modules/tedious/lib/connection.js", line 2623, col 22, in Parser.onEndOfMessage
    sqlRequest.callback(sqlRequest.error, sqlRequest.rowCount, sqlRequest.rows);
  File "node:events", line 628, col 28, in Object.onceWrapper
  File "node:events", line 514, col 28, in Parser.emit
  File "node:domain", line 489, col 12, in Parser.emit
  File "/home/site/wwwroot/node_modules/tedious/lib/token/token-stream-parser.js", line 24, col 12, in Readable.<anonymous>
    this.emit('end');
  File "node:events", line 514, col 28, in Readable.emit
  File "node:domain", line 489, col 12, in Readable.emit
  File "node:internal/streams/readable", line 1359, col 12, in endReadableNT
  File "node:internal/process/task_queues", line 82, col 21, in process.processTicksAndRejections

Finding the cause is very difficult then, I basically have to check every value 1-by-1 manually to see which one is wrong... or put a debugger into the tedious source code.

This PR a bit improves that behaviour. If an error is thrown during the generateParameterData call, the error is wrapped, and its' message is kept, but also enhanced with information about which param is the wrong one.

P.S. I can add some tests or change the implementation if you want, just let me know if you're okay with the general idea of doing this.

codecov[bot] commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 78.28%. Comparing base (3856dc5) to head (87dbc1a).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1603 +/- ## ========================================== - Coverage 78.97% 78.28% -0.70% ========================================== Files 93 93 Lines 4871 4876 +5 Branches 937 937 ========================================== - Hits 3847 3817 -30 - Misses 718 761 +43 + Partials 306 298 -8 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

MichaelSun90 commented 4 months ago

Hi @jtomaszewski, sorry for the long silence period on this PR. I have worked with @arthurschreiber , and made some change to this PR, and fix the failing tests. Please give a look, see if you ok with the changes. Hi @arthurschreiber, I found a way to get the change in here, not s smart way, but I think it works. Please take a look, see if this one is ok to be merged.

jtomaszewski commented 4 months ago

Awesome work, it looks great to me :)

github-actions[bot] commented 3 months ago

:tada: This PR is included in version 18.4.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: