Feature Request: Request.prototype.setTimeout()

[fullsailor]

I need a way to set different request timeouts without using multiple connection pools. Tedious has a setTimeout function on Request that implements this behavior, but there's not a defined way to get at the driver Request object from a mssql.Request object.

Expected behaviour:

That I could have a pool and request like this, and that the specified timeout would be set on the underlying tedious Request when it is created.

const pool = new ConnectionPool({options: { requestTimeout: 15_000 });
await pool.request().setTimeout(90_000).query("waitfor delay '00:01'"); // synthetic slow query

Actual behaviour:

The only supported workaround is to have different connection pools with different timeouts. This works in some limited cases, but is not very flexible and we can't have two requests in the same transaction with different timeouts.

Software versions

• NodeJS: 18
• node-mssql: 9.1.1
• SQL Server: 2019

Monkey patch

A bad implementation of this feature would be something like this patch.

declare module 'mssql' {
    interface Request {
        setTimeout(timeout: number | undefined): this;
    }
}

const kTimeout = Symbol('request timeout');
Request.prototype.setTimeout = function (timeout: number | undefined) {
    this[kTimeout] = timeout;
    // for chaining like other mssql Request functions
    return this;
};

// _setCurrentRequest is the only 'hook' that we can patch to get at the underlying
// tedious request before it is passed to the acquired connection
const orig = Request.prototype._setCurrentRequest as (req: any) => any;
Request.prototype._setCurrentRequest = function _setCurrentRequest(req: tedious.Request) {
    if (this[kTimeout] !== undefined) {
        req.setTimeout(this[kTimeout] as number);
    }
    return orig.call(this, req);
};

[dhensby]

Interesting, can you explain a little more about why you need to be able to adjust your timeout on the fly?

Usually I'd expect an application as a whole to share an acceptable timeout, but perhaps there are some cases when it is needed on a connection-by-connection basis.

There are a couple of challenges here:

1. The connection isn't actually released until the request is made, so this config needs to be held in memory and applied to connections as they are acquired from the pool. But that's not too hard.
2. We need to make sure this works with the msnodesqlv8 driver too.

[fullsailor]

can you explain a little more about why you need to be able to adjust your timeout on the fly?

Its not so much on the fly, but we have different expected SLAs for specific queries and sprocs. Some kinds of queries we know are slower, so the global timeout has to be raised. In general we want queries that should be fast to fail faster if they hit a 15s timeout.

The connection isn't actually released until the request is made, so this config needs to be held in memory and applied to connections as they are acquired from the pool. But that's not too hard.

I'm specifically trying to reach this API http://tediousjs.github.io/tedious/api-request.html#function_setTimeout from this wrapper package. So, not configuring a connection property but a request one. The life of the tedious.Request is entirely within Request _query/_execute. I also would not expect changing the timeout after calling query/execute to work for any in progress requests.

[dhensby]

I'm specifically trying to reach this API http://tediousjs.github.io/tedious/api-request.html#function_setTimeout from this wrapper package.

I understand, but the point of this package is to provide a generic interface around the underlying connections/requests. So whilst I appreciate you are trying to achieve low-level access, it's not an intended feature to expose that.

So, not configuring a connection property but a request one. The life of the tedious.Request is entirely within Request _query/_execute.

Sure, but that is an academic differentiation, one way or another, the timeout needs to be applied in a one-off manner or within the context of a request.

Looking at it, it should be possible, the question is how to do it nicely. It looks like we could either allow a subset of options to be given to the request:

const pool = await new ConnectionPool(config).connect();
const req = pool.request(someOptionalConfig); // optional config here could contain `requestTimeout`

or we could expose a specific setRequestTimeout() method on requests, but that feels very tightly coupled and doesn't provide any scalability for the future beyond polluting the API surface of Requests.

This change would also need to work for other objects, like PreparedStatement and Transaction, which should follow a similar API.

[dhensby]

#1530 is a quick example of what might work.

[csod-bganatra]

Hey @dhensby what is going on in this issue, I need this feature as well ,
We need to add timeouts for some queries that is not in config,
like high or low,
Can you please let me know about the status of this issue.

[dhensby]

The PR is "ready" and I was hoping for some feedback from those that wanted it to let me know if it was the right approach (or even better a bit of testing).

Then I was going to finish it up. I didn't hear anything so I wasn't sure if the demand was still there.

[csod-bganatra]

Okay got it, @dhensby ,
So once the timeout is reached,
is this timeout different than calling,

request.cancel() 

as currently I am using timeout like this, as it is not natively supported by this package, or it is different?

[dhensby]

Good question, the change I authored just uses the underlying driver's default behaviour for per-request timeouts. I suspect you're right that it essentially is just calling cancel on the request after a set time.

[csod-bganatra]

Yes @dhensby can you please confirm this,
so I can go ahead,
as once query is fired I don't think we can cancel the query ?
It is just that it will close the connection, right?

[dhensby]

We don't close connections when we cancel queries because the connection is put back into the pool. The cancel call just fires whatever the underlying library does, you'll have to look into that if you want a precise idea of exactly what is going on under-the-hood.

[csod-bganatra]

Okay got it.