fix: clear error when sql_variant used in TVP columns#1841
Merged
dhensby merged 3 commits intotediousjs:masterfrom Apr 17, 2026
Merged
fix: clear error when sql_variant used in TVP columns#1841dhensby merged 3 commits intotediousjs:masterfrom
dhensby merged 3 commits intotediousjs:masterfrom
Conversation
Collaborator
Author
|
@Avi-E-Koenig - how does this look to you? |
hi @dhensby this could definitley clear confusion in this use case |
The tedious driver does not implement serialization for sql_variant (generateTypeInfo, validate, etc. all throw 'not implemented'). When a TVP contains sql_variant columns, this caused an unhelpful error during request execution. This change: - Detects sql_variant columns in parameterCorrection() and throws a clear RequestError explaining the limitation and suggesting a more specific data type - Wraps parameter serialization in try/catch in both _query() and _execute() paths so errors from parameterCorrection() are properly routed to the callback instead of thrown as uncaught exceptions - Adds a reproduction script (repro-1796.js) and unit test Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
b3311c7 to
6effc3e
Compare
There was a problem hiding this comment.
Pull request overview
Improves developer-facing error handling for Table-Valued Parameters (TVPs) containing sql_variant columns when using the tedious driver, preventing unhelpful uncaught exceptions and providing a clearer limitation message.
Changes:
- Detects
sql_variantTVP columns during parameter correction and throws a clearerRequestError. - Wraps parameter addition/serialization with
try/catchin_query()and_execute()to route parameter-processing failures to the callback. - Adds a unit test to assert that the error message mentions
sql_variantand the problematic column name.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
lib/tedious/request.js |
Adds early sql_variant TVP detection and wraps parameter setup in try/catch for query/execute paths. |
test/common/unit.js |
Adds a unit test validating the improved error message for sql_variant TVP columns. |
Comments suppressed due to low confidence (1)
lib/tedious/request.js:730
- The
catchhere callshandleError(true, connection, error), buthandleErrordoes not deletethis._canceland (whenthis.stream === true) it will emiterrorwithout removing listeners / releasing the borrowed connection / invoking the callback. For errors thrown beforeexecSql/execSqlBatchis called (e.g., fromparameterCorrection()), this can leak the connection and leave a stale_cancelfunction pointing at a released connection. Consider handling this catch as a fatal pre-exec failure: remove the event listeners, deletethis._cancel, release the connection, markhasReturned, and surface the original error (without re-wrapping if it’s already aRequestError).
try {
if (!this._isBatch) {
for (const name in this.parameters) {
if (!objectHasProperty(this.parameters, name)) {
continue
}
const param = this.parameters[name]
if (param.io === 1) {
req.addParameter(param.name, getTediousType(param.type), parameterCorrection(param.value), { length: param.length, scale: param.scale, precision: param.precision })
} else {
req.addOutputParameter(param.name, getTediousType(param.type), parameterCorrection(param.value), { length: param.length, scale: param.scale, precision: param.precision })
}
}
}
connection[this._isBatch ? 'execSqlBatch' : 'execSql'](req)
} catch (error) {
handleError(true, connection, error)
}
When a parameter validation error occurs during batch execution in _query(), the connection was released back to the pool without removing the infoMessage/errorMessage/error listeners and without clearing the _cancel callback. This could cause memory leaks and cross-request event handling on pooled connections. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Apply the same listener cleanup pattern to the _execute() catch block: remove event listeners, delete _cancel, and set hasReturned before releasing the connection. Also preserve RequestError instances thrown by parameterCorrection() instead of double-wrapping them. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🎉 This PR is included in version 12.3.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #1796.
When a TVP contains
sql_variantcolumns, the tedious driver throws an unhelpful uncaught exception because itssql_varianttype has all serialization methods (generateTypeInfo,validate,generateParameterLength,generateParameterData) stubbed withthrow new Error('not implemented').This PR:
Detects
sql_variantcolumns early inparameterCorrection()and throws a clearRequestErrorexplaining the limitation and suggesting a more specific data type.Wraps parameter serialization in try/catch in both
_query()and_execute()paths so that errors fromparameterCorrection()(and any other parameter processing errors) are properly routed to the callback instead of thrown as uncaught exceptions.Adds a unit test verifying the error message.
Root cause
This is ultimately a limitation in the tedious driver —
sql_variantis not implemented for parameter serialization. A full fix would require implementingsql_variantsupport in tedious itself, which is non-trivial as it is a container type that can hold many different SQL Server types. This PR provides a clear error message in the meantime.Error message
Before:
After: