Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Socket appears not to be released if there is a connection error #1516

Open
akc42 opened this issue Jan 25, 2023 · 3 comments
Open

Socket appears not to be released if there is a connection error #1516

akc42 opened this issue Jan 25, 2023 · 3 comments

Comments

@akc42
Copy link

akc42 commented Jan 25, 2023

Software versions

  • Tedious: 15.1.2
  • SQL Server: Microsoft SQL Server 2019 (RTM-CU4) (KB4548597) - 15.0.4033.1 (X64)
    Mar 14 2020 16:10:35
    Copyright (C) 2019 Microsoft Corporation
    Developer Edition (64-bit) on Linux (Ubuntu 18.04.4 LTS)
  • Node.js: 18.12.1

Additional Libraries Used and Versions

my own code
Table schema

N/A
Connection configuration

const connectionConfig = {
  server:kanga.home,
  authentication: {
    type: 'default',
    options: {
      userName: DBUSER,
      password: DBPASS,
    }
  },

  options: {
    port: 1433,
    database: 'PAS_Live',
    connectTimeout: 15000,
    requestTimeout: 60000,
    encrypt: false,
    trustServerCertificate: true
  }
};

`

Problem description
Start my Node.js application that uses tedious.
I am turning the database off (its running as a docker container, so I just tell docker to stop the container).
Attempt a database connection through my software and let it fail.
Start the docker container again so that the database restarts;
Retry a database connection through my software;
I continue to get the following error

 ConnectionError: Failed to connect to kanga.home:1433 - Could not connect (sequence)
     at Connection.socketError (/home/alan/dev/pasv5/node_modules/tedious/lib/connection.js:1395:28)
     at /home/alan/dev/pasv5/node_modules/tedious/lib/connection.js:1176:14
     at process.processTicksAndRejections (node:internal/process/task_queues:77:11) {
   code: 'ESOCKET',
   isTransient: undefined
 }

If I stop my application and then restart it. the error clears and I can use the database as normal

Expected behavior
Once the database has restarted connection requests should no longer fail, without restarting the application.

Actual behavior
As shown above, I continue to get socket failures at every attempt to call new Connection(config)

Error message/stack trace
see above

Any other details that can be helpful
I can't prove it, but it looks as through the socket is not properly closed on the connection error. As a result Node.js is not cleaning it up.

@MichaelSun90
Copy link
Contributor

Hi @akc42, the behavior could be caused by the socket error is thrown before the script can properly call connection. close().
Can you provide your script for the part that handling the error event? Just want to make sure that my assumption is correct.
If the call back for connect is looks like this:

connection.connect((err) => {
  if (err) {
    console.log('Connection Failed');
    throw err;
  }

  executeStatement();
});

The error will be thrown even before the internal logic for socket error handling and socket close has not fully finished. Can you try add a connection.close() call before the throw statement, see if that closed the socket properly? This should be just a walk around.

Also, I will discuses this within the team. I think the code could handle this part better, and the connection.close() call that added should not be necessary.

@akc42
Copy link
Author

akc42 commented Jan 27, 2023

@MichaelSun90 you have it about right, I just wrap things in promises, I decided to write a little test harness to test this. However I am finding that even the test harness only fails occasionally. Seemingly quicker without the process.on('uncaughtException'), although I could NOT find evidence of it being called when it was there. I already have a work around, I just exit the app and allow PM2 to restart it.

import {Connection, Request} from 'tedious';

const DBUSER= 'xxxxxxx';
const DBPASS= 'xxxxxxx'; 


const connectionConfig = {
  server: 'kanga.home',
  authentication: {
    type: 'default',
    options: {
      userName: DBUSER,
      password: DBPASS,
    }
  },

  options: {
   
    port: 1433,
    database: 'PAS_Live',
    connectTimeout: 15000,
    requestTimeout: 60000,
    encrypt: false,
    trustServerCertificate: true
  }
};
let BlockCount;
let InitialConnection;
let initialConnectionComplete = false 

process.on('SIGINT', async () => {
  if (initialConnectionComplete) {
    InitialConnection.close();
  }
  process.exit(0);  
});
process.on('uncaughtException', (err)  => {
  console.log('got uncaught exception');
}); 

while (true) { //loop until killed with Control C
  try {
    initialConnectionComplete = false 

    while (!initialConnectionComplete) {
      try {
      
        await new Promise((resolve, reject) => {
          InitialConnection = new Connection(connectionConfig);
          InitialConnection.once('connect', (err) => {
            if (err) {
              reject(err);
            } else {
              resolve();
            }
          });
          InitialConnection.connect();
        });
        initialConnectionComplete = true;
        await new Promise((resolve, reject) => {
          const reqCallback = row => {  //set as named function so we can refer to in the request.off and request.on calls.
            BlockCount = row[0].value;
            console.log('Block Count Set To', BlockCount);
          };
          InitialConnection.once('end', (err) => {
            if (err) {
              console.log('end errored with', err);
              reject(err);
            } else {
              resolve();
            }
          });
          const sql = `SELECT value FROM settings WHERE name = 'DbBlockCount'`;
          const request = new Request(sql, (err, rowcount) => {
            request.off('row', reqCallback);
            if (err) {
              console.log('request rejected with', err);
              reject(err);
            } else {
              console.log('initial database connection read', rowcount, 'rows');
            }
          });
          request.on('row', reqCallback);
          request.once('requestCompleted', () => {
            InitialConnection.close();
          });
          InitialConnection.execSql(request);
        });

      } catch (err) {
        console.log('connection failed with err', err);
        await new Promise(resolve => setTimeout(resolve, 10000)); //wait 10 secs before trying again
      }
    }
    console.log('completed a request')
    await new Promise(resolve => setTimeout(resolve, 3000)); //wait 3 secs before trying again
  } catch(err) {
    console.log('caught outer error', err);
  }
}

@akc42
Copy link
Author

akc42 commented Jan 27, 2023

My work around

        if (err) {
          if (err.code === 'ESOCKET') {
            logger('error', 'Tedious connection failed with socket failure, so we are exiting');
            process.kill(process.pid, "SIGINT"); //kill ourselves
          } else {
            logger('error','Failed to get a tedious connection with error' + err);
            reject(err);
          }
        } else {
          debug('got a connection event, about to resolve connection promise');
          accept(connection);
        }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants