Node-mssql: ResourceRequest timed out

[munir131]

Node: 6.9.1
configuration:

pool: {
    max: 100,
    min: 1,
    idleTimeoutMillis: 15000
}

I am getting this error when there is less traffic or there is gap between requests.

error:  { TimeoutError: ResourceRequest timed out
        at ResourceRequest._fireTimeout (/home/munir/node-workspace/server/node_modules/generic-pool/lib/ResourceRequest.js:62:17)
        at Timeout.bound (/home/munir/node-workspace/server/node_modules/generic-pool/lib/ResourceRequest.js:8:15)
        at ontimeout (timers.js:365:14)
        at tryOnTimeout (timers.js:237:5)
        at Timer.listOnTimeout (timers.js:207:5) name: 'TimeoutError' }

[labs20]

+1, and its random.

Please any help?

[glebson1]

If you're using transactions in your calls it seems to be an issue. Improper usage of them makes pool get stacked. Try to get rid off them.

[munir131]

@glebson1 Thanks for your reply. But, I am not getting how it is affecting to it?

[munir131]

@patriksimek Can you give me any hint that can help?

[gentlerainsky]

I got a similar problem with ESOCKETTIMEOUT. It happened when I deploy my app and my docker install the latest version of tedious (v2.3.1, I think) as its dependency. When I ran my batch jobs which used quite a large and complex query, it made a lot of timeout which didn't happen before in the previous version. While I don't know how write reproduce steps without using the same database and query, My current solution is to force its dependency to install tedious (v2.1.1) using npm-shrinkwrap.json that I made from my local repo.

[munir131]

@gentlerainsky Thanks. I will try it.

[munir131]

@gentlerainsky I tried by using tedious (v1.x) But still getting resourceRequest timeout issue.

[anton-bot]

Sequelize fixed this bug. Please just reference their pull request. This is a huge issue that prevents normal operation!

https://github.com/sequelize/sequelize/pull/8815

[munir131]

