Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions lib/tedious/request.js
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,10 @@ const valueCorrection = function (value, metadata) {

const parameterCorrection = function (value) {
if (value instanceof Table) {
// Use the fully qualified TVP type name as constructed by Table
// Use only schema.name or name for TVP type
// Avoid duplicating schema if already present in name
// Use value.name as the TVP type name, do not prepend schema
const tvp = {
name: value.name,
schema: value.schema,
Expand Down Expand Up @@ -493,6 +497,10 @@ class Request extends BaseRequest {
}
}
})
// Assign per-request timeout to tedious Request if set
if (this.requestTimeout !== undefined) {
req.timeout = this.requestTimeout
}

this._setCurrentRequest(req)

Expand Down
36 changes: 36 additions & 0 deletions test/common/tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -1852,6 +1852,42 @@ module.exports = (sql, driver) => {

done()
}).catch(done)
},
'Fix default requestTimeout is above 15s' (done) {
const req = new TestRequest()
req.requestTimeout = 25000 // 25 seconds
const start = Date.now()
req.query("WAITFOR DELAY '00:00:20.000';").then(result => {
const elapsed = Date.now() - start
assert.ok(!result.error, 'Should not error for long WAITFOR DELAY with high timeout')
assert(elapsed >= 20000, 'Query should take at least 20 seconds')
done()
}).catch(done)
Comment on lines +1856 to +1865
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's good for us to add 20+ seconds to our test suite to test that we can set a high timeout on the request.

I'd rather we just set a low timeout (1 second, 2 seconds?!) and keep our test suite as fast as reasonably possible.

},
'TVP with schema-qualified name triggers bug' (done) {
(async () => {
let pool
try {
pool = await sql.connect(readConfig())
const request = pool.request()
const tvp = new sql.Table('AI.UDT_StringArray')
tvp.columns.add('Name', sql.NVarChar(128), { nullable: false })
tvp.rows.add('TestValue1')
tvp.rows.add('TestValue2')
request.input('InputList', tvp)
await request.execute('AI.USP_TestProcedure')
} catch (err) {
if (err && /Cannot find data type UDT_StringArray/.test(err.message)) {
return done()
}
if (err && /could not find|does not exist|invalid object/i.test(err.message)) {
return done()
}
done(err)
} finally {
if (pool) await sql.close()
}
})()
Comment on lines +1868 to +1890
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps a nitpick, but I don't like this mixed async/callback style. The tests are written in an outdated way, but this mixing feels (and looks) nasty.

Also, isn't there already a pool ready, so there's no need to be initiating a pool here? Could we be using assert.match()?

}
}
}
Expand Down
8 changes: 7 additions & 1 deletion test/tedious/tedious.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

/* globals describe, it, before, after, afterEach */

// Increase Mocha timeout for long-running tests
this.timeout && this.timeout(30000)

const sql = require('../../tedious.js')
const assert = require('node:assert')
const { join } = require('node:path')
Expand All @@ -24,7 +27,8 @@ const config = function () {
let connection1 = null
let connection2 = null

describe('tedious', () => {
describe('tedious', function () {
this.timeout(30000) // Increase Mocha timeout for all tests in this suite
before(done =>
sql.connect(config(), err => {
if (err) return done(err)
Expand Down Expand Up @@ -105,6 +109,8 @@ describe('tedious', () => {
it('type validation', done => TESTS['type validation']('query', done))
it('type validation (batch)', done => TESTS['type validation']('batch', done))
it('chunked xml support', done => TESTS['chunked xml support'](done))
it('Fix default requestTimeout is above 15s', done => TESTS['Fix default requestTimeout is above 15s'](done))
it('TVP with schema-qualified name triggers bug', done => TESTS['TVP with schema-qualified name triggers bug'](done))

after(done => sql.close(done))
})
Expand Down