brianc / node-pg-pool

A connection pool for node-postgres
MIT License
180 stars 64 forks source link

UnhandledPromiseRejectionWarning when connection fails, .catch() not firing #47

Closed PrimeLens closed 7 years ago

PrimeLens commented 7 years ago

I was testing some scenarios where the connection fails and got UnhandledPromiseRejectionWarning from node. I the catch from the client.query().catch() didn't fire so there seems to be no way to handle bad connection errors and send UI change back to browser. I haven't had time to investigate code, just making a note of this before I forget.

edit: scenarios I tried involved bad username, password or host string in the config object passed in to new Pool({ })

charmander commented 7 years ago

Please show your entire script.

PrimeLens commented 7 years ago

I butchered my code just down to the essentials

var pg = require('pg');
var Pool = require('pg-pool')
var pool = new Pool({
    user: 'myuser',
    password: 'muserpassword',
    database: 'mytestdatabase',  
    host: 'AAAAAAAAAAAAAlocalhost', // force a fail in connection
    port: 5433,
    max: 20,
    min: 4,
    idleTimeoutMillis: 1000
});

app.get('/data', (req, res) => {
    pool.connect().then(client => {
        client.query("SELECT * FROM mytable;")
        .then(data => {
            client.release();
            return res.json(data.rows);
        })
        .catch(e => {
            console.log('!!!!! not called !!!!!! ',e);
            client.release();
            return res.status(500).json({success: false, data: e});
        })
    });
}); 
charmander commented 7 years ago
pool.connect().then(client => {
    // …
});

You didn’t handle errors on pool.connect(). With a router that allows a promise to be returned and the convenient Pool.prototype.query:

app.get('/data', (req, res) => {
    return pool.query('SELECT * FROM mytable').then(result => {
        res.json(result.rows);
    });
});
PrimeLens commented 7 years ago

Here is another example without a router. In fact this code is copy/pasted from the example in the README.md file. The connection will fail because there is no database and I'm not user brianc with password secret. Only code I added is the line console.log('this line never runs');

var Pool = require('pg-pool')

//by default the pool uses the same
//configuration as whatever `pg` version you have installed
var pool = new Pool()

//you can pass properties to the pool
//these properties are passed unchanged to both the node-postgres Client constructor
//and the node-pool (https://github.com/coopernurse/node-pool) constructor
//allowing you to fully configure the behavior of both
var pool2 = new Pool({
  database: 'postgres',
  user: 'brianc',
  password: 'secret!',
  port: 5432,
  ssl: true,
  max: 20, //set pool max size to 20
  min: 4, //set min pool size to 4
  idleTimeoutMillis: 1000 //close idle clients after 1 second
})

pool2.connect().then(client => {
  client.query('select $1::text as name', ['pg-pool']).then(res => {
    client.release()
    console.log('hello from', res.rows[0].name)
  })
  .catch(e => {
    console.log('this line never runs');
    client.release()
    console.error('query error', e.message, e.stack)
  })
})
charmander commented 7 years ago

The README is missing error handling:

pool2.connect().then(client => {
  client.query('select $1::text as name', ['pg-pool']).then(res => {
    client.release()
    console.log('hello from', res.rows[0].name)
  })
  .catch(e => {
    client.release()
    console.error('query error', e.message, e.stack)
  })
})
.catch(e => {
  console.error('connection error', e.message, e.stack)
})

You shouldn’t use connect like this at all, though. query() when possible; Bluebird otherwise.

PrimeLens commented 7 years ago

Thanks for the explanation

PrimeLens commented 7 years ago

Oops I closed this prematurely. Reopened. I placed .catch for the outer promise (thanks for that) and it at least gives me a hook to send a response back to the client, however Node still kicks the following message

(node:32161) UnhandledPromiseRejectionWarning: Unhandled promise rejection 
(rejection id: 1): ReferenceError: client is not defined
(node:32161) DeprecationWarning: Unhandled promise rejections are deprecated. 
In the future, promise rejections that are not handled will terminate the 
Node.js process with a non-zero exit code.

Quoting

You shouldn’t use connect like this at all, though. query() when possible; Bluebird otherwise.

I'm copying and pasting from the docs !

charmander commented 7 years ago

Could you include the entire script that produces an unhandled promise rejection, please?

I'm copying and pasting from the docs !

The docs are minimal examples; Bluebird’s resource management will make your pool use more reliable.

PrimeLens commented 7 years ago

Sure!
Here is everything in its entirety

Error given is

(node:32161) UnhandledPromiseRejectionWarning: Unhandled promise rejection 
(rejection id: 1): ReferenceError: client is not defined
(node:32161) DeprecationWarning: Unhandled promise rejections are deprecated. 
In the future, promise rejections that are not handled will terminate the 
Node.js process with a non-zero exit code.

