node-postgres blocks with faulty SQL #1082

brianc · 2016-07-19T14:46:46Z

I'm using ES6 with async/await and node-postgres with connection pooling.

Let there is a table with a column which has UNIQUE constraint and a primary key column, the following code block blocks on client.query:

export async function someFunction () {
    const client = Postgres.connect()

    try {
        //  First insertion
        const result = await client.query('INSERT INTO table (column) VALUES ($1::text) RETURNING *', ['test'])

        //  Second insertion, which will fail
        const secondResult = await client.query('INSERT INTO table (column) VALUES ($1::text) RETURNING *', ['test'])

        console.log(secondResult.rows)    //  never called
    } catch (err) {
        console.error(err)
    } finally {
        client.release()
    }
}

However, I'm expecting to see this in somewhere:

 ERROR:  duplicate key value violates unique constraint "table_column_key"
 DETAIL:  Key (column)=(test) already exists.

Am I doing something wrong?

vitaly-t · 2016-07-19T00:16:23Z

Your error is not related to this library, it is a pure server-side error, which states clearly that your insert query violates the unique table_column_key constraint.

brikou · 2016-07-19T13:59:42Z

Having the same issue so I monkey patched client.query method.

const query = client.query;
client.query = (sql, params) => new Promise((resolve, reject) => {
    query(sql, params, (error, result) => {
        if (error) {
            reject(error);
        } else {
            resolve(result);
        }
    });
});

vitaly-t · 2016-07-19T14:02:37Z

@brikou

Or you can use pg-promise to avoid monkey-patching like this 😉

brianc · 2016-07-19T14:05:27Z

Looks like there might be an issue with query not rejecting properly on
errors. Ill try to reproduce and if i cant ill let you know and might ask
for more info. Do you have a self-contained reproducable test case?

On Tuesday, July 19, 2016, Vitaly Tomilov notifications@github.com wrote:

Or you can use pg-promise https://github.com/vitaly-t/pg-promise to
avoid monkey-patching like this 😉

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#1082 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AADDoaP6ZXtxclPCdFqL2M1KzGoVLHkMks5qXNj9gaJpZM4JOtRk
.

brikou · 2016-07-19T14:10:03Z

select true
-- works as expected

select tru_______e -- little typo here
-- no errors thrown (as expected)

Using async/await or yield.

brianc · 2016-07-19T14:18:59Z

Just to be clear, you're using babel to transpile your code, correct?

brikou · 2016-07-19T14:30:35Z

With async/await I do as it is required, but with yield I think you can reproduce it without babel.

brianc · 2016-07-19T14:36:49Z

yeah you're spot on, just wrote a failing test now - fix incoming...I think I know what's going on.

The promise adapter I had implemented wasn't spec compliant: it didn't accept both `onSuccess` and `onFailure` in the call to `query#then`. This subtly broke yield & async/await because they both rely on `onError` being passed into `Promise#then`. The pool was also not returning the promise after a client was acquired, which broke awaiting on `pool.connect` - this is also fixed now.

itisbugra · 2016-07-19T14:47:00Z

I'm using babel with async-to-bluebird-promises transformation module.

Buğra Ekuklu

Github, Bitbucket, Stack Overflow: @Chatatata

SO Ready To Help

Please information if this is considered spam.

19 Tem 2016 tarihinde 17:19 saatinde, Brian C notifications@github.com şunları yazdı:

Just to be clear, you're using babel to transpile your code, correct?

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

brianc · 2016-07-19T14:48:01Z

@brikou - I've pushed a fix up here; once the tests pass I'll merge & push out a new patch version - thanks for reporting this and the information you supplied in helping me identify the bug! 💃

brikou · 2016-07-19T14:52:38Z

@brianc Thank you, can't wait for the release :)

brikou · 2016-07-19T15:12:05Z

test/integration/connection-pool/yield-support-body.js

+var co = require('co')
+
+var tid = setTimeout(function() {
+  throw new Error('Tests did not complete in tme')


ah thanks - i'll fix on master when I push the new version.

brianc · 2016-07-19T15:19:02Z

Published version with fix: pg@6.0.3

brikou reviewed Jul 19, 2016
View reviewed changes

brianc merged commit b1b2801 into master Jul 19, 2016

brianc deleted the 1082-invalid-promise-implementation branch July 19, 2016 15:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

node-postgres blocks with faulty SQL #1082

node-postgres blocks with faulty SQL #1082

brianc commented Jul 19, 2016

vitaly-t commented Jul 19, 2016

brikou commented Jul 19, 2016

vitaly-t commented Jul 19, 2016 •

edited

Loading

brianc commented Jul 19, 2016

brikou commented Jul 19, 2016

brianc commented Jul 19, 2016

brikou commented Jul 19, 2016

brianc commented Jul 19, 2016

itisbugra commented Jul 19, 2016

brianc commented Jul 19, 2016

brikou commented Jul 19, 2016

brikou Jul 19, 2016

brianc Jul 19, 2016

brianc commented Jul 19, 2016

node-postgres blocks with faulty SQL #1082

node-postgres blocks with faulty SQL #1082

Conversation

brianc commented Jul 19, 2016

vitaly-t commented Jul 19, 2016

brikou commented Jul 19, 2016

vitaly-t commented Jul 19, 2016 • edited Loading

brianc commented Jul 19, 2016

brikou commented Jul 19, 2016

brianc commented Jul 19, 2016

brikou commented Jul 19, 2016

brianc commented Jul 19, 2016

itisbugra commented Jul 19, 2016

brianc commented Jul 19, 2016

brikou commented Jul 19, 2016

brikou Jul 19, 2016

Choose a reason for hiding this comment

brianc Jul 19, 2016

Choose a reason for hiding this comment

brianc commented Jul 19, 2016

vitaly-t commented Jul 19, 2016 •

edited

Loading