@catcher-in-the-try They did same thing which is already done by some developers in pending PR(coopernurse/node-pool#198 & coopernurse/node-pool#221) of generic-pool. So, looks like there is issue in generic-pool

[JankiGadhiya]

Guys, Any updates in this? My production is heading to a big event today and I am in a big mess because of this issue.

[labs20]

I had a really hard time on my customer with this too. Managed to reduce the damage somehow with the config below, but its urgent to fix this one.
pool: { max: 100000, min: 1, idleTimeoutMillis: 50, evictionRunIntervalMillis: 5, softIdleTimeoutMillis: 5 }

[munir131]

@labs20 Are you not facing more issue with this configuration? I think 50ms timeout will create this issue more frequently. Also, It will increase chances of this error.

[labs20]

Well, at least on my customer server it seems to have reduced the problem.

[munir131]

@catcher-in-the-try @labs20 It is working fine for me after I used https://github.com/munir131/node-mssql

[anton-bot]

@munir131 I couldn't figure out how to use, it doesn't seem to be documented or published to NPM.

[munir131]

@catcher-in-the-try "mssql": "https://github.com/munir131/node-mssql/tarball/7227cdffbf66ca2e3dc31b150d9dd2c044c88bce"
Add this in your package.json instead of version in dependencies.
It is same as this library. Difference is I am ensuring that there should be at least one connection available when there is pending request for resource.

[yss14]

Would be nice to have an official fix of this issue. I encountered the same problem on our production servers and it's pretty annoying...

[anton-bot]

I ended up doing this:

• Counter for SQL errors
• Timer job runs every second to check if there have been more than two errors recently
• If so, reestablish the connection by overwriting the instance of mssql with a new one
• All the SQL operations are requeued in case of failure, so they don't get lost and eventually complete

[yss14]

@catcher-in-the-try What do you mean by If so, reestablish the connection by overwriting the instance of mssql with a new one? Just reconnecting to the database via mssql and retry all failed queries?

[munir131]

@catcher-in-the-try How did you accomplish All the SQL operations are re-queued in case of failure, so they don't get lost and eventually complete?

[munir131]

I fixed this issue by fork https://github.com/coopernurse/node-pool library. It is not looking issue of node-mssql. But, I am not sure.

[yss14]

@munir131 So you just enforce npm to install your forked node-pool fix and it's working now without any TimeoutError: ResourceRequest timed out?

[jacqt]

The issue for us was that that the _poolCreate will sometimes randomly return a promise that never resolves when under high load. https://github.com/tediousjs/node-mssql/blob/dee9086569ea007d632813a1f209e6b2a716c2e3/lib/tedious.js#L176

This is a problem because of the way the generic node-pool counts how many TDS connections are potentially available. The generic pool adds the number of currently pending promises that the _poolCreate method returns to the number of potentially available TDS connections - see https://github.com/coopernurse/node-pool/blob/e73247728432f2a681f87b3228ccf10ebe67d5aa/lib/Pool.js#L629

As a result, whenever the _poolCreate method fails to return a promise that resolves, the generic node-pool will over-estimate the number of potentially available TDS connections, and will wait indefinitely for the promise returned by the _poolCreate to resolve. (https://github.com/coopernurse/node-pool/blob/e73247728432f2a681f87b3228ccf10ebe67d5aa/lib/Pool.js#L218)

The simple fix for this was to just add the following lines to the tedious.js file:

https://github.com/tediousjs/node-mssql/compare/master...jacqt:master

I don't think this is the best way to encode this logic but perhaps this will be helpful to other people in this thread

[spearmootz]

there was an alleged fix for this but i still get the error :(

[dreaganluna]

I'm also getting this error. This affects our production environment rather badly every other 2 days (and the we have to restart our service). Is there any more news on this?

[Spragalas]

Is this going to be fixed any time soon ? It effects multiple users in theirs productions including mine as well, @patriksimek ?

[munir131]

@yss14 Yes. I pointed to my forked version in which I fixed a generic-pool issue.

[jacqt]

@munir131 it is not an issue in generic-pool, it is an issue with node-mssql. I have explained in more detail in my previous comment, and I have submitted a pull request with a simple fix for this.

[Strandedpirate]

Update:
@patriksimek, the munir131 fix doesn't work after all. It actually masked an underlying memory leak issue that I tracked down to not properly committing or rolling back transactions that had been opened. That "fix" also produces the error "Resource not currently part of this pool".

If you are using transactions make sure that your commit or rollback under any condition whether those be exceptions or your own logic decision tree. Don't just simply return from a function otherwise the connection associated with that transaction will be left "open" and never get returned to the pool.

Here is an example of what caused my memory leak and ultimately the "ResourceRequest timed out" error:

export default async (req: any, args: any, res: any) => {
  const logger = req.logger;
  const connectionPool = req.connectionPool;
  const transaction = new sql.Transaction(connectionPool);

  try {
    await transaction.begin(); // we started a transaction

    const decodeResult = decode(args.customerId.value);
    if (decodeResult.error) {
      httpUtils.setError(res, new Error(decodeResult.error));
      return; // this just caused a memory leak! our connection will never go back into the pool!! rollback or commit the transaction before returning from the method always. eventually you will run out of connections in your pool
    }

    ... // more logic
  }
  catch (e) {
    await handleRollback(transaction, logger); // good, we rolled back during unhandled conditions
    httpUtils.setError(res, e);
  }
}

In addition I forked this repo and updated the references to generic-pool to 3.4.2. I haven't seen the "ResourceRequest timed out" error since then. I believe the owners of generic-pool have made change regarding this same error message on their end unrelated to my memory leak detailed above.

---

Original:
I've tried all the posted "fixes" here and none of them work EXCEPT for @munir131 fork at https://github.com/munir131/node-mssql.

To install latest use this command:

npm i https://github.com/munir131/node-mssql/tarball/e539a725eed2161e7d0f4cd0b0eda05680b8564c -S

You can install specific commits by replacing the hash after the tarball with any commit-id found at:
https://github.com/munir131/node-mssql/commits/master

[munir131]

@Strandedpirate Thanks for sharing steps to use that fork. That fork is not a perfect solution but it will work. I will add proper solution once I will get time. I am happy that my changes helped you with your issue.

[patriksimek]

I believe I was able to isolate the issue. Try adding this to your connection configuration:

{
    pool: {
        acquireTimeoutMillis: 0
    }
}

Awaiting your feedback. Thank you!

[anton-bot]

@patriksimek Can you give an example of how to use that connection config when the connection is initialized via a connection string? (mssql://...)

[munir131]

@patriksimek If we will use acquireTimeoutMillis more than zero also, in that case, we will get this

[patriksimek]

@catcher-in-the-try Unfortunately, there is no way to set this option via connection string atm.

@munir131 Yep, that's what most probably caused the issue. I misunderstood the meaning of acquireTimeoutMillis option. I was expecting this option to work as an additional timeout mechanism for connection establishment. But after I reviewed the source code of node-pool, I figured out this timeout limits the maximum time request can wait for a connection.

The issue can be easily reproduced by running multiple WAITFOR DELAY \'00:01\' queries. Once all the connections in the pool became borrowed, the problem appears.

[patriksimek]

I've just published release candidate for 4.2. Please, install with npm i mssql@next.

[george-wilson-iproov]

Not finding that this change is making a difference (pulled from master branch at noon BST yesterday) - also tried explicitly setting the acquireTimeoutMillis to 0 on the current dev branch which also appeared to make no difference :(

[patriksimek]

@george-wilson-iproov Can you please post the stack of the exception you're getting?

[dreaganluna]

I've updated our service to have 4.2 of node-mssql, but we're still seeing the same behaviour. I can't provide you with a stack, because we tend to kill off the service (in cluster) before it starts to throw errors. We've found that we can prevent it throwing errors when we see the steady climb in response times.

For the record: I didn't do anything in my config, I was unsure if I still needed to add the acquireTimeoutMillis property.

[patriksimek]

I'll need that stack because I have no idea what else should cause the issue. Maybe I don't understand the code of node-pool correctly.

• The exception is thrown here - https://github.com/coopernurse/node-pool/blob/71fc5582712dc5982d2b3987b84f9fbc93fe8501/lib/ResourceRequest.js#L62
• The line gets executed only when the ResourceRequest object is created with a ttl parameter - https://github.com/coopernurse/node-pool/blob/71fc5582712dc5982d2b3987b84f9fbc93fe8501/lib/ResourceRequest.js#L27
• This is the place where node-pool creates ResourceRequests -https://github.com/coopernurse/node-pool/blob/72b31a434c7b05ad879b2ace9830a9aa9fbba002/lib/Pool.js#L449
• The ttl parameter is linked to the pool's acquireTimeoutMillis parameter.
• I have removed acquireTimeoutMillis from the defaults in 4.2 - https://github.com/tediousjs/node-mssql/commit/b353b31af320927a591275479bd837e361f54205#diff-86b75ecfbd25aa771c8e9e0dff8eb00f

Any help with that is appreciated.

[anton-bot]

@patriksimek Thanks Patrik. The README says acquireTimeoutMillis is 30000 ms by default, does this need to be changed? (not sure if the value is set elsewhere or just the README needs updating)

[patriksimek]

@catcher-in-the-try Can you please send me the link to the README? What I see here is:

acquireTimeoutMillis: max milliseconds an acquire call will wait for a resource before timing out. (default no limit), if supplied should non-zero positive integer.

[dsbert]

Has anyone established a reliable way to reproduce this issue? I'm seeing it in production and it sounds like the latest release may or may not be a solution.

[willmorgan]

One thing I noticed is that we don't reject the factory create promise on error; we just emit an error event:

https://github.com/tediousjs/node-mssql/blob/master/lib/tedious.js#L245-L252

Going to try rejecting on error, in tandem with running the explicit timeout code (from @jacqt ), and see if that solves the problem for us: https://github.com/tediousjs/node-mssql/compare/master...DigitalIdentification:pool-fix?expand=1

I also noticed that node-pool swallows all errors which is going to make any sort of timeout / unexpected error extremely difficult to notice and diagnose.

[willmorgan]

I left this to run overnight and our connection pool no longer depletes in size, so I think it works.

[dsbert]

@willmorgan

I'm not sure if this is the same problem that you are trying to fix, but after running on your code for a while, I eventually hit an ETIMEOUT error from the connection pool. This caused the app to get stuck until I restart it. The only work-around I have been able to come up with is to kill the app if any error is emitted by the connection pool object. It looks like when an error occurs in the connection pool when running a request, the request is never rejected. Here is an example.

pool.on("error", (err) => {
  // ETIMEOUT error occurs
});

try {
  const results = await pool
    .request()
    .input("message", sql.NVarChar(50), message.substr(0, 50))
    .output("result", sql.NVarChar(1000))
    .execute("db.procedure");
  return results;
} catch (err) {
  // this never gets hit
}

[willmorgan]

@dsbert That's interesting. I haven't seen an ETIMEOUT error crash our Node services yet but I'll keep an eye out for that. Do you have a stack trace you can share? If there's a defect in my PR then would be keen to fix...

[dsbert]

@willmorgan

Here is the stack trace.

[2018-07-25T13:02:50.259] [ERROR] default - Database connection pool error { error: 
   { ConnectionError: Failed to connect to 1.1.1.1:1433 in 15000ms
    at Connection.tedious.once.err (C:\projects\host\node_modules\mssql\lib\tedious.js:216:17)
    at Object.onceWrapper (events.js:255:19)
    at Connection.emit (events.js:160:13)
    at Connection.connectTimeout (C:\projects\host\node_modules\tedious\lib\connection.js:936:12)
    at Timeout._onTimeout (C:\projects\host\node_modules\tedious\lib\connection.js:906:16)
    at ontimeout (timers.js:458:11)
    at tryOnTimeout (timers.js:296:5)
    at Timer.listOnTimeout (timers.js:259:5)
     code: 'ETIMEOUT',
     originalError: 
      { ConnectionError: Failed to connect to 1.1.1.1:1433 in 15000ms
    at ConnectionError (C:\projects\host\node_modules\tedious\lib\errors.js:12:12)
    at Connection.connectTimeout (C:\projects\host\node_modules\tedious\lib\connection.js:936:28)
    at Timeout._onTimeout (C:\projects\host\node_modules\tedious\lib\connection.js:906:16)
    at ontimeout (timers.js:458:11)
    at tryOnTimeout (timers.js:296:5)
    at Timer.listOnTimeout (timers.js:259:5)
        message: 'Failed to connect to 1.1.1.1:1433 in 15000ms',
        code: 'ETIMEOUT' },
     name: 'ConnectionError' } }

[munir131]

We are also facing @dsbert's error too frequently.

[willmorgan]

FWIW Azure support suggests a connect timeout of 30 seconds.

I know that's high, but that is literally what they suggest.

[madal3x]

Hello,

I made some tests (on 4.2.1) and it seems that i cannot have the script recover after the database had errors for more than a while (1 min in my case). I've put at the end of this message the two files with which i tested, and here are the tests and the results i had:

Test1 (without setting acquireTimeoutMillis; stop and start db after 10s)

• start sql db
• run node index.js
  script is outputting the word "tick" and the content of the db table
• stop sql db and start sql db again after ~10seconds
  script is outputting the connection error and the word "tick"
  after sql db is started again, all the promises of the queries that were previously run will now resolve

Test2 (without acquireTimeoutMillis; stop and start db after >1min)

• start sql db
• run node index.js
  script is outputting the word "tick" and the content of the db table
• stop sql db and start sql db again after >1min
  script is outputting the connection error and the word "tick"
  after sql db is started again, the script continues to output "tick" without resolving nor rejecting neither the previous promises, nor new promises

Test3 (with acquireTimeoutMillis set to 10000; stop and start db after >1min)

• start sql db
• run node index.js
  script is outputting the word "tick" and the content of the db table
• stop sql db and start sql db again after >1min
  script is outputting the connection error, the word "tick"
  and the promises of queries that took longer than 10seconds to acquire a connection are rejected with the error: TimeoutError: ResourceRequest timed out
  after sql db is started again, the promises of queries continue to be rejected with the error: TimeoutError: ResourceRequest timed out

I would expect that in all these 3 types of tests, after the database is back up, to start resolving promises again. How could i achieve this, am i missing something?

index.js

process
    .on('unhandledRejection', (reason, promise) => {
        console.error(`unhandled rejection`);
        console.error(promise);
        console.error(reason);

        process.exit(1);
    })
    .on('uncaughtException', err => {
        const msg = `Uncaught exception: ${err.stack}\n exiting...`;

        console.error(msg);

        process.exit(1);
    });

const app = require('./app');
app(3);

app.js

const sql = require('mssql');

class Repo {
    constructor(connectionPool) {
        this.connectionPool = connectionPool;
    }

    async names() {
        const pool = await this.connectionPool;

        const queryRes = await pool
            .request()
            .query(
                'select name from names'
            );

        return queryRes['recordset'].map(r => r.name);
    }
}

function createConn() {
    const user = 'sa';
    const password = 'P@55w0rd';
    const server = 'localhost';
    const database = 'test';

    const pool = new sql.ConnectionPool({user, password, server, database, pool: {acquireTimeoutMillis: 10000}});

    pool.on('error', err => {
        console.error('onPoolError listener');
        console.error(err);
    });

    return pool.connect();
}

module.exports = (
    intervalSeconds,
    repo = new Repo(createConn())
) => {
    const main = () => run().catch(console.error);
    main();
    setInterval(
        main,
        intervalSeconds * 1000
    );

    setInterval(
        () => console.log('tick'),
        1500
    );

    async function run() {
        await repo.names()
            .then(res => console.log(`names res: ${res}`))
            .catch(err => console.error(`names err: ${err}`));
    }
};

[madal3x]

Opening a new issue, since reopening this one is not possible: #701

[madal3x]

Regarding the fix that was made by @willmorgan for this issue, https://github.com/tediousjs/node-mssql/pull/683/files#diff-740c6ca62b823be279cef39ece34cf7e

which rejects the promise for creating the pool. It looks to me that the method poolCreate is called when the connect method is called, so in the beginning. If then the connection was successful, poolCreate will return a resolved promise. And if later there are connection errors, that resolved promise cannot change, since a promise, resolved or rejected, is immutable.

[willmorgan]

To clarify, my understanding is that poolCreate is called any time a connection is made, not just when the pool itself is created. It may reject on the creation of a connection: if that's the case, the the pool library will try to create another.

That fix works for my team in production.

[madal3x]

The way i see it is that you connect when you instantiate the application, and afterwards you issue queries, and don't try to connect again.

When you say it's working for your team, can you tell me, is it working in the scenario that the database goes down and recovers? Also, could you please show a simplified code of how you are using it?

[willmorgan]

If I recall correctly, poolCreate is one of the "pool factory" methods. It calls the underlying Tedious _poolCreate method. If you read through, it should become clear that you're operating on singular connections rather than the pool itself: https://github.com/tediousjs/node-mssql/blob/master/lib/tedious.js#L194

We're using the pool configuration more or less as per the docs, so there's no special code to show there.

Regarding a total database outage, we haven't had one of those, but understanding how pool connection eviction works, as a guess, I'd expect to see RequestErrors due to timeouts for query operations on open connections that haven't yet been evicted. However just because a timeout occurs doesn't necessarily mean the connection should be evicted from the pool. Eventually you'd see lots of create operations to ensure a minimum size in the pool, and as it came up, those would eventually recover some sort of usable state.

[madal3x]

as i interpret the code, poolCreate is called when the ConnectionPool is instantiated. If connect() is successful, it will return a resolved Promise with the ConnectionPool. I cannot see any subsequent calls to connect, or reinstantiation of the ConnectionPool, for instance in query() methods.

The issue i encounter is that the library will continue returning timeout RequestError even after the database recovered. Maybe i didnt understand exactly what was the edge case that you tried to fix, can you tell me what edge case(s) you fixed by adding that reject in the poolCreate?

[willmorgan]

poolCreate is called when the ConnectionPool is instantiated

Yes, because when the connection pool starts it will start to populate the pool with connections. The first connect is there to test if the config is good.

the library will continue returning timeout RequestError even after the database recovered

That sounds plausible; my guess is that the mssql library might be trying to run the request through a connection to the database that existed prior to the unavailability.

If this is true then you might want to look at custom eviction logic to evict dead connections, or implement a more stringent validate handle (I tried this by running SELECT 1 and adding a short timeout to complete), to avoid borrowing them to begin with.

[madal3x]

Thanks, though I wasn't looking for an explanation on why poolCreate is called when it's called, i wanted to understand what is it that you fixed by adding that reject in there.

I managed to "hackfix" it so that it recovers by replacing the whole connection pool when there are errors. I had to do it in a timeout, since doing it immediately would somehow result in no on error handler attached and issueing unhandled rejections. Though this will lose previous issued queries. If someone would want to keep those, a queue would have to sit between the repo method and the connection pool.

const sql = require('mssql');

class Repo {
    constructor(cfg) {
        const self = this;

        self.config = cfg;
        self.timerNewConn = null;
        self.connectionPool = self.newConn();
    }

    newConn() {
        const self = this;

        const pool = new sql.ConnectionPool(self.config);
        pool.on('error', err => {
            console.error('onPoolError listener');
            //console.error(err);

            if (self.timerNewConn) {
                clearTimeout(self.timerNewConn);
            }

            self.timerNewConn = setTimeout(() => {
                pool.close().then(() => self.connectionPool = self.newConn());
            }, 7000)
        });

        return pool.connect(err => {
            if (err) {
                console.error('onPoolConnectError listener');
                console.error(err);

                if (self.timerNewConn) {
                    clearTimeout(self.timerNewConn);
                }

                self.timerNewConn = setTimeout(() => {
                    pool.close().then(() => self.connectionPool = self.newConn());
                }, 7000)
            }
        });
    }

    async names() {
        const pool = await this.connectionPool;

        const queryRes = await pool
            .request()
            .query(
                'select name from names'
            );

        return queryRes['recordset'].map(r => r.name);
    }
}

function connCfg() {
    const user = 'sa';
    const password = 'P@55w0rd';
    const server = 'localhost';
    const database = 'test';

    return {user, password, server, database/*, debug: true*/, pool: {
            min: 1,
            max: 5,
            acquireTimeoutMillis: 5000,
            evictionRunIntervalMillis: 500
        }};
}

module.exports = (
    intervalSeconds,
    repo = new Repo(connCfg())
) => {
    const main = () => run().catch(console.error);
    main();
    setInterval(
        main,
        intervalSeconds * 1000
    );

    async function run() {
        console.log("next");

        await repo.names()
            .then(res => console.log(`names res: ${res}`))
            .catch(err => console.error(`names err: ${err.stack}`));
    }
};

[madal3x]

Though your solution proposals @willmorgan sound plausible too.

[willmorgan]

Thanks, though I wasn't looking for an explanation on why poolCreate is called when it's called, i wanted to understand what is it that you fixed by adding that reject in there.

Cool, good luck.

[LoveMeWithoutAll]

@dsbert That's interesting. I haven't seen an ETIMEOUT error crash our Node services yet but I'll keep an eye out for that. Do you have a stack trace you can share? If there's a defect in my PR then would be keen to fix...

I am also facing same error frequently. So I have updated node-mssql version to v4.2.2. I'll keep an eye out for that.

[madal3x]

There is a fix in tedious that would be nice to be taken into node-mssql to fix this issue: https://github.com/tediousjs/tedious/pull/769

[madal3x]

Ah sorry, i see it was included in 4.2.2

[LoveMeWithoutAll]

@dsbert That's interesting. I haven't seen an ETIMEOUT error crash our Node services yet but I'll keep an eye out for that. Do you have a stack trace you can share? If there's a defect in my PR then would be keen to fix...

I am also facing same error frequently. So I have updated node-mssql version to v4.2.2. I'll keep an eye out for that.

I updated node-mssql version to v4.2.2. But I am facing same error too. Connection timeout.

[dhensby]

@LoveMeWithoutAll Can you provide some code that reproduces the error, the config and also details of the environment / platform you are running on, please?

[LoveMeWithoutAll]

@LoveMeWithoutAll Can you provide some code that reproduces the error, the config and also details of the environment / platform you are running on, please?

@dhensby I have encountered connection timeout error even if upgraded node-mssql to v4.2.2. I couldn't reproduce the error immediately. But after running 1~2 days, the error has occured. Getting connection every 1 second may raise the error? I'm speculating so. At now, I changed timing of getting connection to only once at a process lifetime, and keeping an eye out for that.

• environment/platform
• Windows Server 2008R2 standard
• MSSQL: Microsoft SQL Server 2014 - 12.0.4100.1 (X64)
  Apr 20 2015 17:29:27
  Copyright (c) Microsoft Corporation
  Enterprise Edition (64-bit) on Windows NT 6.3 (Build 9600: ) (Hypervisor)
• Node.js v8.12.0
• node-mssql v4.2.2

Error rasing code is below.

// ./config/dbConfig.json
{
  "user": "-",
  "password": "-",
  "server": "-",
  "port": 0,
  "database": "-",
  "options": {
    "encrypt": false
  },
  "pool": {
    "acquireTimeoutMillis": 15000
  }
}

// services/db.js
const sql = require("mssql")
const dbConfig = require("../config/dbConfig.json")
const { onDbError } = require("./log")
let connectionPool

sql.on("error", err => {
    sql.close()
    onDbError(err, "mssql error")
})

const getConnectionPool = async () => {
    try {
        connectionPool = await new sql.ConnectionPool(dbConfig).connect()
        connectionPool.on("error", async err => {
            await closeConnectionPool()
            onDbError(err, "getConnectionPool error")
        })
    } catch (err) {
        onDbError(err, "getConnectionPool error in catch")
    }
}

const closeConnectionPool = async () => {
    try {
        await connectionPool.close()
    } catch (err) {
        onDbError(err, "closeConnectionPool error")
    }
}

const getPushList = async () => {
    try {
        let result = await connectionPool.request()
            .input("GUBUN", sql.NVarChar, "ENGINE_PUSH")
            .input("CONDITION1", sql.NVarChar, "Android")
            .input("CONDITION2", sql.NVarChar, engineConfig.runningServer)
            .execute("b.dbo.TU_M_PUSH_XMLLIST")
        return result.recordset
    } catch (err) {
        onDbError(err, "getPushList")
        return []
    }
}

// ./server.js
// ...
const dbService = require("./services/db")
// ...
const sendPush = async () => {
    await dbService.getConnectionPool()
    dbService.getPushList()
        .then((msgList) => {
            fcmService.sendMsgList(msgList)
        })
        .catch(async (error) => {
            await dbService.closeConnectionPool()
            onServerError(error)
        })
        .finally(async () => {
            await  dbService.closeConnectionPool()
            setTimeout(() => {
                sendPush()
            }, 1000)
        })
}
// ...
sendPush()

error log is below

{ message: 'Error on getConnectionPool error in catch in db.js',
  level: 'error',
  timestamp: '2018-10-23 10:27:42',
  [Symbol(level)]: 'error',
  [Symbol(message)]: '{"message":"Error on getConnectionPool error in catch in db.js","level":"error","timestamp":"2018-10-23 10:27:42"}' }
{ ConnectionError: Failed to connect to 165.246.17.29:3361 in 15000ms
    at Connection.tedious.once.err (D:\push_engine\FcmPushEngine\node_modules\mssql\lib\tedious.js:237:17)
    at Object.onceWrapper (events.js:315:30)
    at emitOne (events.js:116:13)
    at Connection.emit (events.js:211:7)
    at Connection.connectTimeout (D:\push_engine\FcmPushEngine\node_modules\tedious\lib\connection.js:944:12)
    at Timeout._onTimeout (D:\push_engine\FcmPushEngine\node_modules\tedious\lib\connection.js:913:16)
    at ontimeout (timers.js:498:11)
    at tryOnTimeout (timers.js:323:5)
    at Timer.listOnTimeout (timers.js:290:5)
  code: 'ETIMEOUT',
  originalError: 
   { ConnectionError: Failed to connect to 165.246.17.29:3361 in 15000ms
    at ConnectionError (D:\push_engine\FcmPushEngine\node_modules\tedious\lib\errors.js:12:12)
    at Connection.connectTimeout (D:\push_engine\FcmPushEngine\node_modules\tedious\lib\connection.js:944:28)
    at Timeout._onTimeout (D:\push_engine\FcmPushEngine\node_modules\tedious\lib\connection.js:913:16)
    at ontimeout (timers.js:498:11)
    at tryOnTimeout (timers.js:323:5)
    at Timer.listOnTimeout (timers.js:290:5)
     message: 'Failed to connect to 165.246.17.29:3361 in 15000ms',
     code: 'ETIMEOUT' },
  name: 'ConnectionError',
  level: 'error',
  timestamp: '2018-10-23 10:27:42',
  [Symbol(level)]: 'error',
  [Symbol(message)]: '{"code":"ETIMEOUT","originalError":{"message":"Failed to connect to 165.246.17.29:3361 in 15000ms","code":"ETIMEOUT"},"name":"ConnectionError","level":"error","timestamp":"2018-10-23 10:27:42"}' }

[dhensby]

@LoveMeWithoutAll Something about your getConnectionPoolcode feels weird because you're always reassigning connectionPool with a new pool rather than making use of a pool that may have already been created...

Can you try changing getConnectionPool to be something more like:

const getConnectionPool = async () => {
+   if (connectionPool) {
+       return connectionPool;
+   }
    try {
        connectionPool = await new sql.ConnectionPool(dbConfig).connect()
        connectionPool.on("error", async err => {
            await closeConnectionPool()
            onDbError(err, "getConnectionPool error")
        })
    } catch (err) {
        onDbError(err, "getConnectionPool error in catch")
    }
}

There's a possibility you're just creating a connection pool every time and eventually hitting the maximum number of connections allowed on the server and so the new pool never connects again.

Can you monitor the number of active connections on the server and see if it is constantly increasing or remains stable?

[LoveMeWithoutAll]

@dhensby Thank you for your guide! I changed code as you recommended. And now 3 different version apps is running on my each server.

1. Old one(always reassigning connectionPool)
2. New one as you recommended
3. Another one(Getting connection once at starting app)

Until now, the number of active connections on the all servers are stable.

I'm keeping an eye out for that.

P.S.

1. I added some code on your if statement

const getConnectionPool = async () => {
    try {
+               if (connectionPool && connectionPool.connected) return
        connectionPool = await new sql.ConnectionPool(dbConfig).connect()
        connectionPool.on("error", async err => {
            await closeConnectionPool()
            onDbError(err, "getConnectionPool error")
        })
    } catch (err) {
        onDbError(err, "getConnectionPool error in catch")
    }
}

1. I used below query to check the number of active connections

SELECT PROGRAM_NAME, HOSTNAME, STATUS, COUNT(DBID) AS NumberOfConnections
FROM sys.sysprocesses
WHERE PROGRAM_NAME = 'node-mssql'
GROUP BY PROGRAM_NAME, HOSTNAME, STATUS

[dhensby]

1. I added some code on your if statement

It shouldn't matter if the connectionPool is connected or not because it could be in the process of connecting when it's called and you'll still be creating many pools that you lose reference to and thus can't close...

perhaps you should just un-assign the variable in the catch block?

[LoveMeWithoutAll]

1. I added some code on your if statement

It shouldn't matter if the connectionPool is connected or not because it could be in the process of connecting when it's called and you'll still be creating many pools that you lose reference to and thus can't close...

perhaps you should just un-assign the variable in the catch block?

@dhensby Thank you for your advice! I have agreed with you. I changed my code.

const getConnectionPool = async () => {
    try {
+       if (connectionPool) return
        connectionPool = await new sql.ConnectionPool(dbConfig).connect()
        connectionPool.on("error", async err => {
            await closeConnectionPool()
            onDbError(err, "getConnectionPool error")
        })
    } catch (err) {
+       connectionPool = null
        onDbError(err, "getConnectionPool error in catch")
    }
}

const closeConnectionPool = async () => {
    try {
        await connectionPool.close()
    } catch (err) {
+       connectionPool = null
        onDbError(err, "closeConnectionPool error")
    }
}

And so far, there is no connection problem in my app I am keeping an eye for 4 days!

[dhensby]

Fantastic

[LoveMeWithoutAll]

@dhensby Finally I got a same error Failed to connect to 165.246.17.29:3361 in 15000ms after 6 days running. No too many connections were in there. So I edited my code. When error occured, the app would be restarted by process.exit() without any trying to reconnect on DB.

[dhensby]

@LoveMeWithoutAll ok, so is it now completely resolved for you by fixing your code?

[LoveMeWithoutAll]

@dhensby All problems is resolved by restarting app when error occured. Thank you for your kindness!!