Code from my second post (which is also copy/pasta from readme) combined with your extra .catch for the connection error. Used with Node v7.5.0.

var Pool = require('pg-pool')

//by default the pool uses the same
//configuration as whatever `pg` version you have installed
var pool = new Pool()

//you can pass properties to the pool
//these properties are passed unchanged to both the node-postgres Client constructor
//and the node-pool (https://github.com/coopernurse/node-pool) constructor
//allowing you to fully configure the behavior of both
var pool2 = new Pool({
  database: 'postgres',
  user: 'brianc',
  password: 'secret!',
  port: 5432,
  ssl: true,
  max: 20, //set pool max size to 20
  min: 4, //set min pool size to 4
  idleTimeoutMillis: 1000 //close idle clients after 1 second
})

pool2.connect().then(client => {
  client.query('select $1::text as name', ['pg-pool']).then(res => {
    client.release()
    console.log('hello from', res.rows[0].name)
  })
  .catch(e => {
    client.release()
    console.error('query error', e.message, e.stack)
  })
}).catch(e => {
   console.error('connection error', e.message, e.stack)
})
charmander commented 7 years ago

Hmm… I can’t reproduce this. Which OS are you using, and which version of node-pg-pool?

PrimeLens commented 7 years ago

OSX Yosemite 10.10.5 "pg": "^6.1.2", "pg-pool": "^1.6.0"

erreina commented 7 years ago

Any progress on this? I am running with the same problem that @PrimeLens mentioned. With this code:

var Pool = require ('pg-pool');

var pool = new Pool({
        database: 'test',
        user: 'test',
        password: 'test',
        port: 5432,
        max: 10,
        min: 1,
        idleTimeoutMillis: 1000
});

pool
    .connect()
        .then(client =>
        {
            client.release();
            console.log("Connected Yayyyy!!");
        })
        .catch(error =>
        {
            client.release();
            console.log("Upss: " + error);
        });

I get this error:

(node:32380) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): ReferenceError: client is not defined

I am using:

"pg": "^6.4.0", "pg-pool": "^1.8.0"

on

Ubuntu 14.04.5 LTS

charmander commented 7 years ago

@erreina You’re using client.release() in .catch() but there is no client in scope.

erreina commented 7 years ago

@charmander That was it! My bad. Thank you.

himanichopra commented 7 years ago

I'm getting a similar error for the give code

var express = require('express');
var app = express();

function letsgo(req,res){
var sql = require("mssql");

// config for your database
var config = {
    user: 'sa',
    password: '123456',
    server: 'localhost', 
    database: 'Form'

};
console.log('I am running....');

var resultt = sql.connect(config, function (err) {
console.log('Inside connectSql -- IFFF-ERR....');

    if (err) {
        console.log(err);
    }

    // create Request object
    var request = new sql.Request();

    // query to the database and get the records
    request.query('select * from Teacher_Name', function (err, recordset) {

               if (err) {
                   console.log(err);
               }

        // send records as a response
        res.send(recordset);

    });

});
console.log("Printing value of MSSQl connection : " + resultt);

var reqq = new sql.Request();

console.log("Printing value of reqq : " + reqq);

console.log("QueryOutput : " + reqq.query("select * from Teacher_Name"));

console.log("Below final QueryOutput");

//
//connectSql(sql, config);
// connect to your database
}   // JavaScript Documen

function connectSql(sql, config){
console.log('Inside connectSql.... - SQL = ' + sql);
console.log('Inside connectSql.... - CONFIG = ' + config);

}// connectSql

app.get('/', function (req, res) {
console.log('Inside app.get..');
letsgo(req,res);

});

var server = app.listen(1433, function () {
console.log('Server is running..');

                                    });

I'm getting this error:

(node:13916) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): ConnectionError: Connection is closed. (node:13916) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

charmander commented 7 years ago

@himanichopra, you’re using SQL Server and the mssql module. This repository is a PostgreSQL client and has nothing to do with that.

himanichopra commented 7 years ago

@charmander can you please tell me which module should I use to connect my database.

charmander commented 7 years ago

@himanichopra That depends on which database you’re using. If it’s PostgreSQL, pg is an appropriate module and you can find its documentation at https://node-postgres.com/. If not, you should look for support elsewhere.

himanichopra commented 7 years ago

@charmander I'm using Ms Sql Server

charmander commented 7 years ago

@himanichopra Okay, then this repository has nothing at all to do with your issue.

himanichopra commented 7 years ago

@charmander Thank you for help.

brianc commented 7 years ago

This should be fixed in pg-pool@2.0 (the original issue, anyway) 😄