Remove client from pool on connection end - #537

I don't understand the failing test (idle-timeout-tests.js).
What's supposed to be ending the test @brianc ?

Ok I do see that an "end" event is also emitted by the client at the end of a query, even with no error. In those cases we don't really want to remove the item from the pool. So what we'd need is a way to distinguish "good" connections from "bad" connections.
Now I'm not even sure if an "end" event received from a client while a query was in progress is a good signal for a "bad" connection :/

Still doesn't fix all cases of connection breaking after successful connection.
I've found another one resulting in this error:

Error: This socket is closed.
    at Socket._write (net.js:518:19)
    at Socket.write (net.js:510:15)
    at Connection.query (/usr/src/node/modules/node-postgres/lib/connection.js:177:15)
    at Query.submit (/usr/src/node/modules/node-postgres/lib/query.js:113:16)
    at Client._pulseQueryQueue (/usr/src/node/modules/node-postgres/lib/client.js:280:24)
    at Client.query (/usr/src/node/modules/node-postgres/lib/client.js:323:8)
    at Function.me.eventedQuery (/usr/src/node/apps/CartoDB-SQL-API/CartoDB-SQL-API/app/models/psql.js:185:36)

And subsequently failing to error out on an attempt to use the connection from the pool :'(

This new on fails ./test/unit/client/stream-and-query-error-interaction-tests.js

ARGH (I love testcases :)

I do not think emitting an error event here is correct. If the connection is ended that shouldn't result in the client object emitting an error. What about when the connection is ended on purpose such as when one calls client.end()? Calling client.end() and having the client emit an error is not good.

On Sun, Mar 16, 2014 at 07:41:09PM -0700, Brian C wrote:

I do not think emitting an error event here is correct.

Agreed, see a3a1cc9

--strk;

Sorry, you meant it's not good to get 'error' when calling client.end()
even if a query was in progress ? But is it correct to keep the client in the pool on 'end' ?

I actually think removing 'end'-ed connections from the pool is correct, but let's close this pull, too confusing. Will file a cleaner one.